Re: [WARNING] lockdep and kmemcheck_alloc_shadow

[David Rientjes]

On Sun, 20 May 2012, Ben Hutchings wrote:

> Seems to be a bug in SLUB:
> 
> > static struct page *allocate_slab(struct kmem_cache *s, gfp_t flags, int node)
> > {
> [...]
> > 	if (flags & __GFP_WAIT)
> > 		local_irq_disable();
> > 
> > 	if (!page)
> > 		return NULL;
> > 
> > 	if (kmemcheck_enabled
> > 		&& !(s->flags & (SLAB_NOTRACK | DEBUG_DEFAULT_FLAGS))) {
> > 		int pages = 1 << oo_order(oo);
> > 
> > 		kmemcheck_alloc_shadow(page, oo_order(oo), flags, node);
> [...]
> 
> So kmemcheck_alloc_shadow() runs with IRQs off, but uses the same flags
> as the original allocation.
> 
> Looks like this was introduced by:
> 
> commit 7e0528dadc9f8b04e4de0dba48a075100c2afe75
> Author: Christoph Lameter <cl@xxxxxxxxx>
> Date:   Wed Jun 1 12:25:44 2011 -0500
> 
>     slub: Push irq disable into allocate_slab()
> 
> which went into 3.1.
> 

Agreed, slab handles this correctly and it looks like slub ends up 
disabling irqs too early.

Does this fix it?  If so, we'll need to annotate it for stable as Ben 
noted.


mm, slub: ensure irqs are not disabled for kmemcheck allocation

kmemcheck_alloc_shadow() cannot be called with irqs disabled, which is
possible with __GFP_WAIT.  Ensure we always have them enabled at
allocation time like slab does.

Reported-by: Steven Rostedt <rostedt@xxxxxxxxxxx>
Suggested-by: Ben Hutchings <ben@xxxxxxxxxxxxxxx>
Signed-off-by: David Rientjes <rientjes@xxxxxxxxxx>
---
diff --git a/mm/slub.c b/mm/slub.c
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1314,13 +1314,7 @@ static struct page *allocate_slab(struct kmem_cache *s, gfp_t flags, int node)
 			stat(s, ORDER_FALLBACK);
 	}
 
-	if (flags & __GFP_WAIT)
-		local_irq_disable();
-
-	if (!page)
-		return NULL;
-
-	if (kmemcheck_enabled
+	if (page && kmemcheck_enabled
 		&& !(s->flags & (SLAB_NOTRACK | DEBUG_DEFAULT_FLAGS))) {
 		int pages = 1 << oo_order(oo);
 
@@ -1336,6 +1330,11 @@ static struct page *allocate_slab(struct kmem_cache *s, gfp_t flags, int node)
 			kmemcheck_mark_unallocated_pages(page, pages);
 	}
 
+	if (flags & __GFP_WAIT)
+		local_irq_disable();
+	if (!page)
+		return NULL;
+
 	page->objects = oo_objects(oo);
 	mod_zone_page_state(page_zone(page),
 		(s->flags & SLAB_RECLAIM_ACCOUNT) ?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/