slub/debugobjects: lockup when freeing memory

[  690.770137] ? __this_cpu_preempt_check (lib/smp_processor_id.c:63)
[  690.770137] __slab_alloc (mm/slub.c:1732 mm/slub.c:2205 mm/slub.c:2369)
[  690.770137] ? __lock_acquire (kernel/locking/lockdep.c:3189)
[  690.770137] ? __debug_object_init (lib/debugobjects.c:100 lib/debugobjects.c:312)
[  690.770137] kmem_cache_alloc (mm/slub.c:2442 mm/slub.c:2484 mm/slub.c:2489)

[  690.770137] ? __debug_object_init (lib/debugobjects.c:100 lib/debugobjects.c:312)
[  690.770137] ? debug_object_activate (lib/debugobjects.c:439)
[  690.770137] __debug_object_init (lib/debugobjects.c:100 lib/debugobjects.c:312)
[  690.770137] debug_object_init (lib/debugobjects.c:365)
[  690.770137] rcuhead_fixup_activate (kernel/rcu/update.c:231)
[  690.770137] debug_object_activate (lib/debugobjects.c:280 lib/debugobjects.c:439)
[  690.770137] ? discard_slab (mm/slub.c:1486)
[  690.770137] __call_rcu (kernel/rcu/rcu.h:76 (discriminator 2) kernel/rcu/tree.c:2585 (discriminator 2))

Quoted 5 lines · Sasha Levin

[  690.770137] ? __this_cpu_preempt_check (lib/smp_processor_id.c:63)
[  690.770137] __slab_alloc (mm/slub.c:1732 mm/slub.c:2205 mm/slub.c:2369)
[  690.770137] ? __lock_acquire (kernel/locking/lockdep.c:3189)
[  690.770137] ? __debug_object_init (lib/debugobjects.c:100 lib/debugobjects.c:312)
[  690.770137] kmem_cache_alloc (mm/slub.c:2442 mm/slub.c:2484 mm/slub.c:2489)

Quoted 8 lines

[  690.770137] ? __debug_object_init (lib/debugobjects.c:100 lib/debugobjects.c:312)
[  690.770137] ? debug_object_activate (lib/debugobjects.c:439)
[  690.770137] __debug_object_init (lib/debugobjects.c:100 lib/debugobjects.c:312)
[  690.770137] debug_object_init (lib/debugobjects.c:365)
[  690.770137] rcuhead_fixup_activate (kernel/rcu/update.c:231)
[  690.770137] debug_object_activate (lib/debugobjects.c:280 lib/debugobjects.c:439)
[  690.770137] ? discard_slab (mm/slub.c:1486)
[  690.770137] __call_rcu (kernel/rcu/rcu.h:76 (discriminator 2) kernel/rcu/tree.c:2585 (discriminator 2))

__call_rcu does a slab allocation? This means __call_rcu can no longer be used in slab allocators? What happened?

Quoted 16 lines · Christoph Lameter

Quoted 13 lines · Sasha Levin

[  690.770137] ? __this_cpu_preempt_check (lib/smp_processor_id.c:63)
[  690.770137] __slab_alloc (mm/slub.c:1732 mm/slub.c:2205 mm/slub.c:2369)
[  690.770137] ? __lock_acquire (kernel/locking/lockdep.c:3189)
[  690.770137] ? __debug_object_init (lib/debugobjects.c:100 lib/debugobjects.c:312)
[  690.770137] kmem_cache_alloc (mm/slub.c:2442 mm/slub.c:2484 mm/slub.c:2489)
[  690.770137] ? __debug_object_init (lib/debugobjects.c:100 lib/debugobjects.c:312)
[  690.770137] ? debug_object_activate (lib/debugobjects.c:439)
[  690.770137] __debug_object_init (lib/debugobjects.c:100 lib/debugobjects.c:312)
[  690.770137] debug_object_init (lib/debugobjects.c:365)
[  690.770137] rcuhead_fixup_activate (kernel/rcu/update.c:231)
[  690.770137] debug_object_activate (lib/debugobjects.c:280 lib/debugobjects.c:439)
[  690.770137] ? discard_slab (mm/slub.c:1486)
[  690.770137] __call_rcu (kernel/rcu/rcu.h:76 (discriminator 2) kernel/rcu/tree.c:2585 (discriminator 2))

Quoted 2 lines

__call_rcu does a slab allocation? This means __call_rcu can no longer be used in slab allocators? What happened?

My guess is that the root cause is a double call_rcu(), call_rcu_sched(), call_rcu_bh(), or call_srcu().

Perhaps the DEBUG_OBJECTS code now allocates memory to report errors? That would be unfortunate...

Well, no. Look at the callchain:

__call_rcu
debug_object_activate
rcuhead_fixup_activate
debug_object_init
kmem_cache_alloc

So call rcu activates the object, but the object has no reference in the debug objects code so the fixup code is called which inits the object and allocates a reference ....

Quoted 8 lines · Thomas Gleixner

Well, no. Look at the callchain:

__call_rcu
debug_object_activate
rcuhead_fixup_activate
debug_object_init
kmem_cache_alloc

Quoted 3 lines

So call rcu activates the object, but the object has no reference in the debug objects code so the fixup code is called which inits the object and allocates a reference ....

So we need to init the object in the page struct before the __call_rcu?

Quoted 8 lines · Thomas Gleixner

Well, no. Look at the callchain:

__call_rcu
debug_object_activate
rcuhead_fixup_activate
debug_object_init
kmem_cache_alloc

Quoted 3 lines

So call rcu activates the object, but the object has no reference in the debug objects code so the fixup code is called which inits the object and allocates a reference ....

