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

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

@@ -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?  And if it is just c->reserve, should
we avoid the call to alloc_debug_processing?


Thanks,
NeilBrown

quoted

quoted

@@ -265,7 +267,8 @@ struct array_cache {
 	unsigned int avail;
 	unsigned int limit;
 	unsigned int batchcount;
-	unsigned int touched;
+	unsigned int touched:1,
+		     reserve:1;

This sort of thing always worries me.
It is a per-cpu data structure so you won't get SMP races corrupting
fields.  But you do get read-modify-write in place of simple updates.
I guess it's not a problem..  But it worries me :-)

Right,.. do people prefer I just add another int?