mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH for-next-fixes v2 0/5] memcg,slab: kmalloc_nolock() fixes
@ 2026-11-03 14:47 Harry Yoo (Meta)
  2026-11-03 14:47 ` [PATCH for-next-fixes v2 1/5] mm/memcontrol: defer obj_cgroup_release() to irq_work Harry Yoo (Meta)
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Harry Yoo (Meta) @ 2026-11-03 14:47 UTC (permalink / raw)
  To: Johannes Weiner, Michal Hocko, Roman Gushchin, Shakeel Butt,
	Muchun Song, Andrew Morton, Harry Yoo, Alexei Starovoitov,
	Vlastimil Babka, Hao Li, Christoph Lameter, David Rientjes,
	Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt
  Cc: kernel-team, cgroups, linux-mm, linux-kernel, linux-rt-devel, Sashiko

This patchset fixes three kmalloc_nolock() bugs.

1. obj_cgroup_put() takes spinlocks in the release path when it drops
   the last reference, but it can be called from any context.

2. A spinlock may be taken in the following path and may lead
   to deadlock:

   kmalloc_nolock()
   -> slab_post_alloc_hook()
   -> memcg_slab_post_alloc_hook()
   -> memcg_alloc_abort_single().

3. irq_work_sync() is called without synchronization for rcuwait
   (on PREEMPT_RT or some architectures), potentially causing a
   hang.

Bug 1 was reported by lockdep, and bugs 2 [1] and 3 [2] were
reported by Sashiko.

Patch 1 only touches mm/memcontrol.c and no later patche depends on
it, so it can go through to the memcg tree.

Deferring obj_cgroup_release()
==============================

RFC v1 skipped the stock drain in __refill_obj_stock() when spinning is
not allowed. Making sure "obj_cgroup_put() is not called in a restricted
context that doesn't allow spinning." is easy to get wrong.

v2 makes obj_cgroup_release() safe in any context instead (as mentioned
in cover letter of the RFC v1). The release callback only queues the
objcg (llist_add() + irq_work_queue(), both NMI-safe) and the release
runs from irq_work.

This also fixes a performance regression in RFC v1 in a free-heavy
kmalloc_nolock() microbenchmark.

Changes in v2
=============

- Rebased onto the latest slab/for-next-fixes.

- Rebasing the series, writing patch 1 and 5, and verification was
  assisted by AI and carefully reviewed by Harry Yoo.

- Patch 1: replaced with the new approach described above.

- Patch 2:
  - Now introduces the allow_spin local variables itself
    (previously added by the old patch 1).
  - Addressed v1 review comments from Vlastimil (thanks!):
    - restored the blank lines.

- Patch 3 (new): call debug_locks and KCSAN checks in kfree_nolock()
  regardless of allow_spin, as they acquire a spinlock only when
  reporting a bug.

- Patch 5 (patch 4 in v1): Introduce a mutex for
  deferred_work_barrier() as it can no longer reuse slab_mutex. In v1
  the only caller was kmem_cache_destroy(). The barrier is now also
  called from kvfree_rcu_barrier() and friends, which are usable outside
  mm where slab_mutex is not visible. Dropped Vlastimil's Acked-by
  accordingly.

- Dropped the Cc: stable tags to avoid intrusive backports for unlikely
  issues in practice [3].

- Link to v1: https://lore.kernel.org/20260624-kmalloc-nolock-fixes-v1-0-fdf4d17351dd@kernel.org

[1] https://sashiko.dev/#/patchset/20260610-slab_alloc_flags-v2-0-7190909db118%40kernel.org?part=9
[2] https://sashiko.dev/#/patchset/20260615-kfree_rcu_nolock-v3-0-70a54f3775bb%40kernel.org?part=5
[3] https://lore.kernel.org/1816e94b-87e0-4276-8ff7-1fb651326ab3@kernel.org

---
Harry Yoo (Meta) (5):
      mm/memcontrol: defer obj_cgroup_release() to irq_work
      mm/slab: handle allow_spin in slab_free_hook() instead of open coding
      mm/slab: call debug_locks and KCSAN checks in kfree_nolock()
      mm/slab: fix a deadlock in memcg_alloc_abort_single()
      mm/slab: serialize deferred_work_barrier()

 include/linux/memcontrol.h |   2 +
 mm/memcontrol.c            |  27 +++++++++--
 mm/slub.c                  | 112 ++++++++++++++++++++++++++-------------------
 3 files changed, 91 insertions(+), 50 deletions(-)
---
base-commit: 03a176b9248c4513bb705ed3d931af3a7e9c2a08
change-id: 20260624-kmalloc-nolock-fixes-c97675328773

Best regards,
--  
Cheers,
Harry / Hyeonggon


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH for-next-fixes v2 1/5] mm/memcontrol: defer obj_cgroup_release() to irq_work
  2026-11-03 14:47 [PATCH for-next-fixes v2 0/5] memcg,slab: kmalloc_nolock() fixes Harry Yoo (Meta)
@ 2026-11-03 14:47 ` Harry Yoo (Meta)
  2026-11-03 14:47 ` [PATCH for-next-fixes v2 2/5] mm/slab: handle allow_spin in slab_free_hook() instead of open coding Harry Yoo (Meta)
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Harry Yoo (Meta) @ 2026-11-03 14:47 UTC (permalink / raw)
  To: Johannes Weiner, Michal Hocko, Roman Gushchin, Shakeel Butt,
	Muchun Song, Andrew Morton, Harry Yoo, Alexei Starovoitov,
	Vlastimil Babka, Hao Li, Christoph Lameter, David Rientjes,
	Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt
  Cc: kernel-team, cgroups, linux-mm, linux-kernel, linux-rt-devel

obj_cgroup_put() can be called from any context.
__memcg_slab_free_hook() puts a per-object reference on
every accounted free, and draining a per-cpu objcg stock slot puts
the cached reference. When it drops the last reference,
obj_cgroup_release() takes objcg_lock, frees percpu memory
(pcpu_lock, via percpu_ref_exit()) and calls kfree_rcu(). Each of
these can deadlock when the current context interrupted a holder of
the respective lock.

Defer the release to irq_work. llist_add() and irq_work_queue() are
NMI-safe, and the irq_work runs in a context where acquiring a
spinlock is allowed.

Add a separate llist_node instead of reusing the list/rcu union
because the list stays linked until the deferred release. struct
obj_cgroup grows from 56 to 64 bytes, but both are served from
kmalloc-64, so no memory is additionally consumed.

Closes: https://lore.kernel.org/20261001044056.75079-3-kmehltretter@gmail.com
Fixes: af92793e52c3 ("slab: Introduce kmalloc_nolock() and kfree_nolock().")
Signed-off-by: Harry Yoo (Meta) <harry@kernel.org>
---
 include/linux/memcontrol.h |  2 ++
 mm/memcontrol.c            | 27 ++++++++++++++++++++++++---
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 69e1bcc67c58..4d5c634daef8 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -185,6 +185,8 @@ struct obj_cgroup {
 		struct list_head list; /* protected by objcg_lock */
 		struct rcu_head rcu;
 	};
+	/* Used to defer obj_cgroup_release() to irq_work */
+	struct llist_node release_node;
 	bool is_root;
 };
 
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index a05d9a8baa1b..c53147a88580 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -147,9 +147,8 @@ static void memcg_uncharge_kmem(struct mem_cgroup *memcg, unsigned int nr_pages)
 		memcg_uncharge(memcg, nr_pages);
 }
 
-static void obj_cgroup_release(struct percpu_ref *ref)
+static void __obj_cgroup_release(struct obj_cgroup *objcg)
 {
-	struct obj_cgroup *objcg = container_of(ref, struct obj_cgroup, refcnt);
 	unsigned int nr_bytes;
 	unsigned int nr_pages;
 	unsigned long flags;
@@ -190,10 +189,32 @@ static void obj_cgroup_release(struct percpu_ref *ref)
 	list_del(&objcg->list);
 	spin_unlock_irqrestore(&objcg_lock, flags);
 
-	percpu_ref_exit(ref);
+	percpu_ref_exit(&objcg->refcnt);
 	kfree_rcu(objcg, rcu);
 }
 
+static LLIST_HEAD(objcg_release_llist);
+
+static void objcg_release_workfn(struct irq_work *work)
+{
+	struct llist_node *head = llist_del_all(&objcg_release_llist);
+	struct obj_cgroup *objcg, *tmp;
+
+	llist_for_each_entry_safe(objcg, tmp, head, release_node)
+		__obj_cgroup_release(objcg);
+}
+
+static DEFINE_IRQ_WORK(objcg_release_work, objcg_release_workfn);
+
+static void obj_cgroup_release(struct percpu_ref *ref)
+{
+	struct obj_cgroup *objcg = container_of(ref, struct obj_cgroup, refcnt);
+
+	/* The last put may come from a context that cannot take spinlocks */
+	if (llist_add(&objcg->release_node, &objcg_release_llist))
+		irq_work_queue(&objcg_release_work);
+}
+
 static struct obj_cgroup *obj_cgroup_alloc(void)
 {
 	struct obj_cgroup *objcg;

-- 
2.54.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH for-next-fixes v2 2/5] mm/slab: handle allow_spin in slab_free_hook() instead of open coding
  2026-11-03 14:47 [PATCH for-next-fixes v2 0/5] memcg,slab: kmalloc_nolock() fixes Harry Yoo (Meta)
  2026-11-03 14:47 ` [PATCH for-next-fixes v2 1/5] mm/memcontrol: defer obj_cgroup_release() to irq_work Harry Yoo (Meta)