So we need to init the object in the page struct before the __call_rcu?

Quoted 8 lines · Thomas Gleixner

Well, no. Look at the callchain:

__call_rcu
debug_object_activate
rcuhead_fixup_activate
debug_object_init
kmem_cache_alloc

Quoted 3 lines

So call rcu activates the object, but the object has no reference in the debug objects code so the fixup code is called which inits the object and allocates a reference ....

So we need to init the object in the page struct before the __call_rcu?

Quoted 4 lines

Quoted 3 lines

So call rcu activates the object, but the object has no reference in the debug objects code so the fixup code is called which inits the object and allocates a reference ....

Quoted 1 line

So we need to init the object in the page struct before the __call_rcu?

And the needed APIs are now in mainline:

	void init_rcu_head(struct rcu_head *head);
	void destroy_rcu_head(struct rcu_head *head);

Over to you, Christoph! ;-)

Quoted 11 lines · Paul E. McKenney

Quoted 5 lines

Quoted 3 lines

So call rcu activates the object, but the object has no reference in the debug objects code so the fixup code is called which inits the object and allocates a reference ....

So we need to init the object in the page struct before the __call_rcu?

And the needed APIs are now in mainline:

	void init_rcu_head(struct rcu_head *head);
	void destroy_rcu_head(struct rcu_head *head);

Quoted 1 line

Over to you, Christoph! ;-)

The field we are using for the rcu head serves other purposes before the free action. We cannot init the field at slab creation as we thought since it is used for the queueing of slabs on the partial, free and full lists. The kmem_cache information is not available when doing the freeing so we must force the allocation of reserve fields and the use of the reserved areas for rcu on all kmem_caches.

I made this conditional on CONFIG_RCU_XYZ. This needs to be the actual Debug options that will require allocations when initializing rcu heads.

Also note that the allocations in the rcu head initialization must be restricted to non RCU slabs otherwise the recursion may not terminate.

Subject RFC: Allow allocations on initializing rcu fields in slub.

Signed-off-by Christoph Lameter <[email protected]>

Index: linux/mm/slub.c
===================================================================

mm/slub.c +52 -17

