[PATCH RFC 0/4] memcg,slab: kmalloc_nolock() fixes

[PATCH RFC 0/4] memcg,slab: kmalloc_nolock() fixes

[Harry Yoo (Oracle)]

Apologies for posting another series during the merge window.
But these are bug fixes and there are other features that need to be
rebased on top, so...

Overview
========

This patchset tries to fix three kmalloc_nolock() bugs.

1. obj_cgroup_put() takes a spinlock in the release path
   when it is holding the last reference.
   (This needs some thoughts from the memcg folks)

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 [2] and 3 [3] were
reported by Sashiko.

To MEMCG folks: obj_cgroup_put() is not safe in unknown context
===============================================================

I tried to fix the bug 1 in the __refill_obj_stock() path in patch 1.

Patch 1 considers correctness aspect only, and performance may
degrade because we have to fall back to per-objcg atomics unless
some else drains it for us.

Ouch, while writing the cover letter, I realized that two paths need
some attention:

  1. __memcg_slab_free_hook() -> obj_cgroup_put() and
  2. current_obj_cgroup() -> current_objcg_update() -> obj_cgroup_put()

An easy solution would be to somehow defer obj_cgroup_put() or
obj_cgroup_release(). I would like to hear thoughts from the memcg folks
on which is the preferred way.

To BPF folks: do we need to backport kmalloc_nolock() support
for architectures without __CMPXCHG_DOUBLE to v6.18?
=============================================================

Originally I intended to fix this as part of this series.
However, the issue reported by Levi Zim [1] was on kernel v6.19,
(Thanks to Vlastimil for mentioning this), and v6.18 does not use
kmalloc_nolock() for BPF local storage.

There are still few users in v6.18, but I can't tell whether it is
necessary to backport it to v6.18 (hopefully not as urgent as other
bugfixes).

Thoughts?

[1] https://lore.kernel.org/linux-mm/9bea1536-534a-4a59-9b5f-92389fb05688@kxxt.dev
[2] https://sashiko.dev/#/patchset/20260610-slab_alloc_flags-v2-0-7190909db118%40kernel.org?part=9
[3] https://sashiko.dev/#/patchset/20260615-kfree_rcu_nolock-v3-0-70a54f3775bb%40kernel.org?part=5

Signed-off-by: Harry Yoo (Oracle) <harry@kernel.org>
---
Harry Yoo (Oracle) (4):
      mm/memcontrol: do not drain objcg stock when spinning is not allowed
      mm/slab: handle allow_spin in slab_free_hook() instead of open coding
      mm/slab: fix a deadlock in memcg_alloc_abort_single()
      mm/slab: serialize defer_free_barrier()

 mm/memcontrol.c  |  34 ++++++++-----
 mm/slab.h        |   3 +-
 mm/slab_common.c |   5 +-
 mm/slub.c        | 148 +++++++++++++++++++++++++++++++------------------------
 4 files changed, 111 insertions(+), 79 deletions(-)
---
base-commit: 892a7864730775c3dbee2a39e9ead4fa8d4256e7
change-id: 20260624-kmalloc-nolock-fixes-c97675328773

Best regards,
-- 
Harry Yoo (Oracle) <harry@kernel.org>

[PATCH RFC 1/4] mm/memcontrol: do not drain objcg stock when spinning is not allowed

[Harry Yoo (Oracle)]

When kmalloc_nolock() drains objcg stock, the stock might be holding
the last reference to the objcg. Since obj_cgroup_release() is a
callback for percpu refcount and does not know whether spinning is
allowed, it is not safe to invoke obj_cgroup_put().

This was caught by lockdep on PREEMPT_RT because acquiring
a sleeping lock (objcg_lock) violates lock nesting rules:

  kernel: BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:48
  kernel: in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 1267, name: systemd-resolve
  preempt_count: 1, expected: 0
  RCU nest depth: 3, expected: 3
  6 locks held by systemd-resolve/1267:
   #0: ffff888a8165fa20 ((&pcs->lock)){+.+.}-{3:3}, at: kmem_cache_alloc_noprof+0x185/0xa20
   #1: ffffffff9658a4c0 (rcu_read_lock){....}-{1:3}, at: rt_spin_trylock+0x74/0x2a0
   #2: ffff888a81648598 ((lock)#4){+.+.}-{3:3}, at: trylock_stock+0x118/0x380
   #3: ffffffff9658a4c0 (rcu_read_lock){....}-{1:3}, at: rt_spin_trylock+0x74/0x2a0
   #4: ffffffff9658a4c0 (rcu_read_lock){....}-{1:3}, at: percpu_ref_put_many.constprop.0+0x40/0x270
   #5: ffffffff96af11d8 (objcg_lock){+.+.}-{3:3}, at: obj_cgroup_release+0x8a/0x410
  [...]
  Call Trace:
   <TASK>
   dump_stack_lvl+0x8a/0xe0
   dump_stack+0x14/0x1c
   __might_resched.cold+0x233/0x2bb
   rt_spin_lock+0xd3/0x410
   obj_cgroup_release+0x8a/0x410
   percpu_ref_put_many.constprop.0+0x226/0x270
   drain_obj_stock_slot+0x27e/0x8d0
   __refill_obj_stock+0x409/0x6d0
   __memcg_slab_post_alloc_hook+0xa45/0x1500
   __kmalloc_nolock_noprof+0x988/0xc40
   [...]

However, this is illegal in !RT kernels too because the objcg release
callback acquires a spinlock even when spinning is not allowed.

To fix this issue, fall back to atomics when the cached objcg doesn't
match, but it is unsafe to drain because spinning is not allowed.

This is expected to affect performance of kmalloc_nolock() since
it can no longer drain and refill the stock and falls back to a
per-objcg atomic counter (objcg->nr_charged_bytes).