@ 2026-11-03 14:47 ` Harry Yoo (Meta)
  2026-11-03 14:47 ` [PATCH for-next-fixes v2 3/5] mm/slab: call debug_locks and KCSAN checks in kfree_nolock() Harry Yoo (Meta)
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Harry Yoo (Meta) @ 2026-11-03 14:47 UTC (permalink / raw)
  To: Johannes Weiner, Michal Hocko, Roman Gushchin, Shakeel Butt,
	Muchun Song, Andrew Morton, Harry Yoo, Alexei Starovoitov,
	Vlastimil Babka, Hao Li, Christoph Lameter, David Rientjes,
	Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt
  Cc: kernel-team, cgroups, linux-mm, linux-kernel, linux-rt-devel

kfree_nolock() deliberately skips slab free hooks that should not be
called in unknown context. However, there is one more place that
needs the same thing: memcg_alloc_abort_single().

Instead of open coding slab_free_hook(), let it handle the allow_spin
parameter. This is required to correctly handle allocation failures due
to memcg charging.

There are two functional changes from this.

First, memcg_alloc_abort_single() -> slab_free_hook() path no longer
calls free hooks that acquire spinlocks if !allow_spin.

Second, kfree_nolock() now follows init_on_free which has been ignored.

Besides that, no functional change intended.

