Messages in this thread Patch in this message | | | Date | Tue, 3 Jul 2007 09:31:04 -0700 (PDT) | From | Christoph Lameter <> | Subject | Re: 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/
| |