On Tue, 2008-08-12 at 19:35 +1000, Neil Brown wrote:

On Tuesday August 12, a.p.zijlstra@chello.nl wrote:

quoted

On Tue, 2008-08-12 at 15:35 +1000, Neil Brown wrote:

quoted

On Thursday July 24, a.p.zijlstra@chello.nl wrote:

quoted

Restrict objects from reserve slabs (ALLOC_NO_WATERMARKS) to allocation
contexts that are entitled to it. This is done to ensure reserve pages don't
leak out and get consumed.

This looks good (we are still missing slob though, aren't we :-( )

I actually have that now, just needs some testing..

Cool!

quoted

quoted

quoted

@@ -1526,7 +1540,7 @@ load_freelist:
 	object = c->page->freelist;
 	if (unlikely(!object))
 		goto another_slab;
-	if (unlikely(SLABDEBUG && PageSlubDebug(c->page)))
+	if (unlikely(PageSlubDebug(c->page) || c->reserve))
 		goto debug;

This looks suspiciously like debugging code that you have left in.
Is it??

Its not, we need to force slub into the debug slow path when we have a
reserve page, otherwise we cannot do the permission check on each
allocation.

I see.... a little.  I'm trying to avoid understanding slub too
deeply, I don't want to use up valuable brain cell :-)

:-)

Would we be justified in changing the label from 'debug:' to
'slow_path:'  or something?  

Could do I guess.

Index: linux-2.6/mm/slub.c
===================================================================

--- linux-2.6.orig/mm/slub.c
+++ linux-2.6/mm/slub.c

@@ -1543,7 +1543,7 @@ load_freelist:
 	if (unlikely(!object))
 		goto another_slab;
 	if (unlikely(PageSlubDebug(c->page) || c->reserve))
-		goto debug;
+		goto slow_path;
 
 	c->freelist = object[c->offset];
 	c->page->inuse = c->page->objects;

@@ -1586,11 +1586,21 @@ grow_slab:
 		goto load_freelist;
 	}
 	return NULL;
-debug:
+
+slow_path:
 	if (PageSlubDebug(c->page) &&
 			!alloc_debug_processing(s, c->page, object, addr))
 		goto another_slab;
 
+	/*
+	 * Avoid the slub fast path in slab_alloc by not setting
+	 * c->freelist and the fast path in slab_fere by making 
+	 * node_match() fail by setting c->node to -1.
+	 *
+	 * We use this for for debug checks and reserve handling,
+	 * which needs to do permission checks on each allocation.
+	 */
+
 	c->page->inuse++;
 	c->page->freelist = object[c->offset];
 	c->node = -1;

And if it is just c->reserve, should
we avoid the call to alloc_debug_processing?

We already do:

	if (PageSlubDebug(c->page) &&
			!alloc_debug_processing(s, c->page, object, addr))
		goto another_slab;

since in that case PageSlubDebug() will be false.