lkml.org 
[lkml]   [2007]   [Jul]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: Linux 2.6.22-rc7
    On Tue, 3 Jul 2007, Linus Torvalds wrote:

    > > There seems to be a locking problem:
    >
    > Ok, I _think_ this is actually ok, and the lock validator is unhappy just
    > because we don't disable irq's when initializing the slab, so the fact
    > that we take the list_lock with interrupts enabled looks scary.

    Yes it is okay since no one else can use the slab at this point.

    > But the reason it seems to be ok is that it doesn't matter if interrupts
    > are enabled or not, because nobody can *get* to the list_lock, since the
    > thing hasn't been fully set up yet. So no interrupts will try to take the
    > lock (and cause any deadlocks) anyway.

    Right.

    > However, it might be worth avoiding the warning, even if it seems bogus in
    > this case. Christoph? Do you agree with the analysis? And the patch might
    > be as simple as changing early_kmem_cache_node_alloc() to enable
    > interrupts at the _end_ of the function, rather than immediately after
    > calling new_slab().

    new_slab() enables and disables interrupts during usual operations. During
    bootstrap interrupts are enabled and so new_slab() falsely disables
    interrupts when we do the alloc by hand thing in early_kmem_cache_node_alloc
    for NUMA. We need to enable interrupts there since otherwise boot will
    continue with interrupts disabled.

    > Andre, does that simple change fix it for you (move the
    > "local_irq_enable()" to the end of early_kmem_cache_node_alloc)?

    Yes that should make the lock checker happy since interrupts are disabled
    when calling add_partial(). local_irq_enable can be called later without a
    problem. Useless longer interrupt hold off though.



    SLUB: Make lockdep happy by not calling add_partial with interrupts
    enabled during bootstrap

    If we move the local_irq_enable() to the end of the function then
    add_partial() in early_kmem_cache_node_alloc() will be called
    with interrupts disabled like during regular operations.

    Signed-off-by: Christoph Lameter <clameter@sgi.com>

    ---
    mm/slub.c | 8 ++++++--
    1 file changed, 6 insertions(+), 2 deletions(-)

    Index: linux-2.6/mm/slub.c
    ===================================================================
    --- linux-2.6.orig/mm/slub.c 2007-07-03 09:27:00.000000000 -0700
    +++ linux-2.6/mm/slub.c 2007-07-03 09:27:45.000000000 -0700
    @@ -1798,8 +1798,6 @@ static struct kmem_cache_node * __init e
    BUG_ON(kmalloc_caches->size < sizeof(struct kmem_cache_node));

    page = new_slab(kmalloc_caches, gfpflags | GFP_THISNODE, node);
    - /* new_slab() disables interupts */
    - local_irq_enable();

    BUG_ON(!page);
    n = page->freelist;
    @@ -1811,6 +1809,12 @@ static struct kmem_cache_node * __init e
    init_kmem_cache_node(n);
    atomic_long_inc(&n->nr_slabs);
    add_partial(n, page);
    +
    + /*
    + * new_slab() disables interupts. If we do not reenable interrupts here
    + * then bootup would continue with interrupts disabled.
    + */
    + local_irq_enable();
    return n;
    }



    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2007-07-03 18:33    [W:3.536 / U:0.068 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site