+/*
+ * We may have to do alloations during the initialization of the
+ * debug portion of the rcu structure for a slab. It must therefore
+ * be separately allocated and initized on allocation.
+ * We cannot overload the lru field in the page struct at all.
+ */
+#define need_reserve_slab_rcu 1
+#else
+/*
+ * Overload the lru field in struct page if it fits.
+ * Should struct rcu_head grow due to debugging fields etc then
+ * additional space will be allocated from the end of the slab to
+ * store the rcu_head.
+ */
+#define need_reserve_slab_rcu						\
+	(sizeof(((struct page *)NULL)->lru) < sizeof(struct rcu_head))
+#endif
+
+static struct rcu_head *get_rcu_head(struct kmem_cache *s, struct page *page)
+{
+	if (need_reserve_slab_rcu) {
+		int order = compound_order(page);
+		int offset = (PAGE_SIZE << order) - s->reserved;
+
+		VM_BUG_ON(s->reserved != sizeof(struct rcu_head));
+		return page_address(page) + offset;
+	} else {
+		/*
+		 * RCU free overloads the RCU head over the LRU
+		 */
+		return (void *)&page->lru;
+	}
+}
+
 static struct page *allocate_slab(struct kmem_cache *s, gfp_t flags, int node)
 {
 	struct page *page;

@@ -1357,6 +1392,21 @@ static struct page *allocate_slab(struct
 			kmemcheck_mark_unallocated_pages(page, pages);
 	}
 
+#ifdef CONFIG_RCU_DEBUG_XYZ
+	if (unlikely(s->flags & SLAB_DESTROY_BY_RCU) && page)
+		/*
+		 * Initialize rcu_head and potentially do other
+		 * allocations. Note that this is still a recursive
+		 * call into the allocator which may recurse endlessly
+		 * if the same kmem_cache is used for allocation here.
+		 *
+		 * So in order to be safe the slab caches used
+		 * in init_rcu_head must be restricted to be of the
+		 * non rcu kind only.
+		 */
+		init_rcu_head(get_rcu_head(s, page));
+#endif
+
 	if (flags & __GFP_WAIT)
 		local_irq_disable();
 	if (!page)

@@ -1452,13 +1502,13 @@ static void __free_slab(struct kmem_cach
 	memcg_uncharge_slab(s, order);
 }
 
-#define need_reserve_slab_rcu						\
-	(sizeof(((struct page *)NULL)->lru) < sizeof(struct rcu_head))
-
 static void rcu_free_slab(struct rcu_head *h)
 {
 	struct page *page;
 
+#ifdef CONFIG_RCU_DEBUG_XYZ
+	destroy_rcu_head(h);
+#endif
 	if (need_reserve_slab_rcu)
 		page = virt_to_head_page(h);
 	else

@@ -1469,24 +1519,9 @@ static void rcu_free_slab(struct rcu_hea
 
 static void free_slab(struct kmem_cache *s, struct page *page)
 {
-	if (unlikely(s->flags & SLAB_DESTROY_BY_RCU)) {
-		struct rcu_head *head;
-
-		if (need_reserve_slab_rcu) {
-			int order = compound_order(page);
-			int offset = (PAGE_SIZE << order) - s->reserved;
-
-			VM_BUG_ON(s->reserved != sizeof(*head));
-			head = page_address(page) + offset;
-		} else {
-			/*
-			 * RCU free overloads the RCU head over the LRU
-			 */
-			head = (void *)&page->lru;

Quoted 1 line

+#ifdef CONFIG_RCU_DEBUG_XYZ

If you make CONFIG_RCU_DEBUG_XYZ instead be CONFIG_DEBUG_OBJECTS_RCU_HEAD, then it will automatically show up when it needs to.

The rest looks plausible, for whatever that is worth.

Quoted 4 lines · Paul E. McKenney

Quoted 1 line

+#ifdef CONFIG_RCU_DEBUG_XYZ

If you make CONFIG_RCU_DEBUG_XYZ instead be CONFIG_DEBUG_OBJECTS_RCU_HEAD, then it will automatically show up when it needs to.

Ok.

Quoted 1 line

The rest looks plausible, for whatever that is worth.

We talked in the hallway about init_rcu_head not touching the contents of the rcu_head. If that is the case then we can simplify the patch.

We could also remove the #ifdefs if init_rcu_head and destroy_rcu_head are no ops if CONFIG_DEBUG_RCU_HEAD is not defined.

Index: linux/mm/slub.c
===================================================================

mm/slub.c +38 -17

@@ -1308,6 +1308,25 @@ static inline struct page *alloc_slab_pa
 	return page;
 }
 
+#define need_reserve_slab_rcu						\
+	(sizeof(((struct page *)NULL)->lru) < sizeof(struct rcu_head))
+
+static struct rcu_head *get_rcu_head(struct kmem_cache *s, struct page *page)
+{
+	if (need_reserve_slab_rcu) {
+		int order = compound_order(page);
+		int offset = (PAGE_SIZE << order) - s->reserved;
+
+		VM_BUG_ON(s->reserved != sizeof(struct rcu_head));
+		return page_address(page) + offset;
+	} else {
+		/*
+		 * RCU free overloads the RCU head over the LRU
+		 */
+		return (void *)&page->lru;
+	}
+}
+
 static struct page *allocate_slab(struct kmem_cache *s, gfp_t flags, int node)
 {
 	struct page *page;

@@ -1357,6 +1376,22 @@ static struct page *allocate_slab(struct
 			kmemcheck_mark_unallocated_pages(page, pages);
 	}
 
+#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
+	if (unlikely(s->flags & SLAB_DESTROY_BY_RCU) && page)
+		/*
+		 * Initialize various things. However, this init is
+	 	 * not allowed to modify the contents of the rcu head.
+		 * Allocations are permitted. However, the use of
+		 * the same cache or another cache with SLAB_RCU_DESTROY
+		 * set may cause additional recursions.
+		 *
+		 * So in order to be safe the slab caches used
+		 * in init_rcu_head should be restricted to be of the
+		 * non rcu kind only.
+		 */
+		init_rcu_head(get_rcu_head(s, page));
+#endif
+
 	if (flags & __GFP_WAIT)
 		local_irq_disable();
 	if (!page)

@@ -1452,13 +1487,13 @@ static void __free_slab(struct kmem_cach
 	memcg_uncharge_slab(s, order);
 }
 
-#define need_reserve_slab_rcu						\
-	(sizeof(((struct page *)NULL)->lru) < sizeof(struct rcu_head))
-
 static void rcu_free_slab(struct rcu_head *h)
 {
 	struct page *page;
 
+#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
+	destroy_rcu_head(h);
+#endif
 	if (need_reserve_slab_rcu)
 		page = virt_to_head_page(h);
 	else

@@ -1469,24 +1504,9 @@ static void rcu_free_slab(struct rcu_hea
 
 static void free_slab(struct kmem_cache *s, struct page *page)
 {
-	if (unlikely(s->flags & SLAB_DESTROY_BY_RCU)) {
-		struct rcu_head *head;
-
-		if (need_reserve_slab_rcu) {
-			int order = compound_order(page);
-			int offset = (PAGE_SIZE << order) - s->reserved;
-
-			VM_BUG_ON(s->reserved != sizeof(*head));
-			head = page_address(page) + offset;
-		} else {
-			/*
-			 * RCU free overloads the RCU head over the LRU
-			 */
-			head = (void *)&page->lru;

Quoted 2 lines

We could also remove the #ifdefs if init_rcu_head and destroy_rcu_head are no ops if CONFIG_DEBUG_RCU_HEAD is not defined.

And indeed they are, good point! It appears to me that both sets of #ifdefs can go away.

Quoted 5 lines · Paul E. McKenney

Quoted 2 lines

We could also remove the #ifdefs if init_rcu_head and destroy_rcu_head are no ops if CONFIG_DEBUG_RCU_HEAD is not defined.

And indeed they are, good point! It appears to me that both sets of #ifdefs can go away.

Ok then this is a first workable version I think. How do we test this?

From: Christoph Lameter <[email protected]>
Subject: slub: Add init/destroy function calls for rcu_heads

In order to do proper debugging for rcu_head use we need some additional structures allocated when an object potentially using a rcu_head is allocated in the slub allocator.

This adds the proper calls to init_rcu_head()
and destroy_rcu_head().

init_rcu_head() is a bit of an unusual function since:
1. It does not touch the contents of the rcu_head. This is required since the rcu_head is only used during slab_page freeing. Outside of that the same memory location is used for slab page list management. However, the initialization occurs when the slab page is initially allocated. So in the time between init_rcu_head() and destroy_rcu_head() there may be multiple uses of the indicated address as a list_head.

2. It is called without gfp flags and could potentially be called from atomic contexts. Allocations from init_rcu_head() context need to deal with this.

3. init_rcu_head() is called from within the slab allocation functions. Since init_rcu_head() calls the allocator again for more allocations it must avoid to use slabs that use rcu freeing. Otherwise endless recursion may occur (We may have to convince lockdep that what we do here is sane).

Signed-off-by Christoph Lameter <[email protected]>

Index: linux/mm/slub.c
===================================================================

mm/slub.c +43 -17

@@ -1308,6 +1308,25 @@ static inline struct page *alloc_slab_pa
 	return page;
 }
 
+#define need_reserve_slab_rcu						\
+	(sizeof(((struct page *)NULL)->lru) < sizeof(struct rcu_head))
+
+static struct rcu_head *get_rcu_head(struct kmem_cache *s, struct page *page)
+{
+	if (need_reserve_slab_rcu) {
+		int order = compound_order(page);
+		int offset = (PAGE_SIZE << order) - s->reserved;
+
+		VM_BUG_ON(s->reserved != sizeof(struct rcu_head));
+		return page_address(page) + offset;
+	} else {
+		/*
+		 * RCU free overloads the RCU head over the LRU
+		 */
+		return (void *)&page->lru;
+	}
+}
+
 static struct page *allocate_slab(struct kmem_cache *s, gfp_t flags, int node)
 {
 	struct page *page;

@@ -1357,6 +1376,29 @@ static struct page *allocate_slab(struct
 			kmemcheck_mark_unallocated_pages(page, pages);
 	}
 
+	if (unlikely(s->flags & SLAB_DESTROY_BY_RCU) && page)
+		/*
+		 * Initialize various things. However, this init is
+	 	 * not allowed to modify the contents of the rcu head.
+		 * The allocator typically overloads the rcu head over
+		 * page->lru which is also used to manage lists of
+		 * slab pages.
+		 *
+		 * Allocations are permitted in init_rcu_head().
+		 * However, the use of the same cache or another
+		 * cache with SLAB_DESTROY_BY_RCU set will cause
+		 * additional recursions.
+		 *
+		 * So in order to be safe the slab caches used
+		 * in init_rcu_head() should be restricted to be of the
+		 * non rcu kind only.
+		 *
+		 * Note also that no GFPFLAG is passed. The function
+		 * may therefore be called from atomic contexts
+		 * and somehow(?) needs to do the right thing.
+		 */
+		init_rcu_head(get_rcu_head(s, page));
+
 	if (flags & __GFP_WAIT)
 		local_irq_disable();
 	if (!page)

@@ -1452,13 +1494,11 @@ static void __free_slab(struct kmem_cach
 	memcg_uncharge_slab(s, order);
 }
 
-#define need_reserve_slab_rcu						\
-	(sizeof(((struct page *)NULL)->lru) < sizeof(struct rcu_head))
-
 static void rcu_free_slab(struct rcu_head *h)
 {
 	struct page *page;
 
+	destroy_rcu_head(h);
 	if (need_reserve_slab_rcu)
 		page = virt_to_head_page(h);
 	else

@@ -1469,24 +1509,9 @@ static void rcu_free_slab(struct rcu_hea
 
 static void free_slab(struct kmem_cache *s, struct page *page)
 {
-	if (unlikely(s->flags & SLAB_DESTROY_BY_RCU)) {
-		struct rcu_head *head;
-
-		if (need_reserve_slab_rcu) {
-			int order = compound_order(page);
-			int offset = (PAGE_SIZE << order) - s->reserved;
-
-			VM_BUG_ON(s->reserved != sizeof(*head));
-			head = page_address(page) + offset;
-		} else {
-			/*
-			 * RCU free overloads the RCU head over the LRU
-			 */
-			head = (void *)&page->lru;

Quoted 17 lines · Paul E. McKenney

Quoted 16 lines · Christoph Lameter

Quoted 13 lines · Sasha Levin

[  690.770137] ? __this_cpu_preempt_check (lib/smp_processor_id.c:63)
[  690.770137] __slab_alloc (mm/slub.c:1732 mm/slub.c:2205 mm/slub.c:2369)
[  690.770137] ? __lock_acquire (kernel/locking/lockdep.c:3189)
[  690.770137] ? __debug_object_init (lib/debugobjects.c:100 lib/debugobjects.c:312)
[  690.770137] kmem_cache_alloc (mm/slub.c:2442 mm/slub.c:2484 mm/slub.c:2489)
[  690.770137] ? __debug_object_init (lib/debugobjects.c:100 lib/debugobjects.c:312)
[  690.770137] ? debug_object_activate (lib/debugobjects.c:439)
[  690.770137] __debug_object_init (lib/debugobjects.c:100 lib/debugobjects.c:312)
[  690.770137] debug_object_init (lib/debugobjects.c:365)
[  690.770137] rcuhead_fixup_activate (kernel/rcu/update.c:231)
[  690.770137] debug_object_activate (lib/debugobjects.c:280 lib/debugobjects.c:439)
[  690.770137] ? discard_slab (mm/slub.c:1486)
[  690.770137] __call_rcu (kernel/rcu/rcu.h:76 (discriminator 2) kernel/rcu/tree.c:2585 (discriminator 2))

Quoted 8 lines

Quoted 2 lines

__call_rcu does a slab allocation? This means __call_rcu can no longer be used in slab allocators? What happened?

My guess is that the root cause is a double call_rcu(), call_rcu_sched(), call_rcu_bh(), or call_srcu().

Perhaps the DEBUG_OBJECTS code now allocates memory to report errors? That would be unfortunate...

Well, no. Look at the callchain:

__call_rcu
debug_object_activate
rcuhead_fixup_activate
debug_object_init
kmem_cache_alloc

So call rcu activates the object, but the object has no reference in the debug objects code so the fixup code is called which inits the object and allocates a reference ....

Quoted 36 lines · Thomas Gleixner

Quoted 36 lines · Paul E. McKenney

Quoted 18 lines

Quoted 18 lines · Christoph Lameter

Quoted 16 lines

Quoted 16 lines · Sasha Levin

Quoted 14 lines

Quoted 14 lines

[  690.770137] ? __this_cpu_preempt_check (lib/smp_processor_id.c:63)
[  690.770137] __slab_alloc (mm/slub.c:1732 mm/slub.c:2205 mm/slub.c:2369)
[  690.770137] ? __lock_acquire (kernel/locking/lockdep.c:3189)
[  690.770137] ? __debug_object_init (lib/debugobjects.c:100 lib/debugobjects.c:312)
[  690.770137] kmem_cache_alloc (mm/slub.c:2442 mm/slub.c:2484 mm/slub.c:2489)
[  690.770137] ? __debug_object_init (lib/debugobjects.c:100 lib/debugobjects.c:312)
[  690.770137] ? debug_object_activate (lib/debugobjects.c:439)
[  690.770137] __debug_object_init (lib/debugobjects.c:100 lib/debugobjects.c:312)
[  690.770137] debug_object_init (lib/debugobjects.c:365)
[  690.770137] rcuhead_fixup_activate (kernel/rcu/update.c:231)
[  690.770137] debug_object_activate (lib/debugobjects.c:280 lib/debugobjects.c:439)
[  690.770137] ? discard_slab (mm/slub.c:1486)
[  690.770137] __call_rcu (kernel/rcu/rcu.h:76 (discriminator 2) kernel/rcu/tree.c:2585 (discriminator 2))

Quoted 8 lines

Quoted 8 lines

Quoted 2 lines

Quoted 2 lines

__call_rcu does a slab allocation? This means __call_rcu can no longer be used in slab allocators? What happened?

My guess is that the root cause is a double call_rcu(), call_rcu_sched(), call_rcu_bh(), or call_srcu().

Perhaps the DEBUG_OBJECTS code now allocates memory to report errors? That would be unfortunate...

Well, no. Look at the callchain:

__call_rcu
debug_object_activate
rcuhead_fixup_activate
debug_object_init
kmem_cache_alloc

Quoted 3 lines

Quoted 3 lines

So call rcu activates the object, but the object has no reference in the debug objects code so the fixup code is called which inits the object and allocates a reference ....

OK, got it. And you are right, call_rcu() has done this for a very long time, so not sure what changed.

Quoted 40 lines · Paul E. McKenney

Quoted 39 lines · Thomas Gleixner

Quoted 39 lines · Paul E. McKenney

Quoted 18 lines

Quoted 18 lines · Christoph Lameter

Quoted 16 lines

Quoted 16 lines · Sasha Levin

Quoted 14 lines

Quoted 14 lines

[  690.770137] ? __this_cpu_preempt_check (lib/smp_processor_id.c:63)
[  690.770137] __slab_alloc (mm/slub.c:1732 mm/slub.c:2205 mm/slub.c:2369)
[  690.770137] ? __lock_acquire (kernel/locking/lockdep.c:3189)
[  690.770137] ? __debug_object_init (lib/debugobjects.c:100 lib/debugobjects.c:312)
[  690.770137] kmem_cache_alloc (mm/slub.c:2442 mm/slub.c:2484 mm/slub.c:2489)
[  690.770137] ? __debug_object_init (lib/debugobjects.c:100 lib/debugobjects.c:312)
[  690.770137] ? debug_object_activate (lib/debugobjects.c:439)
[  690.770137] __debug_object_init (lib/debugobjects.c:100 lib/debugobjects.c:312)
[  690.770137] debug_object_init (lib/debugobjects.c:365)
[  690.770137] rcuhead_fixup_activate (kernel/rcu/update.c:231)
[  690.770137] debug_object_activate (lib/debugobjects.c:280 lib/debugobjects.c:439)
[  690.770137] ? discard_slab (mm/slub.c:1486)
[  690.770137] __call_rcu (kernel/rcu/rcu.h:76 (discriminator 2) kernel/rcu/tree.c:2585 (discriminator 2))

Quoted 8 lines

Quoted 8 lines

Quoted 2 lines

Quoted 2 lines

__call_rcu does a slab allocation? This means __call_rcu can no longer be used in slab allocators? What happened?

My guess is that the root cause is a double call_rcu(), call_rcu_sched(), call_rcu_bh(), or call_srcu().

Perhaps the DEBUG_OBJECTS code now allocates memory to report errors? That would be unfortunate...

Well, no. Look at the callchain:

__call_rcu
debug_object_activate
rcuhead_fixup_activate
debug_object_init
kmem_cache_alloc

So call rcu activates the object, but the object has no reference in the debug objects code so the fixup code is called which inits the object and allocates a reference ....

Quoted 2 lines

OK, got it. And you are right, call_rcu() has done this for a very long time, so not sure what changed.

It's probable my fault. I've introduced clone() and unshare() fuzzing.

Those two are full with issues and I've been waiting with enabling those until the rest of the kernel could survive trinity for more than an hour.

Quoted 9 lines · Thomas Gleixner

On Thu, 19 Jun 2014, Paul E. McKenney wrote:
Well, no. Look at the callchain:

__call_rcu
debug_object_activate
rcuhead_fixup_activate
debug_object_init
kmem_cache_alloc

Quoted 3 lines

So call rcu activates the object, but the object has no reference in the debug objects code so the fixup code is called which inits the object and allocates a reference ....

OK, got it. And you are right, call_rcu() has done this for a very long time, so not sure what changed. But it seems like the right approach is to provide a debug-object-free call_rcu_alloc() for use by the memory allocators.

Seem reasonable? If so, please see the following patch.

Quoted 19 lines · Paul E. McKenney

Quoted 9 lines · Thomas Gleixner

On Thu, 19 Jun 2014, Paul E. McKenney wrote:
Well, no. Look at the callchain:

__call_rcu
debug_object_activate
rcuhead_fixup_activate
debug_object_init
kmem_cache_alloc

Quoted 3 lines

So call rcu activates the object, but the object has no reference in the debug objects code so the fixup code is called which inits the object and allocates a reference ....

OK, got it. And you are right, call_rcu() has done this for a very long time, so not sure what changed. But it seems like the right approach is to provide a debug-object-free call_rcu_alloc() for use by the memory allocators.

Quoted 1 line

Seem reasonable? If so, please see the following patch.

Not really, you're torpedoing the whole purpose of debugobjects :)

So, why can't we just init the rcu head when the stuff is created?

If that's impossible due to other memory allocator constraints, then instead of inventing a whole new API we can simply flag the relevent data in the memory allocator as we do with the debug objects mem cache itself (SLAB_DEBUG_OBJECTS).

Thanks,

tglx

Quoted 20 lines · Thomas Gleixner

Quoted 19 lines · Paul E. McKenney

Quoted 9 lines · Thomas Gleixner

On Thu, 19 Jun 2014, Paul E. McKenney wrote:
Well, no. Look at the callchain:

__call_rcu
debug_object_activate
rcuhead_fixup_activate
debug_object_init
kmem_cache_alloc

Quoted 3 lines

So call rcu activates the object, but the object has no reference in the debug objects code so the fixup code is called which inits the object and allocates a reference ....

OK, got it. And you are right, call_rcu() has done this for a very long time, so not sure what changed. But it seems like the right approach is to provide a debug-object-free call_rcu_alloc() for use by the memory allocators.

Quoted 5 lines

Quoted 1 line

Seem reasonable? If so, please see the following patch.

Not really, you're torpedoing the whole purpose of debugobjects :)

So, why can't we just init the rcu head when the stuff is created?

That would allow me to keep my code unchanged, so I am in favor. ;-)

Quoted 25 lines · Paul E. McKenney

Quoted 24 lines · Thomas Gleixner

Quoted 14 lines · Paul E. McKenney

Quoted 9 lines · Thomas Gleixner

On Thu, 19 Jun 2014, Paul E. McKenney wrote:
Well, no. Look at the callchain:

__call_rcu
debug_object_activate
rcuhead_fixup_activate
debug_object_init
kmem_cache_alloc

Quoted 3 lines

So call rcu activates the object, but the object has no reference in the debug objects code so the fixup code is called which inits the object and allocates a reference ....

Quoted 6 lines

OK, got it. And you are right, call_rcu() has done this for a very long time, so not sure what changed. But it seems like the right approach is to provide a debug-object-free call_rcu_alloc() for use by the memory allocators.

Seem reasonable? If so, please see the following patch.

Not really, you're torpedoing the whole purpose of debugobjects :)

Quoted 3 lines

Quoted 1 line

So, why can't we just init the rcu head when the stuff is created?

That would allow me to keep my code unchanged, so I am in favor. ;-)

Almost unchanged. You need to provide a function to do so, i.e. make use of

debug_init_rcu_head()

Quoted 31 lines · Thomas Gleixner

Quoted 27 lines · Paul E. McKenney

Quoted 25 lines · Thomas Gleixner

Quoted 14 lines · Paul E. McKenney

Quoted 9 lines · Thomas Gleixner

On Thu, 19 Jun 2014, Paul E. McKenney wrote:
Well, no. Look at the callchain:

__call_rcu
debug_object_activate
rcuhead_fixup_activate
debug_object_init
kmem_cache_alloc

Quoted 3 lines

So call rcu activates the object, but the object has no reference in the debug objects code so the fixup code is called which inits the object and allocates a reference ....

Quoted 6 lines

OK, got it. And you are right, call_rcu() has done this for a very long time, so not sure what changed. But it seems like the right approach is to provide a debug-object-free call_rcu_alloc() for use by the memory allocators.

Seem reasonable? If so, please see the following patch.

Not really, you're torpedoing the whole purpose of debugobjects :)

So, why can't we just init the rcu head when the stuff is created?

Quoted 6 lines

Quoted 1 line

That would allow me to keep my code unchanged, so I am in favor. ;-)

Almost unchanged. You need to provide a function to do so, i.e. make use of

debug_init_rcu_head()

You mean like this?

Quoted 39 lines · Paul E. McKenney

Quoted 37 lines · Thomas Gleixner

Quoted 21 lines · Paul E. McKenney

Quoted 20 lines · Thomas Gleixner

Quoted 19 lines · Paul E. McKenney

Quoted 9 lines · Thomas Gleixner

On Thu, 19 Jun 2014, Paul E. McKenney wrote:
Well, no. Look at the callchain:

__call_rcu
debug_object_activate
rcuhead_fixup_activate
debug_object_init
kmem_cache_alloc

Quoted 3 lines

So call rcu activates the object, but the object has no reference in the debug objects code so the fixup code is called which inits the object and allocates a reference ....

OK, got it. And you are right, call_rcu() has done this for a very long time, so not sure what changed. But it seems like the right approach is to provide a debug-object-free call_rcu_alloc() for use by the memory allocators.

Quoted 7 lines

Quoted 5 lines

Quoted 1 line

Seem reasonable? If so, please see the following patch.

Not really, you're torpedoing the whole purpose of debugobjects :)

So, why can't we just init the rcu head when the stuff is created?

That would allow me to keep my code unchanged, so I am in favor. ;-)

Almost unchanged. You need to provide a function to do so, i.e. make use of

debug_init_rcu_head()

Quoted 1 line

You mean like this?

I'd rather name it init_rcu_head() and free_rcu_head() w/o the debug_ prefix, so it's consistent with init_rcu_head_on_stack / destroy_rcu_head_on_stack. But either way works for me.

Acked-by Thomas Gleixner <[email protected]>

rcu: Export debug_init_rcu_head() and and debug_init_rcu_head()

Currently, call_rcu() relies on implicit allocation and initialization for the debug-objects handling of RCU callbacks. If you hammer the kernel hard enough with Sasha's modified version of trinity, you can end up with the sl*b allocators recursing into themselves via this implicit call_rcu() allocation.

This commit therefore exports the debug_init_rcu_head() and debug_rcu_head_free() functions, which permits the allocators to allocated and pre-initialize the debug-objects information, so that there no longer any need for call_rcu() to do that initialization, which in turn prevents the recursion into the memory allocators.

Reported-by Sasha Levin <[email protected]>

Suggested-by Thomas Gleixner <[email protected]>

Signed-off-by Paul E. McKenney <[email protected]>

Acked-by Thomas Gleixner <[email protected]>

Quoted 14 lines · Paul E. McKenney

rcu: Export debug_init_rcu_head() and and debug_init_rcu_head()

Currently, call_rcu() relies on implicit allocation and initialization for the debug-objects handling of RCU callbacks. If you hammer the kernel hard enough with Sasha's modified version of trinity, you can end up with the sl*b allocators recursing into themselves via this implicit call_rcu() allocation.

This commit therefore exports the debug_init_rcu_head() and debug_rcu_head_free() functions, which permits the allocators to allocated and pre-initialize the debug-objects information, so that there no longer any need for call_rcu() to do that initialization, which in turn prevents the recursion into the memory allocators.

Quoted 4 lines

Reported-by Sasha Levin <[email protected]>

Suggested-by Thomas Gleixner <[email protected]>

Signed-off-by Paul E. McKenney <[email protected]>

Acked-by Thomas Gleixner <[email protected]>

Hi Paul,

Oddly enough, I still see the issue in -next (I made sure that this patch was in the tree):

[  393.810123] =============================================
[  393.810123] [ INFO: possible recursive locking detected ]
[  393.810123] 3.16.0-rc4-next-20140711-sasha-00046-g07d3099-dirty #813 Not tainted
[  393.810123] ---------------------------------------------
[  393.810123] trinity-c32/9762 is trying to acquire lock:
[  393.810123] (&(&n->list_lock)->rlock){-.-...}, at: get_partial_node.isra.39 (mm/slub.c:1628)
[  393.810123]
[  393.810123] but task is already holding lock:
[  393.810123] (&(&n->list_lock)->rlock){-.-...}, at: __kmem_cache_shutdown (mm/slub.c:3210 mm/slub.c:3233 mm/slub.c:3244)
[  393.810123]
[  393.810123] other info that might help us debug this:
[  393.810123]  Possible unsafe locking scenario:
[  393.810123]
[  393.810123]        CPU0
[  393.810123]        ----
[  393.810123]   lock(&(&n->list_lock)->rlock);
[  393.810123]   lock(&(&n->list_lock)->rlock);
[  393.810123]
[  393.810123]  *** DEADLOCK ***
[  393.810123]
[  393.810123]  May be due to missing lock nesting notation
[  393.810123]
[  393.810123] 5 locks held by trinity-c32/9762:
[  393.810123] #0: (net_mutex){+.+.+.}, at: copy_net_ns (net/core/net_namespace.c:254)
[  393.810123] #1: (cpu_hotplug.lock){++++++}, at: get_online_cpus (kernel/cpu.c:90)
[  393.810123] #2: (mem_hotplug.lock){.+.+.+}, at: get_online_mems (mm/memory_hotplug.c:83)
[  393.810123] #3: (slab_mutex){+.+.+.}, at: kmem_cache_destroy (mm/slab_common.c:344)
[  393.810123] #4: (&(&n->list_lock)->rlock){-.-...}, at: __kmem_cache_shutdown (mm/slub.c:3210 mm/slub.c:3233 mm/slub.c:3244)
[  393.810123]
[  393.810123] stack backtrace:
[  393.810123] CPU: 32 PID: 9762 Comm: trinity-c32 Not tainted 3.16.0-rc4-next-20140711-sasha-00046-g07d3099-dirty #813
[  393.843284]  ffff880bc26730e0 0000000000000000 ffffffffb4ae7ff0 ffff880bc26a3848
[  393.843284]  ffffffffb0e47068 ffffffffb4ae7ff0 ffff880bc26a38f0 ffffffffac258586
[  393.843284]  ffff880bc2673e30 000000050000000a ffffffffb444dee0 ffff880bc2673e48
[  393.843284] Call Trace:
[  393.843284] dump_stack (lib/dump_stack.c:52)
[  393.843284] __lock_acquire (kernel/locking/lockdep.c:1739 kernel/locking/lockdep.c:1783 kernel/locking/lockdep.c:2115 kernel/locking/lockdep.c:3182)
[  393.843284] lock_acquire (kernel/locking/lockdep.c:3602)
[  393.843284] ? get_partial_node.isra.39 (mm/slub.c:1628)
[  393.843284] _raw_spin_lock (include/linux/spinlock_api_smp.h:143 kernel/locking/spinlock.c:151)
[  393.843284] ? get_partial_node.isra.39 (mm/slub.c:1628)
[  393.843284] get_partial_node.isra.39 (mm/slub.c:1628)
[  393.843284] ? check_irq_usage (kernel/locking/lockdep.c:1638)
[  393.843284] ? __slab_alloc (mm/slub.c:2307)
[  393.843284] ? __this_cpu_preempt_check (lib/smp_processor_id.c:63)
[  393.843284] __slab_alloc (mm/slub.c:1730 mm/slub.c:2208 mm/slub.c:2372)
[  393.843284] ? __debug_object_init (lib/debugobjects.c:100 lib/debugobjects.c:312)
[  393.843284] ? kvm_clock_read (./arch/x86/include/asm/preempt.h:90 arch/x86/kernel/kvmclock.c:86)
[  393.843284] ? sched_clock (./arch/x86/include/asm/paravirt.h:192 arch/x86/kernel/tsc.c:304)
[  393.843284] kmem_cache_alloc (mm/slub.c:2445 mm/slub.c:2487 mm/slub.c:2492)
[  393.843284] ? debug_smp_processor_id (lib/smp_processor_id.c:57)
[  393.843284] ? __debug_object_init (lib/debugobjects.c:100 lib/debugobjects.c:312)
[  393.843284] ? check_chain_key (kernel/locking/lockdep.c:2188)
[  393.843284] __debug_object_init (lib/debugobjects.c:100 lib/debugobjects.c:312)
[  393.843284] ? _raw_spin_unlock_irqrestore (include/linux/spinlock_api_smp.h:160 kernel/locking/spinlock.c:191)
[  393.843284] ? __this_cpu_preempt_check (lib/smp_processor_id.c:63)
[  393.843284] debug_object_init (lib/debugobjects.c:365)
[  393.843284] rcuhead_fixup_activate (kernel/rcu/update.c:260)
[  393.843284] debug_object_activate (lib/debugobjects.c:280 lib/debugobjects.c:439)
[  393.843284] ? preempt_count_sub (kernel/sched/core.c:2600)
[  393.843284] ? slab_cpuup_callback (mm/slub.c:1484)
[  393.843284] __call_rcu (kernel/rcu/rcu.h:76 (discriminator 8) kernel/rcu/tree.c:2665 (discriminator 8))
[  393.843284] ? __kmem_cache_shutdown (mm/slub.c:3210 mm/slub.c:3233 mm/slub.c:3244)
[  393.843284] call_rcu (kernel/rcu/tree_plugin.h:679)
[  393.843284] discard_slab (mm/slub.c:1522)
[  393.843284] __kmem_cache_shutdown (mm/slub.c:3210 mm/slub.c:3233 mm/slub.c:3244)
[  393.843284] kmem_cache_destroy (mm/slab_common.c:350)
[  393.843284] nf_conntrack_cleanup_net_list (net/netfilter/nf_conntrack_core.c:1569 (discriminator 3))
[  393.843284] nf_conntrack_pernet_exit (net/netfilter/nf_conntrack_standalone.c:558)
[  393.843284] ops_exit_list.isra.1 (net/core/net_namespace.c:135)
[  393.843284] setup_net (net/core/net_namespace.c:180 (discriminator 3))
[  393.843284] copy_net_ns (net/core/net_namespace.c:255)
[  393.843284] create_new_namespaces (kernel/nsproxy.c:95)
[  393.843284] unshare_nsproxy_namespaces (kernel/nsproxy.c:190 (discriminator 4))
[  393.843284] SyS_unshare (kernel/fork.c:1865 kernel/fork.c:1814)
[  393.843284] tracesys (arch/x86/kernel/entry_64.S:542)

Thanks,
Sasha

This commit therefore exports the debug_init_rcu_head() and debug_rcu_head_free() functions, which permits the allocators to allocated and pre-initialize the debug-objects information, so that there no longer any need for call_rcu() to do that initialization, which in turn prevents the recursion into the memory allocators.

rcu: Provide call_rcu_alloc() and call_rcu_sched_alloc() to avoid recursion

The sl*b allocators use call_rcu() to manage object lifetimes, but call_rcu() can use debug-objects, which in turn invokes the sl*b allocators. These allocators are not prepared for this sort of recursion, which can result in failures.

This commit therefore creates call_rcu_alloc() and call_rcu_sched_alloc(), which act as their call_rcu() and call_rcu_sched() counterparts, but which avoid invoking debug-objects. These new API members are intended only for use by the sl*b allocators, and this commit makes the sl*b allocators use call_rcu_alloc(). Why call_rcu_sched_alloc()? Because in CONFIG_PREEMPT=n kernels, call_rcu() maps to call_rcu_sched(), so therefore call_rcu_alloc() must map to call_rcu_sched_alloc().

Reported-by Sasha Levin <[email protected]>

Set-straight-by Thomas Gleixner <[email protected]>

Signed-off-by Paul E. McKenney <[email protected]>

Quoted 15 lines · Paul E. McKenney

rcu: Provide call_rcu_alloc() and call_rcu_sched_alloc() to avoid recursion

The sl*b allocators use call_rcu() to manage object lifetimes, but call_rcu() can use debug-objects, which in turn invokes the sl*b allocators. These allocators are not prepared for this sort of recursion, which can result in failures.

This commit therefore creates call_rcu_alloc() and call_rcu_sched_alloc(), which act as their call_rcu() and call_rcu_sched() counterparts, but which avoid invoking debug-objects. These new API members are intended only for use by the sl*b allocators, and this commit makes the sl*b allocators use call_rcu_alloc(). Why call_rcu_sched_alloc()? Because in CONFIG_PREEMPT=n kernels, call_rcu() maps to call_rcu_sched(), so therefore call_rcu_alloc() must map to call_rcu_sched_alloc().

Quoted 3 lines

Reported-by Sasha Levin <[email protected]>

Set-straight-by Thomas Gleixner <[email protected]>

Signed-off-by Paul E. McKenney <[email protected]>

Paul, what is this patch based on? It won't apply cleanly on -next or Linus's tree.