Fixes: af92793e52c3 ("slab: Introduce kmalloc_nolock() and kfree_nolock().")
Cc: stable@vger.kernel.org
Signed-off-by: Harry Yoo (Oracle) <harry@kernel.org>
---
 mm/memcontrol.c | 34 +++++++++++++++++++++++-----------
 mm/slab.h       |  3 ++-
 mm/slub.c       | 29 +++++++++++++++++++----------
 3 files changed, 44 insertions(+), 22 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 29390ba13baa..5bb5e75ef5b0 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -3316,18 +3316,19 @@ static bool obj_stock_flush_required(struct obj_stock_pcp *stock,
 static void __refill_obj_stock(struct obj_cgroup *objcg,
 			       struct obj_stock_pcp *stock,
 			       unsigned int nr_bytes,
-			       bool allow_uncharge)
+			       bool allow_uncharge,
+			       bool allow_spin)
 {
 	unsigned int nr_pages = 0;
 
-	if (!stock) {
-		nr_pages = nr_bytes >> PAGE_SHIFT;
-		nr_bytes = nr_bytes & (PAGE_SIZE - 1);
-		atomic_add(nr_bytes, &objcg->nr_charged_bytes);
-		goto out;
-	}
+	if (!stock)
+		goto fallback;
 
 	if (READ_ONCE(stock->cached_objcg) != objcg) { /* reset if necessary */
+		/* Not safe to drain since objcg release acquires spinlock */
+		if (unlikely(!allow_spin))
+			goto fallback;
+
 		drain_obj_stock(stock);
 		obj_cgroup_get(objcg);
 		stock->nr_bytes = atomic_read(&objcg->nr_charged_bytes)
@@ -3346,6 +3347,13 @@ static void __refill_obj_stock(struct obj_cgroup *objcg,
 out:
 	if (nr_pages)
 		obj_cgroup_uncharge_pages(objcg, nr_pages);
+	return;
+
+fallback:
+	nr_pages = nr_bytes >> PAGE_SHIFT;
+	nr_bytes = nr_bytes & (PAGE_SIZE - 1);
+	atomic_add(nr_bytes, &objcg->nr_charged_bytes);
+	goto out;
 }
 
 static void refill_obj_stock(struct obj_cgroup *objcg,
@@ -3353,7 +3361,8 @@ static void refill_obj_stock(struct obj_cgroup *objcg,
 			     bool allow_uncharge)
 {
 	struct obj_stock_pcp *stock = trylock_stock();
-	__refill_obj_stock(objcg, stock, nr_bytes, allow_uncharge);
+	__refill_obj_stock(objcg, stock, nr_bytes, allow_uncharge,
+			   /* allow_spin = */ true);
 	unlock_stock(stock);
 }
 
@@ -3428,6 +3437,7 @@ bool __memcg_slab_post_alloc_hook(struct kmem_cache *s, struct list_lru *lru,
 				  size_t size, void **p)
 {
 	size_t obj_size = obj_full_size(s);
+	bool allow_spin = alloc_flags_allow_spinning(slab_alloc_flags);
 	struct obj_cgroup *objcg;
 	struct slab *slab;
 	unsigned long off;
@@ -3497,7 +3507,8 @@ bool __memcg_slab_post_alloc_hook(struct kmem_cache *s, struct list_lru *lru,
 				return false;
 			stock = trylock_stock();
 			if (remainder)
-				__refill_obj_stock(objcg, stock, remainder, false);
+				__refill_obj_stock(objcg, stock, remainder, false,
+						   allow_spin);
 		}
 		__account_obj_stock(objcg, stock, obj_size,
 				    slab_pgdat(slab), cache_vmstat_idx(s));
@@ -3516,7 +3527,8 @@ bool __memcg_slab_post_alloc_hook(struct kmem_cache *s, struct list_lru *lru,
 }
 
 void __memcg_slab_free_hook(struct kmem_cache *s, struct slab *slab,
-			    void **p, int objects, unsigned long obj_exts)
+			    void **p, int objects, unsigned long obj_exts,
+			    bool allow_spin)
 {
 	size_t obj_size = obj_full_size(s);
 
@@ -3535,7 +3547,7 @@ void __memcg_slab_free_hook(struct kmem_cache *s, struct slab *slab,
 		obj_ext->objcg = NULL;
 
 		stock = trylock_stock();
-		__refill_obj_stock(objcg, stock, obj_size, true);
+		__refill_obj_stock(objcg, stock, obj_size, true, allow_spin);
 		__account_obj_stock(objcg, stock, -obj_size,
 				    slab_pgdat(slab), cache_vmstat_idx(s));
 		unlock_stock(stock);
diff --git a/mm/slab.h b/mm/slab.h
index 281a65233795..a6b4ac298d08 100644
--- a/mm/slab.h
+++ b/mm/slab.h
@@ -660,7 +660,8 @@ bool __memcg_slab_post_alloc_hook(struct kmem_cache *s, struct list_lru *lru,
 				  gfp_t flags, unsigned int slab_alloc_flags,
 				  size_t size, void **p);
 void __memcg_slab_free_hook(struct kmem_cache *s, struct slab *slab,
-			    void **p, int objects, unsigned long obj_exts);
+			    void **p, int objects, unsigned long obj_exts,
+			    bool allow_spin);
 #endif
 
 void kvfree_rcu_cb(struct rcu_head *head);
diff --git a/mm/slub.c b/mm/slub.c
index 917635203f73..32672a92581b 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -2488,7 +2488,7 @@ bool memcg_slab_post_alloc_hook(struct kmem_cache *s, gfp_t flags,
 
 static __fastpath_inline
 void memcg_slab_free_hook(struct kmem_cache *s, struct slab *slab, void **p,
-			  int objects)
+			  int objects, bool allow_spin)
 {
 	unsigned long obj_exts;
 
@@ -2500,7 +2500,7 @@ void memcg_slab_free_hook(struct kmem_cache *s, struct slab *slab, void **p,
 		return;
 
 	get_slab_obj_exts(obj_exts);
-	__memcg_slab_free_hook(s, slab, p, objects, obj_exts);
+	__memcg_slab_free_hook(s, slab, p, objects, obj_exts, allow_spin);
 	put_slab_obj_exts(obj_exts);
 }
 
@@ -2575,7 +2575,7 @@ static inline bool memcg_slab_post_alloc_hook(struct kmem_cache *s,
 }
 
 static inline void memcg_slab_free_hook(struct kmem_cache *s, struct slab *slab,
-					void **p, int objects)
+					void **p, int objects, bool allow_spin)
 {
 }
 
@@ -2946,11 +2946,12 @@ 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]);
 
-		memcg_slab_free_hook(s, slab, p + i, 1);
+		memcg_slab_free_hook(s, slab, p + i, 1, allow_spin);
 		alloc_tagging_slab_free_hook(s, slab, p + i, 1);
 
 		if (unlikely(!slab_free_hook(s, p[i], init, true))) {
@@ -6215,12 +6216,13 @@ static void free_to_pcs_bulk(struct kmem_cache *s, size_t size, void **p)
 	struct node_barn *barn;
 	void *remote_objects[PCS_BATCH_MAX];
 	unsigned int remote_nr = 0;
+	bool allow_spin = true;
 
 next_remote_batch:
 	while (i < size) {
 		struct slab *slab = virt_to_slab(p[i]);
 
-		memcg_slab_free_hook(s, slab, p + i, 1);
+		memcg_slab_free_hook(s, slab, p + i, 1, allow_spin);
 		alloc_tagging_slab_free_hook(s, slab, p + i, 1);
 
 		if (unlikely(!slab_free_hook(s, p[i], init, false))) {
@@ -6398,13 +6400,16 @@ static __fastpath_inline
 void slab_free(struct kmem_cache *s, struct slab *slab, void *object,
 	       unsigned long addr)
 {
-	memcg_slab_free_hook(s, slab, &object, 1);
+	bool allow_spin = true;
+
+	memcg_slab_free_hook(s, slab, &object, 1, allow_spin);
 	alloc_tagging_slab_free_hook(s, slab, &object, 1);
 
 	if (unlikely(!slab_free_hook(s, object, slab_want_init_on_free(s), false)))
 		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);
@@ -6429,7 +6434,9 @@ static __fastpath_inline
 void slab_free_bulk(struct kmem_cache *s, struct slab *slab, void *head,
 		    void *tail, void **p, int cnt, unsigned long addr)
 {
-	memcg_slab_free_hook(s, slab, p, cnt);
+	bool allow_spin = true;
+
+	memcg_slab_free_hook(s, slab, p, cnt, allow_spin);
 	alloc_tagging_slab_free_hook(s, slab, p, cnt);
 	/*
 	 * With KASAN enabled slab_free_freelist_hook modifies the freelist
@@ -6734,6 +6741,7 @@ void kfree_nolock(const void *object)
 	struct slab *slab;
 	struct kmem_cache *s;
 	void *x = (void *)object;
+	bool allow_spin = false;
 
 	if (unlikely(ZERO_OR_NULL_PTR(object)))
 		return;
@@ -6746,7 +6754,7 @@ void kfree_nolock(const void *object)
 
 	s = slab->slab_cache;
 
-	memcg_slab_free_hook(s, slab, &x, 1);
+	memcg_slab_free_hook(s, slab, &x, 1, allow_spin);
 	alloc_tagging_slab_free_hook(s, slab, &x, 1);
 	/*
 	 * Unlike slab_free() do NOT call the following:
@@ -6776,7 +6784,8 @@ void kfree_nolock(const void *object)
 	 */
 	kasan_slab_free(s, x, false, false, /* skip quarantine */true);
 
-	if (likely(can_free_to_pcs(slab)) && likely(free_to_pcs(s, x, false)))
+	if (likely(can_free_to_pcs(slab)) &&
+			likely(free_to_pcs(s, x, allow_spin)))
 		return;
 
 	/*

-- 
2.53.0

[PATCH RFC 2/4] mm/slab: handle allow_spin in slab_free_hook() instead of open coding

[Harry Yoo (Oracle)]

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.

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.

Fixes: af92793e52c3 ("slab: Introduce kmalloc_nolock() and kfree_nolock().")
Cc: stable@vger.kernel.org
Signed-off-by: Harry Yoo (Oracle) <harry@kernel.org>
---
 mm/slub.c | 100 +++++++++++++++++++++++++++++++-------------------------------
 1 file changed, 50 insertions(+), 50 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index 32672a92581b..85760c8ff2e2 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -2611,41 +2611,52 @@ 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, and thus safe to call
+ * when spinning is allowed:
  * - 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);
-
-	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;
+	if (likely(allow_spin)) {
+		/* These hooks acquire spinlocks */
+		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;
+	}
 
 	/*
 	 * 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;
@@ -2654,6 +2665,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) {
 			/*
@@ -2701,8 +2713,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
@@ -2714,9 +2729,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;
 	}
 
@@ -2731,7 +2747,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;
@@ -2954,7 +2970,7 @@ static bool __rcu_free_sheaf_prepare(struct kmem_cache *s,
 		memcg_slab_free_hook(s, slab, p + i, 1, allow_spin);
 		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;
 		}
@@ -6225,7 +6241,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, allow_spin);
 		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;
 		}
@@ -6401,11 +6417,12 @@ 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, allow_spin);
 	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)) &&
@@ -6422,10 +6439,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
@@ -6456,6 +6475,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);
 
@@ -6469,8 +6490,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);
 	}
@@ -6742,6 +6764,7 @@ void kfree_nolock(const void *object)
 	struct kmem_cache *s;
 	void *x = (void *)object;
 	bool allow_spin = false;
+	bool init;
 
 	if (unlikely(ZERO_OR_NULL_PTR(object)))
 		return;
@@ -6756,33 +6779,10 @@ void kfree_nolock(const void *object)
 
 	memcg_slab_free_hook(s, slab, &x, 1, allow_spin);
 	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 (!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, allow_spin)))

-- 
2.53.0

[PATCH RFC 3/4] mm/slab: fix a deadlock in memcg_alloc_abort_single()

[Harry Yoo (Oracle)]

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().")
Cc: stable@vger.kernel.org
Signed-off-by: Harry Yoo (Oracle) <harry@kernel.org>
---
 mm/slub.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index 85760c8ff2e2..4a3618e3967e 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -2459,7 +2459,8 @@ alloc_tagging_slab_free_hook(struct kmem_cache *s, struct slab *slab, void **p,
 
 #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,
@@ -2477,7 +2478,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);
@@ -6436,16 +6439,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.53.0

[PATCH RFC 4/4] mm/slab: serialize defer_free_barrier()

[Harry Yoo (Oracle)]

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 defer_free_barrier() calls irq_work_sync() without any locks,
it can potentially cause a hang as writes to @task are not serialized.

Fix this by calling defer_free_barrier() under slab_mutex and
cpus_read_lock() and add lockdep asserts.

Now that defer_free_barrier() is called inside cpus_read_lock(), iterate
over online cpus instead of possible cpus.

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().")
Cc: stable@vger.kernel.org
Signed-off-by: Harry Yoo (Oracle) <harry@kernel.org>
---
 mm/slab_common.c | 5 ++---
 mm/slub.c        | 6 +++++-
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/mm/slab_common.c b/mm/slab_common.c
index 388eb5980859..27f77273fabe 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -550,11 +550,10 @@ void kmem_cache_destroy(struct kmem_cache *s)
 		rcu_barrier();
 	}
 
-	/* Wait for deferred work from kmalloc/kfree_nolock() */
-	defer_free_barrier();
-
 	cpus_read_lock();
 	mutex_lock(&slab_mutex);
+	/* Wait for deferred work from kmalloc/kfree_nolock() */
+	defer_free_barrier();
 
 	s->refcount--;
 	if (s->refcount) {
diff --git a/mm/slub.c b/mm/slub.c
index 4a3618e3967e..52c8d3f33782 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -6411,7 +6411,11 @@ void defer_free_barrier(void)
 {
 	int cpu;
 
-	for_each_possible_cpu(cpu)
+	/* irq_work_sync() may use rcuwait that requires serialization */
+	lockdep_assert_held(&slab_mutex);
+	lockdep_assert_cpus_held();
+
+	for_each_online_cpu(cpu)
 		irq_work_sync(&per_cpu_ptr(&defer_free_objects, cpu)->work);
 }
 

-- 
2.53.0

Re: [PATCH RFC 0/4] memcg,slab: kmalloc_nolock() fixes

[Alexei Starovoitov]

On Wed Jun 24, 2026 at 6:11 AM PDT, Harry Yoo (Oracle) wrote:
>
> Bug 1 was reported by lockdep, and bugs 2 [2] and 3 [3] were
> reported by Sashiko.

... and in fixes for sashiko complains sashiko finds more issues.
I don't think it will ever end. I suggest to fix realistic scenarios
instead of one out of billion cases that sashiko think is plausible
but will never be hit in reality. The chance of server crashing
due to cosmic rays are higher than such bugs. Hence do not fix them.

> To BPF folks: do we need to backport kmalloc_nolock() support
> for architectures without __CMPXCHG_DOUBLE to v6.18?

nope.

> There are still few users in v6.18, but I can't tell whether it is
> necessary to backport it to v6.18 (hopefully not as urgent as other
> bugfixes).

imo none of these 'fixes' are necessary. Humans are not hitting them.

Re: [PATCH RFC 0/4] memcg,slab: kmalloc_nolock() fixes

[Harry Yoo]

[-- Attachment #1.1: Type: text/plain, Size: 2532 bytes --]



On 6/25/26 1:30 AM, Alexei Starovoitov wrote:
> On Wed Jun 24, 2026 at 6:11 AM PDT, Harry Yoo (Oracle) wrote:
>>
>> Bug 1 was reported by lockdep, and bugs 2 [2] and 3 [3] were
>> reported by Sashiko.
> 
> ... and in fixes for sashiko complains sashiko finds more issues.
> I don't think it will ever end. I suggest to fix realistic scenarios
> instead of one out of billion cases that sashiko think is plausible
> but will never be hit in reality.

But we can trigger debug warnings for the first two bugs fairly
easily with slub_kunit. Doesn't that count as realistic scenarios?

(Ok, I admit that the last bug was purely theoretical, and would not
 have bothered if the fix was not straightforward)

You might argue that it's not as urgent as we might assume
(e.g., it's okay to not fix them asap or backport), but I don't think
we can just ignore them.

It might be bit harder to cause an actual deadlock than to
trigger a debug warning, though. We can discuss that [1] [2].

> The chance of server crashing
> due to cosmic rays are higher than such bugs.

I'm not convinced that it's the case.

Well, I don't know what are the chances of calling kmalloc_nolock()
in NMI, or within slab or memcg (via tracing), and that is an important
factor here.

>> To BPF folks: do we need to backport kmalloc_nolock() support
>> for architectures without __CMPXCHG_DOUBLE to v6.18?
> 
> nope.

Thanks, that was what I was hoping :)

# The discussion

[1] Bug 1: freeing a slab object via kfree_nolock() or draining
the stock in kmalloc_nolock() happens very frequently. The objcg should
have been reparented (which happens upon cgroup removal, which is not
too rare) at some point if the objcg stock or a slab object is holding
the last reference.

Can this cause an actual deadlock? That depends on the chances of
calling kmalloc/kfree_nolock() in the middle of reparenting (see
reparent_[un]locks()) or objcg list manipulation under objcg_lock.

[2] Bug 2: You should exceed memcg limit to invoke
memcg_alloc_abort_single(), but you don't even have to be under
memory pressure to exceed that. (yeah, I had to modify the
kernel to implement a fault-injection-like-feature to trigger this).
Unfortunately, you cannot reclaim memory in unknown context when you
hit the limit. This should be fairly easy to trigger.

Can this cause an actual deadlock? That depends on the chances
of calling kmalloc/kfree_nolock() within the slab allocator.

-- 
Cheers,
Harry / Hyeonggon

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH RFC 0/4] memcg,slab: kmalloc_nolock() fixes

[Harry Yoo]

[-- Attachment #1.1: Type: text/plain, Size: 526 bytes --]



On 6/24/26 10:11 PM, Harry Yoo (Oracle) wrote:
> Apologies for posting another series during the merge window.
> But these are bug fixes and there are other features that need to be
> rebased on top, so...

Okay, given that this needs a bit of discussion with the memcg folks and
this has been there for a year and nobody noticed, I'd suggest we be bit
less hurried and not delay other new features going on because of this.

But I still think they should eventually be fixed.

-- 
Cheers,
Harry / Hyeonggon

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH RFC 1/4] mm/memcontrol: do not drain objcg stock when spinning is not allowed

[Vlastimil Babka (SUSE)]

On 6/24/26 15:11, Harry Yoo (Oracle) wrote:
> When kmalloc_nolock() drains objcg stock, the stock might be holding
> the last reference to the objcg. Since obj_cgroup_release() is a
> callback for percpu refcount and does not know whether spinning is
> allowed, it is not safe to invoke obj_cgroup_put().
> 
> This was caught by lockdep on PREEMPT_RT because acquiring
> a sleeping lock (objcg_lock) violates lock nesting rules:
> 
>   kernel: BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:48
>   kernel: in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 1267, name: systemd-resolve
>   preempt_count: 1, expected: 0
>   RCU nest depth: 3, expected: 3
>   6 locks held by systemd-resolve/1267:
>    #0: ffff888a8165fa20 ((&pcs->lock)){+.+.}-{3:3}, at: kmem_cache_alloc_noprof+0x185/0xa20
>    #1: ffffffff9658a4c0 (rcu_read_lock){....}-{1:3}, at: rt_spin_trylock+0x74/0x2a0
>    #2: ffff888a81648598 ((lock)#4){+.+.}-{3:3}, at: trylock_stock+0x118/0x380
>    #3: ffffffff9658a4c0 (rcu_read_lock){....}-{1:3}, at: rt_spin_trylock+0x74/0x2a0
>    #4: ffffffff9658a4c0 (rcu_read_lock){....}-{1:3}, at: percpu_ref_put_many.constprop.0+0x40/0x270
>    #5: ffffffff96af11d8 (objcg_lock){+.+.}-{3:3}, at: obj_cgroup_release+0x8a/0x410
>   [...]
>   Call Trace:
>    <TASK>
>    dump_stack_lvl+0x8a/0xe0
>    dump_stack+0x14/0x1c
>    __might_resched.cold+0x233/0x2bb
>    rt_spin_lock+0xd3/0x410
>    obj_cgroup_release+0x8a/0x410
>    percpu_ref_put_many.constprop.0+0x226/0x270
>    drain_obj_stock_slot+0x27e/0x8d0
>    __refill_obj_stock+0x409/0x6d0
>    __memcg_slab_post_alloc_hook+0xa45/0x1500
>    __kmalloc_nolock_noprof+0x988/0xc40
>    [...]
> 
> However, this is illegal in !RT kernels too because the objcg release
> callback acquires a spinlock even when spinning is not allowed.
> 
> To fix this issue, fall back to atomics when the cached objcg doesn't
> match, but it is unsafe to drain because spinning is not allowed.
> 
> This is expected to affect performance of kmalloc_nolock() since
> it can no longer drain and refill the stock and falls back to a
> per-objcg atomic counter (objcg->nr_charged_bytes).

Maybe it's not bad that the constrained operation will not do a drain? Could
we say it's likely that both _nolock() and normal operations will likely be
happening on the same cpu for the same memcg, so a normal one will soon
handle it?

> Fixes: af92793e52c3 ("slab: Introduce kmalloc_nolock() and kfree_nolock().")
> Cc: stable@vger.kernel.org
> Signed-off-by: Harry Yoo (Oracle) <harry@kernel.org>
> ---
>  mm/memcontrol.c | 34 +++++++++++++++++++++++-----------
>  mm/slab.h       |  3 ++-
>  mm/slub.c       | 29 +++++++++++++++++++----------
>  3 files changed, 44 insertions(+), 22 deletions(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 29390ba13baa..5bb5e75ef5b0 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -3316,18 +3316,19 @@ static bool obj_stock_flush_required(struct obj_stock_pcp *stock,
>  static void __refill_obj_stock(struct obj_cgroup *objcg,
>  			       struct obj_stock_pcp *stock,
>  			       unsigned int nr_bytes,
> -			       bool allow_uncharge)
> +			       bool allow_uncharge,
> +			       bool allow_spin)
>  {
>  	unsigned int nr_pages = 0;
>  
> -	if (!stock) {
> -		nr_pages = nr_bytes >> PAGE_SHIFT;
> -		nr_bytes = nr_bytes & (PAGE_SIZE - 1);
> -		atomic_add(nr_bytes, &objcg->nr_charged_bytes);
> -		goto out;
> -	}
> +	if (!stock)
> +		goto fallback;
>  
>  	if (READ_ONCE(stock->cached_objcg) != objcg) { /* reset if necessary */
> +		/* Not safe to drain since objcg release acquires spinlock */
> +		if (unlikely(!allow_spin))
> +			goto fallback;
> +
>  		drain_obj_stock(stock);
>  		obj_cgroup_get(objcg);
>  		stock->nr_bytes = atomic_read(&objcg->nr_charged_bytes)
> @@ -3346,6 +3347,13 @@ static void __refill_obj_stock(struct obj_cgroup *objcg,
>  out:
>  	if (nr_pages)
>  		obj_cgroup_uncharge_pages(objcg, nr_pages);
> +	return;
> +
> +fallback:
> +	nr_pages = nr_bytes >> PAGE_SHIFT;
> +	nr_bytes = nr_bytes & (PAGE_SIZE - 1);
> +	atomic_add(nr_bytes, &objcg->nr_charged_bytes);
> +	goto out;
>  }
>  
>  static void refill_obj_stock(struct obj_cgroup *objcg,
> @@ -3353,7 +3361,8 @@ static void refill_obj_stock(struct obj_cgroup *objcg,
>  			     bool allow_uncharge)
>  {
>  	struct obj_stock_pcp *stock = trylock_stock();
> -	__refill_obj_stock(objcg, stock, nr_bytes, allow_uncharge);
> +	__refill_obj_stock(objcg, stock, nr_bytes, allow_uncharge,
> +			   /* allow_spin = */ true);
>  	unlock_stock(stock);
>  }
>  
> @@ -3428,6 +3437,7 @@ bool __memcg_slab_post_alloc_hook(struct kmem_cache *s, struct list_lru *lru,
>  				  size_t size, void **p)
>  {
>  	size_t obj_size = obj_full_size(s);
> +	bool allow_spin = alloc_flags_allow_spinning(slab_alloc_flags);
>  	struct obj_cgroup *objcg;
>  	struct slab *slab;
>  	unsigned long off;
> @@ -3497,7 +3507,8 @@ bool __memcg_slab_post_alloc_hook(struct kmem_cache *s, struct list_lru *lru,
>  				return false;
>  			stock = trylock_stock();
>  			if (remainder)
> -				__refill_obj_stock(objcg, stock, remainder, false);
> +				__refill_obj_stock(objcg, stock, remainder, false,
> +						   allow_spin);
>  		}
>  		__account_obj_stock(objcg, stock, obj_size,
>  				    slab_pgdat(slab), cache_vmstat_idx(s));
> @@ -3516,7 +3527,8 @@ bool __memcg_slab_post_alloc_hook(struct kmem_cache *s, struct list_lru *lru,
>  }
>  
>  void __memcg_slab_free_hook(struct kmem_cache *s, struct slab *slab,
> -			    void **p, int objects, unsigned long obj_exts)
> +			    void **p, int objects, unsigned long obj_exts,
> +			    bool allow_spin)
>  {
>  	size_t obj_size = obj_full_size(s);
>  
> @@ -3535,7 +3547,7 @@ void __memcg_slab_free_hook(struct kmem_cache *s, struct slab *slab,
>  		obj_ext->objcg = NULL;
>  
>  		stock = trylock_stock();
> -		__refill_obj_stock(objcg, stock, obj_size, true);
> +		__refill_obj_stock(objcg, stock, obj_size, true, allow_spin);
>  		__account_obj_stock(objcg, stock, -obj_size,
>  				    slab_pgdat(slab), cache_vmstat_idx(s));
>  		unlock_stock(stock);
> diff --git a/mm/slab.h b/mm/slab.h
> index 281a65233795..a6b4ac298d08 100644
> --- a/mm/slab.h
> +++ b/mm/slab.h
> @@ -660,7 +660,8 @@ bool __memcg_slab_post_alloc_hook(struct kmem_cache *s, struct list_lru *lru,
>  				  gfp_t flags, unsigned int slab_alloc_flags,
>  				  size_t size, void **p);
>  void __memcg_slab_free_hook(struct kmem_cache *s, struct slab *slab,
> -			    void **p, int objects, unsigned long obj_exts);
> +			    void **p, int objects, unsigned long obj_exts,
> +			    bool allow_spin);
>  #endif
>  
>  void kvfree_rcu_cb(struct rcu_head *head);
> diff --git a/mm/slub.c b/mm/slub.c
> index 917635203f73..32672a92581b 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -2488,7 +2488,7 @@ bool memcg_slab_post_alloc_hook(struct kmem_cache *s, gfp_t flags,
>  
>  static __fastpath_inline
>  void memcg_slab_free_hook(struct kmem_cache *s, struct slab *slab, void **p,
> -			  int objects)
> +			  int objects, bool allow_spin)
>  {
>  	unsigned long obj_exts;
>  
> @@ -2500,7 +2500,7 @@ void memcg_slab_free_hook(struct kmem_cache *s, struct slab *slab, void **p,
>  		return;
>  
>  	get_slab_obj_exts(obj_exts);
> -	__memcg_slab_free_hook(s, slab, p, objects, obj_exts);
> +	__memcg_slab_free_hook(s, slab, p, objects, obj_exts, allow_spin);
>  	put_slab_obj_exts(obj_exts);
>  }
>  
> @@ -2575,7 +2575,7 @@ static inline bool memcg_slab_post_alloc_hook(struct kmem_cache *s,
>  }
>  
>  static inline void memcg_slab_free_hook(struct kmem_cache *s, struct slab *slab,
> -					void **p, int objects)
> +					void **p, int objects, bool allow_spin)
>  {
>  }
>  
> @@ -2946,11 +2946,12 @@ 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]);
>  
> -		memcg_slab_free_hook(s, slab, p + i, 1);
> +		memcg_slab_free_hook(s, slab, p + i, 1, allow_spin);
>  		alloc_tagging_slab_free_hook(s, slab, p + i, 1);
>  
>  		if (unlikely(!slab_free_hook(s, p[i], init, true))) {
> @@ -6215,12 +6216,13 @@ static void free_to_pcs_bulk(struct kmem_cache *s, size_t size, void **p)
>  	struct node_barn *barn;
>  	void *remote_objects[PCS_BATCH_MAX];
>  	unsigned int remote_nr = 0;
> +	bool allow_spin = true;
>  
>  next_remote_batch:
>  	while (i < size) {
>  		struct slab *slab = virt_to_slab(p[i]);
>  
> -		memcg_slab_free_hook(s, slab, p + i, 1);
> +		memcg_slab_free_hook(s, slab, p + i, 1, allow_spin);
>  		alloc_tagging_slab_free_hook(s, slab, p + i, 1);
>  
>  		if (unlikely(!slab_free_hook(s, p[i], init, false))) {
> @@ -6398,13 +6400,16 @@ static __fastpath_inline
>  void slab_free(struct kmem_cache *s, struct slab *slab, void *object,
>  	       unsigned long addr)
>  {
> -	memcg_slab_free_hook(s, slab, &object, 1);
> +	bool allow_spin = true;
> +
> +	memcg_slab_free_hook(s, slab, &object, 1, allow_spin);
>  	alloc_tagging_slab_free_hook(s, slab, &object, 1);
>  
>  	if (unlikely(!slab_free_hook(s, object, slab_want_init_on_free(s), false)))
>  		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);
> @@ -6429,7 +6434,9 @@ static __fastpath_inline
>  void slab_free_bulk(struct kmem_cache *s, struct slab *slab, void *head,
>  		    void *tail, void **p, int cnt, unsigned long addr)
>  {
> -	memcg_slab_free_hook(s, slab, p, cnt);
> +	bool allow_spin = true;
> +
> +	memcg_slab_free_hook(s, slab, p, cnt, allow_spin);
>  	alloc_tagging_slab_free_hook(s, slab, p, cnt);
>  	/*
>  	 * With KASAN enabled slab_free_freelist_hook modifies the freelist
> @@ -6734,6 +6741,7 @@ void kfree_nolock(const void *object)
>  	struct slab *slab;
>  	struct kmem_cache *s;
>  	void *x = (void *)object;
> +	bool allow_spin = false;
>  
>  	if (unlikely(ZERO_OR_NULL_PTR(object)))
>  		return;
> @@ -6746,7 +6754,7 @@ void kfree_nolock(const void *object)
>  
>  	s = slab->slab_cache;
>  
> -	memcg_slab_free_hook(s, slab, &x, 1);
> +	memcg_slab_free_hook(s, slab, &x, 1, allow_spin);
>  	alloc_tagging_slab_free_hook(s, slab, &x, 1);
>  	/*
>  	 * Unlike slab_free() do NOT call the following:
> @@ -6776,7 +6784,8 @@ void kfree_nolock(const void *object)
>  	 */
>  	kasan_slab_free(s, x, false, false, /* skip quarantine */true);
>  
> -	if (likely(can_free_to_pcs(slab)) && likely(free_to_pcs(s, x, false)))
> +	if (likely(can_free_to_pcs(slab)) &&
> +			likely(free_to_pcs(s, x, allow_spin)))
>  		return;
>  
>  	/*
> 

Re: [PATCH RFC 2/4] mm/slab: handle allow_spin in slab_free_hook() instead of open coding

[Vlastimil Babka (SUSE)]

On 6/24/26 15:11, Harry Yoo (Oracle) wrote:
> 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.
> 
> 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.
> 
> Fixes: af92793e52c3 ("slab: Introduce kmalloc_nolock() and kfree_nolock().")
> Cc: stable@vger.kernel.org
> Signed-off-by: Harry Yoo (Oracle) <harry@kernel.org>
> ---
>  mm/slub.c | 100 +++++++++++++++++++++++++++++++-------------------------------
>  1 file changed, 50 insertions(+), 50 deletions(-)
> 
> diff --git a/mm/slub.c b/mm/slub.c
> index 32672a92581b..85760c8ff2e2 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -2611,41 +2611,52 @@ 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, and thus safe to call
> + * when spinning is allowed:
>   * - 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);
> -
> -	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;
> +	if (likely(allow_spin)) {
> +		/* These hooks acquire spinlocks */
> +		kmemleak_free_recursive(x, s->flags);
> +		debug_check_no_locks_freed(x, s->object_size);

This line shouldn't be here?

> +		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;

The whole block now misses free space lines that it had.

> +	}
>  
>  	/*
>  	 * 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;
> @@ -2654,6 +2665,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) {
>  			/*
> @@ -2701,8 +2713,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
> @@ -2714,9 +2729,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;
>  	}
>  
> @@ -2731,7 +2747,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;
> @@ -2954,7 +2970,7 @@ static bool __rcu_free_sheaf_prepare(struct kmem_cache *s,
>  		memcg_slab_free_hook(s, slab, p + i, 1, allow_spin);
>  		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;
>  		}
> @@ -6225,7 +6241,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, allow_spin);
>  		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;
>  		}
> @@ -6401,11 +6417,12 @@ 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, allow_spin);
>  	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)) &&
> @@ -6422,10 +6439,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
> @@ -6456,6 +6475,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);
>  
> @@ -6469,8 +6490,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);
>  	}
> @@ -6742,6 +6764,7 @@ void kfree_nolock(const void *object)
>  	struct kmem_cache *s;
>  	void *x = (void *)object;
>  	bool allow_spin = false;
> +	bool init;
>  
>  	if (unlikely(ZERO_OR_NULL_PTR(object)))
>  		return;
> @@ -6756,33 +6779,10 @@ void kfree_nolock(const void *object)
>  
>  	memcg_slab_free_hook(s, slab, &x, 1, allow_spin);
>  	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 (!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, allow_spin)))
> 

Re: [PATCH RFC 3/4] mm/slab: fix a deadlock in memcg_alloc_abort_single()

[Vlastimil Babka (SUSE)]

On 6/24/26 15:11, Harry Yoo (Oracle) wrote:
> 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().")
> Cc: stable@vger.kernel.org
> Signed-off-by: Harry Yoo (Oracle) <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 85760c8ff2e2..4a3618e3967e 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -2459,7 +2459,8 @@ alloc_tagging_slab_free_hook(struct kmem_cache *s, struct slab *slab, void **p,
>  
>  #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,
> @@ -2477,7 +2478,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);
> @@ -6436,16 +6439,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
>  
> 

Re: [PATCH RFC 4/4] mm/slab: serialize defer_free_barrier()

[Vlastimil Babka (SUSE)]

On 6/24/26 15:11, Harry Yoo (Oracle) wrote:
> 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 defer_free_barrier() calls irq_work_sync() without any locks,
> it can potentially cause a hang as writes to @task are not serialized.
> 
> Fix this by calling defer_free_barrier() under slab_mutex and
> cpus_read_lock() and add lockdep asserts.
> 
> Now that defer_free_barrier() is called inside cpus_read_lock(), iterate
> over online cpus instead of possible cpus.
> 
> 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().")
> Cc: stable@vger.kernel.org
> Signed-off-by: Harry Yoo (Oracle) <harry@kernel.org>

Acked-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>

> ---
>  mm/slab_common.c | 5 ++---
>  mm/slub.c        | 6 +++++-
>  2 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 388eb5980859..27f77273fabe 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -550,11 +550,10 @@ void kmem_cache_destroy(struct kmem_cache *s)
>  		rcu_barrier();
>  	}
>  
> -	/* Wait for deferred work from kmalloc/kfree_nolock() */
> -	defer_free_barrier();
> -
>  	cpus_read_lock();
>  	mutex_lock(&slab_mutex);
> +	/* Wait for deferred work from kmalloc/kfree_nolock() */
> +	defer_free_barrier();
>  
>  	s->refcount--;
>  	if (s->refcount) {
> diff --git a/mm/slub.c b/mm/slub.c
> index 4a3618e3967e..52c8d3f33782 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -6411,7 +6411,11 @@ void defer_free_barrier(void)
>  {
>  	int cpu;
>  
> -	for_each_possible_cpu(cpu)
> +	/* irq_work_sync() may use rcuwait that requires serialization */
> +	lockdep_assert_held(&slab_mutex);
> +	lockdep_assert_cpus_held();
> +
> +	for_each_online_cpu(cpu)
>  		irq_work_sync(&per_cpu_ptr(&defer_free_objects, cpu)->work);
>  }
>  
> 

Re: [PATCH RFC 0/4] memcg,slab: kmalloc_nolock() fixes

[Vlastimil Babka (SUSE)]

On 6/24/26 22:19, Harry Yoo wrote:
> 
> 
> On 6/25/26 1:30 AM, Alexei Starovoitov wrote:
>> On Wed Jun 24, 2026 at 6:11 AM PDT, Harry Yoo (Oracle) wrote:
>>>
>>> Bug 1 was reported by lockdep, and bugs 2 [2] and 3 [3] were
>>> reported by Sashiko.
>> 
>> ... and in fixes for sashiko complains sashiko finds more issues.
>> I don't think it will ever end. I suggest to fix realistic scenarios
>> instead of one out of billion cases that sashiko think is plausible
>> but will never be hit in reality.
> 
> But we can trigger debug warnings for the first two bugs fairly
> easily with slub_kunit. Doesn't that count as realistic scenarios?
> 
> (Ok, I admit that the last bug was purely theoretical, and would not
>  have bothered if the fix was not straightforward)
> 
> You might argue that it's not as urgent as we might assume
> (e.g., it's okay to not fix them asap or backport), but I don't think
> we can just ignore them.

Agreed, should be fixed, even if rare.

> It might be bit harder to cause an actual deadlock than to
> trigger a debug warning, though. We can discuss that [1] [2].
> 
>> The chance of server crashing
>> due to cosmic rays are higher than such bugs.
> 
> I'm not convinced that it's the case.

Yeah, especially with large fleets this will manifest for some machines, and
as the usage of the API intensifies.

> Well, I don't know what are the chances of calling kmalloc_nolock()
> in NMI, or within slab or memcg (via tracing), and that is an important
> factor here.
> 
>>> To BPF folks: do we need to backport kmalloc_nolock() support
>>> for architectures without __CMPXCHG_DOUBLE to v6.18?
>> 
>> nope.
> 
> Thanks, that was what I was hoping :)
> 
> # The discussion
> 
> [1] Bug 1: freeing a slab object via kfree_nolock() or draining
> the stock in kmalloc_nolock() happens very frequently. The objcg should
> have been reparented (which happens upon cgroup removal, which is not
> too rare) at some point if the objcg stock or a slab object is holding
> the last reference.
> 
> Can this cause an actual deadlock? That depends on the chances of
> calling kmalloc/kfree_nolock() in the middle of reparenting (see
> reparent_[un]locks()) or objcg list manipulation under objcg_lock.
> 
> [2] Bug 2: You should exceed memcg limit to invoke
> memcg_alloc_abort_single(), but you don't even have to be under
> memory pressure to exceed that. (yeah, I had to modify the
> kernel to implement a fault-injection-like-feature to trigger this).
> Unfortunately, you cannot reclaim memory in unknown context when you
> hit the limit. This should be fairly easy to trigger.
> 
> Can this cause an actual deadlock? That depends on the chances
> of calling kmalloc/kfree_nolock() within the slab allocator.
> 

Re: [PATCH RFC 0/4] memcg,slab: kmalloc_nolock() fixes

[Vlastimil Babka (SUSE)]

On 6/26/26 08:04, Harry Yoo wrote:
> 
> 
> On 6/24/26 10:11 PM, Harry Yoo (Oracle) wrote:
>> Apologies for posting another series during the merge window.
>> But these are bug fixes and there are other features that need to be
>> rebased on top, so...
> 
> Okay, given that this needs a bit of discussion with the memcg folks and
> this has been there for a year and nobody noticed, I'd suggest we be bit
> less hurried and not delay other new features going on because of this.

Seems so, it would be hard to backport anyway.

> But I still think they should eventually be fixed.

Ack.