lkml.org 
[lkml]   [2011]   [May]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH] Revert "slub: Remove node check in slab_free"
On Wed, May 25, 2011 at 1:58 AM, Ingo Molnar <mingo@elte.hu> wrote:
>
> As Linus explained it's broken. Quoting Linus:

Actually, apparently Linus' explanation doesn't hold water.

There does seem to be something very broken in that commit, and a
revert is appropriate, but as Christoph says, "c" is a local per-cpu
thing that is protected by interrupts being disabled. So the case I
thought looked bad cannot happen - it's not actually protected by the
page lock.

That said, we have one bisect that pinpointed that commit, and you
reporting that stability improves dramatically by reverting it, so I
definitely will revert it. I would like to be able to figure out what
the problem with it is, though.

(Not that that will hold up the revert).

One thing that I don't understand is why the debug case does *any* of
that at all: it used to only do "c->node = NUMA_NO_NODE;" and even
that looks bogus. A normal successful allocation doesn't do it, so why
does the debug allocation? The normal allocation path just does

c->freelist = get_freepointer(s, object);
page->inuse = page->objects;
page->freelist = NULL;

so why does the debug path do something totally different:

page->inuse++;
page->freelist = get_freepointer(s, object);
deactivate_slab(s, c);
c->page = NULL;
c->node = NUMA_NO_NODE;

(with that "decativate_slab + c->page = NULL" being the new part to
the offending commit).

The code is also ugly as hell. "deactivate_slab()" already sets
c->page to NULL, and it probably should have set c->node too.
Whatever.

Christoph, please look into this. With the kind of confirmation we
have, there is no question that commit gets reverted. So I'll revert
it soon, but I'd still like to know what is going on.

Linus


\
 
 \ /
  Last update: 2011-05-25 16:57    [W:0.184 / U:0.872 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site