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 <[email protected]>
---
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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
