[lkml]   [2011]   [May]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    Patch in this message
    Subject[PATCH] Revert "slub: Remove node check in slab_free"

    * Ingo Molnar <> wrote:

    > * James Morris <> wrote:
    > > I started experiencing a boot hang in 2.6.39+, and bisected it to this
    > > change:
    > i too was seeing various instabilities and SLUB corruption in -tip testing.
    > Every 5th randconfig kernel crashes.
    > I started testing the revert.

    Stability of -tip has increased dramatically (36 test iterations and
    no crash), so yes this revert resolves the crashes.

    I've attached the changelogified revert patch below. James, since i
    was seeing these sporadically you've saved me a couple of hours of
    rather painful bisection work! :-)



    From 6ac0730862b6dd1b45bc86e8e61e4026293b09f9 Mon Sep 17 00:00:00 2001
    From: Ingo Molnar <>
    Date: Wed, 25 May 2011 09:37:47 +0200
    Subject: [PATCH] Revert "slub: Remove node check in slab_free"

    This reverts commit 442b06bcea23a01934d3da7ec5898fa154a6cafb.

    As Linus explained it's broken. Quoting Linus:

    That whole "deactivate_slab()" + "c->page = NULL" that this patch
    does looks bogus.

    Look at __slab_alloc(), we have:

    page = c->page;
    if (!page)
    goto new_slab;

    if (unlikely(!node_match(c, node)))
    goto another_slab;

    and let's assume we have two users racing on that "c->page". The
    "slab_lock()" is going to work for one of them, right?

    Ok, so the one it works for will then hit:

    if (kmem_cache_debug(s))
    goto debug;

    and thus get to the new "deactivate_slab(s,c) + c->page = NULL" and
    then unlock the page.

    In the meantime, the one that wasn't able to lock the page will now
    go forward, but will not have "node_match()" any more, so it does
    that "goto another_slab".

    Which does "deactivate_slab(s,c)" again, and now c->page is NULL, so
    that totally breaks.

    Reported-and-bisected-by: James Morris <>
    Analyzed-by: Linus Torvalds <>
    Acked-by: Pekka Enberg <>
    Signed-off-by: Ingo Molnar <>
    mm/slub.c | 4 +---
    1 files changed, 1 insertions(+), 3 deletions(-)

    diff --git a/mm/slub.c b/mm/slub.c
    index 4ea7f1a..ed1281b 100644
    --- a/mm/slub.c
    +++ b/mm/slub.c
    @@ -1881,8 +1881,6 @@ debug:

    page->freelist = get_freepointer(s, object);
    - deactivate_slab(s, c);
    - c->page = NULL;
    c->node = NUMA_NO_NODE;
    goto unlock_out;
    @@ -2114,7 +2112,7 @@ redo:
    tid = c->tid;

    - if (likely(page == c->page)) {
    + if (likely(page == c->page && c->node != NUMA_NO_NODE)) {
    set_freepointer(s, object, c->freelist);

    if (unlikely(!irqsafe_cpu_cmpxchg_double(

     \ /
      Last update: 2011-05-25 11:01    [W:0.033 / U:48.620 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site