Closes: https://lore.kernel.org/asiSUpAYYU8Od4HM@gmail.com
Fixes: af92793e52c3 ("slab: Introduce kmalloc_nolock() and kfree_nolock().")
Signed-off-by: Harry Yoo (Meta) <harry@kernel.org>
---
 mm/slub.c | 104 +++++++++++++++++++++++++++++++++-----------------------------
 1 file changed, 56 insertions(+), 48 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index 45e077c7526c..36e7bb93418a 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -2686,41 +2686,54 @@ struct rcu_delayed_free {
  * - memcg_slab_post_alloc_hook()
  * - alloc_tagging_slab_alloc_hook()
  *
+ * slab_free_hook() assumes that memcg_slab_free_hook() and
+ * alloc_tagging_slab_free_hook() are already invoked.
+ *
  * Free hooks that must handle missing corresponding alloc hooks:
  * - kmemleak_free_recursive()
  * - kfence_free()
  *
- * Free hooks that have no alloc hook counterpart, and thus safe to call:
+ * Free hooks that have no alloc hook counterpart:
  * - debug_check_no_locks_freed()
  * - debug_check_no_obj_freed()
  * - __kcsan_check_access()
  */
 static __always_inline
 bool slab_free_hook(struct kmem_cache *s, void *x, bool init,
-		    bool after_rcu_delay)
+		    bool after_rcu_delay, bool allow_spin)
 {
 	/* Are the object contents still accessible? */
 	bool still_accessible = (s->flags & SLAB_TYPESAFE_BY_RCU) && !after_rcu_delay;
 
-	kmemleak_free_recursive(x, s->flags);
 	kmsan_slab_free(s, x);
 
-	debug_check_no_locks_freed(x, s->object_size);
+	/* Hooks that acquire locks or kmalloc_nolock() skips alloc hook counterpart */
+	if (likely(allow_spin)) {
+		kmemleak_free_recursive(x, s->flags);
+		debug_check_no_locks_freed(x, s->object_size);
 
-	if (!(s->flags & SLAB_DEBUG_OBJECTS))
-		debug_check_no_obj_freed(x, s->object_size);
+		if (!(s->flags & SLAB_DEBUG_OBJECTS))
+			debug_check_no_obj_freed(x, s->object_size);
 
-	/* Use KCSAN to help debug racy use-after-free. */
-	if (!still_accessible)
-		__kcsan_check_access(x, s->object_size,
-				     KCSAN_ACCESS_WRITE | KCSAN_ACCESS_ASSERT);
+		/* Use KCSAN to help debug racy use-after-free. */
+		if (!still_accessible)
+			__kcsan_check_access(x, s->object_size,
+					     KCSAN_ACCESS_WRITE | KCSAN_ACCESS_ASSERT);
 
-	if (kfence_free(x))
-		return false;
+		if (kfence_free(x))
+			return false;
+	}
 
 	/*
 	 * Give KASAN a chance to notice an invalid free operation before we
 	 * modify the object.
+	 *
+	 * If KASAN finds a bug it will do kasan_report_invalid_free() which
+	 * will call raw_spin_lock_irqsave() which is technically unsafe from
+	 * NMI, but take chance and report kernel bug. The sequence of
+	 * kasan_report_invalid_free() -> raw_spin_lock_irqsave() -> NMI
+	 * -> kfree_nolock() -> kasan_report_invalid_free() on the same CPU
+	 *  is double buggy and deserves to deadlock.
 	 */
 	if (kasan_slab_pre_free(s, x))
 		return false;
@@ -2729,6 +2742,7 @@ bool slab_free_hook(struct kmem_cache *s, void *x, bool init,
 	if (still_accessible) {
 		struct rcu_delayed_free *delayed_free;
 
+		VM_WARN_ON_ONCE(!allow_spin);
 		delayed_free = kmalloc_obj(*delayed_free, GFP_NOWAIT);
 		if (delayed_free) {
 			/*
@@ -2776,8 +2790,11 @@ bool slab_free_hook(struct kmem_cache *s, void *x, bool init,
 		set_orig_size(s, x, orig_size);
 
 	}
-	/* KASAN might put x into memory quarantine, delaying its reuse. */
-	return !kasan_slab_free(s, x, init, still_accessible, false);
+	/*
+	 * KASAN might put x into memory quarantine, delaying its reuse.
+	 * Skip quarantine when spinning is not allowed as it uses a spinlock.
+	 */
+	return !kasan_slab_free(s, x, init, still_accessible, !allow_spin);
 }
 
 static __fastpath_inline
@@ -2789,9 +2806,10 @@ bool slab_free_freelist_hook(struct kmem_cache *s, void **head, void **tail,
 	void *next = *head;
 	void *old_tail = *tail;
 	bool init;
+	bool allow_spin = true;
 
 	if (is_kfence_address(next)) {
-		slab_free_hook(s, next, false, false);
+		slab_free_hook(s, next, false, false, allow_spin);
 		return false;
 	}
 
@@ -2806,7 +2824,7 @@ bool slab_free_freelist_hook(struct kmem_cache *s, void **head, void **tail,
 		next = get_freepointer(s, object);
 
 		/* If object's reuse doesn't have to be delayed */
-		if (likely(slab_free_hook(s, object, init, false))) {
+		if (likely(slab_free_hook(s, object, init, false, allow_spin))) {
 			/* Move object to the new freelist */
 			set_freepointer(s, object, *head);
 			*head = object;
@@ -3031,6 +3049,7 @@ static bool __rcu_free_sheaf_prepare(struct kmem_cache *s,
 	void **p = &sheaf->objects[0];
 	unsigned int i = 0;
 	bool pfmemalloc = false;
+	bool allow_spin = true;
 
 	while (i < sheaf->size) {
 		struct slab *slab = virt_to_slab(p[i]);
@@ -3038,7 +3057,7 @@ static bool __rcu_free_sheaf_prepare(struct kmem_cache *s,
 		memcg_slab_free_hook(s, slab, p + i, 1);
 		alloc_tagging_slab_free_hook(s, slab, p + i, 1);
 
-		if (unlikely(!slab_free_hook(s, p[i], init, true))) {
+		if (unlikely(!slab_free_hook(s, p[i], init, true, allow_spin))) {
 			p[i] = p[--sheaf->size];
 			continue;
 		}
@@ -6479,6 +6498,7 @@ static void free_to_pcs_bulk(struct kmem_cache *s, size_t size, void **p)
 	bool init = slab_want_init_on_free(s);
 	void **remote_objects = p;
 	unsigned int remote_nr = 0;
+	bool allow_spin = true;
 
 	/*
 	 * Process the free hooks and separate out remote objects by
@@ -6494,7 +6514,7 @@ static void free_to_pcs_bulk(struct kmem_cache *s, size_t size, void **p)
 		memcg_slab_free_hook(s, slab, p + i, 1);
 		alloc_tagging_slab_free_hook(s, slab, p + i, 1);
 
-		if (unlikely(!slab_free_hook(s, p[i], init, false))) {
+		if (unlikely(!slab_free_hook(s, p[i], init, false, allow_spin))) {
 			p[i] = p[--size];
 			continue;
 		}
@@ -6624,13 +6644,17 @@ static __fastpath_inline
 void slab_free(struct kmem_cache *s, struct slab *slab, void *object,
 	       unsigned long addr)
 {
+	bool allow_spin = true;
+	bool init = slab_want_init_on_free(s);
+
 	memcg_slab_free_hook(s, slab, &object, 1);
 	alloc_tagging_slab_free_hook(s, slab, &object, 1);
 
-	if (unlikely(!slab_free_hook(s, object, slab_want_init_on_free(s), false)))
+	if (unlikely(!slab_free_hook(s, object, init, false, allow_spin)))
 		return;
 
-	if (likely(can_free_to_pcs(slab)) && likely(free_to_pcs(s, object, true)))
+	if (likely(can_free_to_pcs(slab)) &&
+			likely(free_to_pcs(s, object, allow_spin)))
 		return;
 
 	__slab_free(s, slab, object, object, 1, addr);
@@ -6643,10 +6667,12 @@ static noinline
 void memcg_alloc_abort_single(struct kmem_cache *s, void *object)
 {
 	struct slab *slab = virt_to_slab(object);
+	bool init = slab_want_init_on_free(s);
+	bool allow_spin = true;
 
 	alloc_tagging_slab_free_hook(s, slab, &object, 1);
 
-	if (likely(slab_free_hook(s, object, slab_want_init_on_free(s), false)))
+	if (likely(slab_free_hook(s, object, init, false, allow_spin)))
 		__slab_free(s, slab, object, object, 1, _RET_IP_);
 }
 #endif
@@ -6675,6 +6701,8 @@ static void slab_free_after_rcu_debug(struct rcu_head *rcu_head)
 	void *object = delayed_free->object;
 	struct slab *slab = virt_to_slab(object);
 	struct kmem_cache *s;
+	bool allow_spin = true;
+	bool init;
 
 	kfree(delayed_free);
 
@@ -6688,8 +6716,9 @@ static void slab_free_after_rcu_debug(struct rcu_head *rcu_head)
 	if (WARN_ON(!(s->flags & SLAB_TYPESAFE_BY_RCU)))
 		return;
 
+	init = slab_want_init_on_free(s);
 	/* resume freeing */
-	if (slab_free_hook(s, object, slab_want_init_on_free(s), true)) {
+	if (slab_free_hook(s, object, init, true, allow_spin)) {
 		__slab_free(s, slab, object, object, 1, _THIS_IP_);
 		stat(s, FREE_SLOWPATH);
 	}
@@ -6938,6 +6967,8 @@ void kfree_nolock(const void *object)
 	struct slab *slab;
 	struct kmem_cache *s;
 	void *x = (void *)object;
+	bool allow_spin = false;
+	bool init;
 
 	if (unlikely(ZERO_OR_NULL_PTR(object)))
 		return;
@@ -6952,33 +6983,10 @@ void kfree_nolock(const void *object)
 
 	memcg_slab_free_hook(s, slab, &x, 1);
 	alloc_tagging_slab_free_hook(s, slab, &x, 1);
-	/*
-	 * Unlike slab_free() do NOT call the following:
-	 * kmemleak_free_recursive(x, s->flags);
-	 * debug_check_no_locks_freed(x, s->object_size);
-	 * debug_check_no_obj_freed(x, s->object_size);
-	 * __kcsan_check_access(x, s->object_size, ..);
-	 * kfence_free(x);
-	 * since they take spinlocks or not safe from any context.
-	 */
-	kmsan_slab_free(s, x);
-	/*
-	 * If KASAN finds a kernel bug it will do kasan_report_invalid_free()
-	 * which will call raw_spin_lock_irqsave() which is technically
-	 * unsafe from NMI, but take chance and report kernel bug.
-	 * The sequence of
-	 * kasan_report_invalid_free() -> raw_spin_lock_irqsave() -> NMI
-	 *  -> kfree_nolock() -> kasan_report_invalid_free() on the same CPU
-	 * is double buggy and deserves to deadlock.
-	 */
-	if (kasan_slab_pre_free(s, x))
+
+	init = slab_want_init_on_free(s);
+	if (unlikely(!slab_free_hook(s, x, init, false, allow_spin)))
 		return;
-	/*
-	 * memcg, kasan_slab_pre_free are done for 'x'.
-	 * The only thing left is kasan_poison without quarantine,
-	 * since kasan quarantine takes locks and not supported from NMI.
-	 */
-	kasan_slab_free(s, x, false, false, /* skip quarantine */true);
 
 	if (likely(can_free_to_pcs(slab)) && likely(free_to_pcs(s, x, false)))
 		return;

-- 
2.54.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH for-next-fixes v2 3/5] mm/slab: call debug_locks and KCSAN checks in kfree_nolock()
  2026-11-03 14:47 [PATCH for-next-fixes v2 0/5] memcg,slab: kmalloc_nolock() fixes Harry Yoo (Meta)
  2026-11-03 14:47 ` [PATCH for-next-fixes v2 1/5] mm/memcontrol: defer obj_cgroup_release() to irq_work Harry Yoo (Meta)
  2026-11-03 14:47 ` [PATCH for-next-fixes v2 2/5] mm/slab: handle allow_spin in slab_free_hook() instead of open coding Harry Yoo (Meta)
@ 2026-11-03 14:47 ` Harry Yoo (Meta)
  2026-11-03 14:47 ` [PATCH for-next-fixes v2 4/5] mm/slab: fix a deadlock in memcg_alloc_abort_single() Harry Yoo (Meta)
  2026-11-03 14:47 ` [PATCH for-next-fixes v2 5/5] mm/slab: serialize deferred_work_barrier() Harry Yoo (Meta)
  4 siblings, 0 replies; 9+ messages in thread
From: Harry Yoo (Meta) @ 2026-11-03 14:47 UTC (permalink / raw)
  To: Johannes Weiner, Michal Hocko, Roman Gushchin, Shakeel Butt,
	Muchun Song, Andrew Morton, Harry Yoo, Alexei Starovoitov,
	Vlastimil Babka, Hao Li, Christoph Lameter, David Rientjes,
	Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt
  Cc: kernel-team, cgroups, linux-mm, linux-kernel, linux-rt-devel

kfree_nolock() skips free hooks that may acquire spinlocks because it
can be called from any context. debug_check_no_locks_freed() and
__kcsan_check_access() are skipped as well.

However, they acquire a spinlock only when they report a bug.
debug_check_no_locks_freed() disables interrupts and walks
current->held_locks, and reaches printk only when a held lock is inside
the freed object. KCSAN takes report_lock only when it observes a race.

Just like KASAN, take the risk of potential deadlock for bug reporting.
Call debug_check_no_locks_freed() and __kcsan_check_access() regardless
of allow_spin.

Signed-off-by: Harry Yoo (Meta) <harry@kernel.org>
---
 mm/slub.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index 36e7bb93418a..48415f24e115 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -2707,19 +2707,25 @@ bool slab_free_hook(struct kmem_cache *s, void *x, bool init,
 
 	kmsan_slab_free(s, x);
 
+	/*
+	 * debug_locks, KCSAN, KASAN may acquire a spinlock to report a bug,
+	 * but otherwise don't acquire any lock. Take the risk of deadlock
+	 * for reporting.
+	 */
+	debug_check_no_locks_freed(x, s->object_size);
+
+	/* Use KCSAN to help debug racy use-after-free. */
+	if (!still_accessible)
+		__kcsan_check_access(x, s->object_size,
+				     KCSAN_ACCESS_WRITE | KCSAN_ACCESS_ASSERT);
+
 	/* Hooks that acquire locks or kmalloc_nolock() skips alloc hook counterpart */
 	if (likely(allow_spin)) {
 		kmemleak_free_recursive(x, s->flags);
-		debug_check_no_locks_freed(x, s->object_size);
 
 		if (!(s->flags & SLAB_DEBUG_OBJECTS))
 			debug_check_no_obj_freed(x, s->object_size);
 
-		/* Use KCSAN to help debug racy use-after-free. */
-		if (!still_accessible)
-			__kcsan_check_access(x, s->object_size,
-					     KCSAN_ACCESS_WRITE | KCSAN_ACCESS_ASSERT);
-
 		if (kfence_free(x))
 			return false;
 	}
@@ -2727,13 +2733,6 @@ bool slab_free_hook(struct kmem_cache *s, void *x, bool init,
 	/*
 	 * Give KASAN a chance to notice an invalid free operation before we
 	 * modify the object.
-	 *
-	 * If KASAN finds a bug it will do kasan_report_invalid_free() which
-	 * will call raw_spin_lock_irqsave() which is technically unsafe from
-	 * NMI, but take chance and report kernel bug. The sequence of
-	 * kasan_report_invalid_free() -> raw_spin_lock_irqsave() -> NMI
-	 * -> kfree_nolock() -> kasan_report_invalid_free() on the same CPU
-	 *  is double buggy and deserves to deadlock.
 	 */
 	if (kasan_slab_pre_free(s, x))
 		return false;

-- 
2.54.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH for-next-fixes v2 4/5] mm/slab: fix a deadlock in memcg_alloc_abort_single()
  2026-11-03 14:47 [PATCH for-next-fixes v2 0/5] memcg,slab: kmalloc_nolock() fixes Harry Yoo (Meta)
                   ` (2 preceding siblings ...)
  2026-11-03 14:47 ` [PATCH for-next-fixes v2 3/5] mm/slab: call debug_locks and KCSAN checks in kfree_nolock() Harry Yoo (Meta)
@ 2026-11-03 14:47 ` Harry Yoo (Meta)
  2026-11-03 18:40   ` Alexei Starovoitov
  2026-11-03 14:47 ` [PATCH for-next-fixes v2 5/5] mm/slab: serialize deferred_work_barrier() Harry Yoo (Meta)
  4 siblings, 1 reply; 9+ messages in thread
From: Harry Yoo (Meta) @ 2026-11-03 14:47 UTC (permalink / raw)
  To: Johannes Weiner, Michal Hocko, Roman Gushchin, Shakeel Butt,
	Muchun Song, Andrew Morton, Harry Yoo, Alexei Starovoitov,
	Vlastimil Babka, Hao Li, Christoph Lameter, David Rientjes,
	Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt
  Cc: kernel-team, cgroups, linux-mm, linux-kernel, linux-rt-devel, Sashiko

When kmalloc_nolock() successfully grabs a slab object but memcg aborts
the allocation, the object is freed via __slab_free(). Calling
__slab_free() in unknown context is not allowed and can lead to a
deadlock.

Free the object via defer_free() when spinning is not allowed.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260610-slab_alloc_flags-v2-0-7190909db118%40kernel.org?part=9
Fixes: af92793e52c3 ("slab: Introduce kmalloc_nolock() and kfree_nolock().")
Signed-off-by: Harry Yoo (Meta) <harry@kernel.org>
Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
---
 mm/slub.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index 48415f24e115..6dfa8ca3c937 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -2533,7 +2533,8 @@ static inline void slab_obj_ext_has_codetag_init(void)
 
 #ifdef CONFIG_MEMCG
 
-static void memcg_alloc_abort_single(struct kmem_cache *s, void *object);
+static void memcg_alloc_abort_single(struct kmem_cache *s, void *object,
+				     bool allow_spin);
 
 static __fastpath_inline
 bool memcg_slab_post_alloc_hook(struct kmem_cache *s, gfp_t flags,
@@ -2551,7 +2552,9 @@ bool memcg_slab_post_alloc_hook(struct kmem_cache *s, gfp_t flags,
 		return true;
 
 	if (likely(size == 1)) {
-		memcg_alloc_abort_single(s, *p);
+		bool allow_spin = alloc_flags_allow_spinning(ac->alloc_flags);
+
+		memcg_alloc_abort_single(s, *p, allow_spin);
 		*p = NULL;
 	} else {
 		kmem_cache_free_bulk(s, size, p);
@@ -6663,16 +6666,20 @@ void slab_free(struct kmem_cache *s, struct slab *slab, void *object,
 #ifdef CONFIG_MEMCG
 /* Do not inline the rare memcg charging failed path into the allocation path */
 static noinline
-void memcg_alloc_abort_single(struct kmem_cache *s, void *object)
+void memcg_alloc_abort_single(struct kmem_cache *s, void *object, bool allow_spin)
 {
 	struct slab *slab = virt_to_slab(object);
 	bool init = slab_want_init_on_free(s);
-	bool allow_spin = true;
 
 	alloc_tagging_slab_free_hook(s, slab, &object, 1);
 
-	if (likely(slab_free_hook(s, object, init, false, allow_spin)))
+	if (unlikely(!slab_free_hook(s, object, init, false, allow_spin)))
+		return;
+
+	if (likely(allow_spin))
 		__slab_free(s, slab, object, object, 1, _RET_IP_);
+	else
+		defer_free(s, object);
 }
 #endif
 

-- 
2.54.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH for-next-fixes v2 5/5] mm/slab: serialize deferred_work_barrier()
  2026-11-03 14:47 [PATCH for-next-fixes v2 0/5] memcg,slab: kmalloc_nolock() fixes Harry Yoo (Meta)
                   ` (3 preceding siblings ...)
  2026-11-03 14:47 ` [PATCH for-next-fixes v2 4/5] mm/slab: fix a deadlock in memcg_alloc_abort_single() Harry Yoo (Meta)
@ 2026-11-03 14:47 ` Harry Yoo (Meta)
  4 siblings, 0 replies; 9+ messages in thread
From: Harry Yoo (Meta) @ 2026-11-03 14:47 UTC (permalink / raw)
  To: Johannes Weiner, Michal Hocko, Roman Gushchin, Shakeel Butt,
	Muchun Song, Andrew Morton, Harry Yoo, Alexei Starovoitov,
	Vlastimil Babka, Hao Li, Christoph Lameter, David Rientjes,
	Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt
  Cc: kernel-team, cgroups, linux-mm, linux-kernel, linux-rt-devel, Sashiko

irq_work_sync() uses rcuwait instead of busy waiting in two cases:

  1. The kernel is using PREEMPT_RT and the irq work does not run in a
     hardirq context.

  2. The architecture cannot send inter-processor interrupts to make
     busy waiting reasonably short.

However, rcuwait.h says:
> The caller is responsible for locking around rcuwait_wait_event(),
> and [prepare_to/finish]_rcuwait() such that writes to @task are
> properly serialized.

Since deferred_work_barrier() calls irq_work_sync() without any locks,
it can potentially cause a hang as writes to @task are not serialized.

Fix this by serializing deferred_work_barrier() with a dedicated mutex.
We cannot reuse slab_mutex because e.g.) some users of
kvfree_rcu_barrier() are outside mm/.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260615-kfree_rcu_nolock-v3-0-70a54f3775bb%40kernel.org?part=5
Fixes: af92793e52c3 ("slab: Introduce kmalloc_nolock() and kfree_nolock().")
Signed-off-by: Harry Yoo (Meta) <harry@kernel.org>
---
 mm/slub.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/mm/slub.c b/mm/slub.c
index 6dfa8ca3c937..50e88edcef3a 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -6636,8 +6636,12 @@ void defer_kfree_rcu(struct kvfree_rcu_head *head)
 /* Must be called before flush_rcu_sheaves_on_cache() */
 void deferred_work_barrier(void)
 {
+	static DEFINE_MUTEX(barrier_mutex);
 	int cpu;
 
+	/* irq_work_sync() may use rcuwait that requires serialization */
+	guard(mutex)(&barrier_mutex);
+
 	for_each_possible_cpu(cpu)
 		irq_work_sync(&per_cpu_ptr(&deferred_percpu_work, cpu)->work);
 }

-- 
2.54.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH for-next-fixes v2 4/5] mm/slab: fix a deadlock in memcg_alloc_abort_single()
  2026-11-03 14:47 ` [PATCH for-next-fixes v2 4/5] mm/slab: fix a deadlock in memcg_alloc_abort_single() Harry Yoo (Meta)
@ 2026-11-03 18:40   ` Alexei Starovoitov
  2026-11-03 20:21     ` Harry Yoo
  0 siblings, 1 reply; 9+ messages in thread
From: Alexei Starovoitov @ 2026-11-03 18:40 UTC (permalink / raw)
  To: Harry Yoo (Meta),
	Johannes Weiner, Michal Hocko, Roman Gushchin, Shakeel Butt,
	Muchun Song, Andrew Morton, Vlastimil Babka, Hao Li,
	Christoph Lameter, David Rientjes, Sebastian Andrzej Siewior,
	Clark Williams, Steven Rostedt
  Cc: kernel-team, cgroups, linux-mm, linux-kernel, linux-rt-devel, Sashiko

On Tue, Nov 03, 2026 at 06:47 AM Harry Yoo (Meta) <harry@kernel.org> wrote:
>  	if (likely(size == 1)) {
> -		memcg_alloc_abort_single(s, *p);
> +		bool allow_spin = alloc_flags_allow_spinning(ac->alloc_flags);
> +
> +		memcg_alloc_abort_single(s, *p, allow_spin);
>  		*p = NULL;
>  	} else {
>  		kmem_cache_free_bulk(s, size, p);

can we call kfree_nolock(*p) here when spinning is not allowed ?
Karl's patch did that. You replied that memcg_slab_free_hook() should
not see an object that is not charged:
https://lore.kernel.org/all/asaAwg1KeZoEYd4K@thinkpad/
It's fine. __memcg_slab_free_hook() skips an object with objcg == NULL.
The else branch relies on that. kmem_cache_free_bulk() also gets
the objects that are not charged yet.

> +	if (likely(allow_spin))
>  		__slab_free(s, slab, object, object, 1, _RET_IP_);
> +	else
> +		defer_free(s, object);

That's an irq_work interrupt per failed charge.
kfree_nolock() tries free_to_pcs() first. It calls defer_free() only
when that fails.

Then no need for allow_spin in memcg_alloc_abort_single(),
and the fix doesn't depend on patch 2.

Something like this (compile tested only):

diff --git a/mm/slub.c b/mm/slub.c
index 45e077c..fb92b48 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -2551,7 +2551,10 @@ bool memcg_slab_post_alloc_hook(struct kmem_cache *s, gfp_t flags,
 		return true;

 	if (likely(size == 1)) {
-		memcg_alloc_abort_single(s, *p);
+		if (likely(alloc_flags_allow_spinning(ac->alloc_flags)))
+			memcg_alloc_abort_single(s, *p);
+		else
+			kfree_nolock(*p);
 		*p = NULL;
 	} else {
 		kmem_cache_free_bulk(s, size, p);

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH for-next-fixes v2 4/5] mm/slab: fix a deadlock in memcg_alloc_abort_single()
  2026-11-03 18:40   ` Alexei Starovoitov
@ 2026-11-03 20:21     ` Harry Yoo
  2026-11-03 21:11       ` Alexei Starovoitov
  0 siblings, 1 reply; 9+ messages in thread
From: Harry Yoo @ 2026-11-03 20:21 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Johannes Weiner, Michal Hocko, Roman Gushchin, Shakeel Butt,
	Muchun Song, Andrew Morton, Vlastimil Babka, Hao Li,
	Christoph Lameter, David Rientjes, Sebastian Andrzej Siewior,
	Clark Williams, Steven Rostedt, kernel-team, cgroups, linux-mm,
	linux-kernel, linux-rt-devel, Sashiko

On Tue, Nov 03, 2026 at 06:40:09PM +0000, Alexei Starovoitov wrote:
> On Tue, Nov 03, 2026 at 06:47 AM Harry Yoo (Meta) <harry@kernel.org> wrote:
> >  	if (likely(size == 1)) {
> > -		memcg_alloc_abort_single(s, *p);
> > +		bool allow_spin = alloc_flags_allow_spinning(ac->alloc_flags);
> > +
> > +		memcg_alloc_abort_single(s, *p, allow_spin);
> >  		*p = NULL;
> >  	} else {
> >  		kmem_cache_free_bulk(s, size, p);
> 
> can we call kfree_nolock(*p) here when spinning is not allowed ?
> Karl's patch did that. You replied that memcg_slab_free_hook() should
> not see an object that is not charged:
> https://lore.kernel.org/all/asaAwg1KeZoEYd4K@thinkpad/
> It's fine. __memcg_slab_free_hook() skips an object with objcg == NULL.
> The else branch relies on that. kmem_cache_free_bulk() also gets
> the objects that are not charged yet.

While this is technically true, this relies too much on the current
implementation details and makes the code hard to follow.

> > +	if (likely(allow_spin))
> >  		__slab_free(s, slab, object, object, 1, _RET_IP_);
> > +	else
> > +		defer_free(s, object);
> 
> That's an irq_work interrupt per failed charge.

irq_work overhead should not matter by the time memcg fails charging.
the kernel can't reclaim memory in unknown context and allocations
won't succeed anyway once we hit the limit (unless somebody else
reclaims memory for us). We have a bigger problem there.

> kfree_nolock() tries free_to_pcs() first. It calls defer_free() only
> when that fails.
> 
> Then no need for allow_spin in memcg_alloc_abort_single(),
> and the fix doesn't depend on patch 2.

Patch 2 is an independent bugfix as-is already. We should really stop
open-coding slab_free_hook(). That ended up missing init_on_free handing.
 
> Something like this (compile tested only):
> 
> diff --git a/mm/slub.c b/mm/slub.c
> index 45e077c..fb92b48 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -2551,7 +2551,10 @@ bool memcg_slab_post_alloc_hook(struct kmem_cache *s, gfp_t flags,
>  		return true;
> 
>  	if (likely(size == 1)) {
> -		memcg_alloc_abort_single(s, *p);
> +		if (likely(alloc_flags_allow_spinning(ac->alloc_flags)))
> +			memcg_alloc_abort_single(s, *p);
> +		else
> +			kfree_nolock(*p);
>  		*p = NULL;
>  	} else {
>  		kmem_cache_free_bulk(s, size, p);

-- 
Cheers,
Harry / Hyeonggon

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH for-next-fixes v2 4/5] mm/slab: fix a deadlock in memcg_alloc_abort_single()
  2026-11-03 20:21     ` Harry Yoo
@ 2026-11-03 21:11       ` Alexei Starovoitov
  0 siblings, 0 replies; 9+ messages in thread
From: Alexei Starovoitov @ 2026-11-03 21:11 UTC (permalink / raw)
  To: Harry Yoo
  Cc: Johannes Weiner, Michal Hocko, Roman Gushchin, Shakeel Butt,
	Muchun Song, Andrew Morton, Vlastimil Babka, Hao Li,
	Christoph Lameter, David Rientjes, Sebastian Andrzej Siewior,
	Clark Williams, Steven Rostedt, kernel-team, cgroups, linux-mm,
	linux-kernel, linux-rt-devel, Sashiko

On Tue, Nov 03, 2026 at 12:21 PM Harry Yoo <harry@kernel.org> wrote:
> While this is technically true, this relies too much on the current
> implementation details and makes the code hard to follow.

Fair enough. I don't insist.

> Patch 2 is an independent bugfix as-is already. We should really stop
> open-coding slab_free_hook(). That ended up missing init_on_free handing.

Right. No objection to patch 2.
With patches 2 and 3 the !allow_spin side of memcg_alloc_abort_single()
is an open coded kfree_nolock():

  kfree_nolock()                    memcg_alloc_abort_single(!allow_spin)
  --------------------------------  -------------------------------------
  memcg_slab_free_hook()            -
  alloc_tagging_slab_free_hook()    alloc_tagging_slab_free_hook()
  slab_free_hook(!allow_spin)       slab_free_hook(!allow_spin)
  free_to_pcs()                     -
  defer_free()                      defer_free()

I don't insist on kfree_nolock() there,
but can we try free_to_pcs() before defer_free() ?
Like:

	if (likely(allow_spin))
		__slab_free(s, slab, object, object, 1, _RET_IP_);
	else if (!can_free_to_pcs(slab) || !free_to_pcs(s, object, false))
		defer_free(s, object);

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-11-03 21:11 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-03 14:47 [PATCH for-next-fixes v2 0/5] memcg,slab: kmalloc_nolock() fixes Harry Yoo (Meta)
2026-11-03 14:47 ` [PATCH for-next-fixes v2 1/5] mm/memcontrol: defer obj_cgroup_release() to irq_work Harry Yoo (Meta)
2026-11-03 14:47 ` [PATCH for-next-fixes v2 2/5] mm/slab: handle allow_spin in slab_free_hook() instead of open coding Harry Yoo (Meta)
2026-11-03 14:47 ` [PATCH for-next-fixes v2 3/5] mm/slab: call debug_locks and KCSAN checks in kfree_nolock() Harry Yoo (Meta)
2026-11-03 14:47 ` [PATCH for-next-fixes v2 4/5] mm/slab: fix a deadlock in memcg_alloc_abort_single() Harry Yoo (Meta)
2026-11-03 18:40   ` Alexei Starovoitov
2026-11-03 20:21     ` Harry Yoo
2026-11-03 21:11       ` Alexei Starovoitov
2026-11-03 14:47 ` [PATCH for-next-fixes v2 5/5] mm/slab: serialize deferred_work_barrier() Harry Yoo (Meta)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®