[PATCH] mm/slab: Call call_rcu() directly from __kfree_rcu_sheaf() in any context

[PATCH] mm/slab: Call call_rcu() directly from __kfree_rcu_sheaf() in any context

[Puranjay Mohan]

__kfree_rcu_sheaf() can run with allow_spin == false, e.g. from
kfree_rcu_nolock(), where it may interrupt call_rcu() itself on the same
CPU. call_rcu() was not reentrant, so commit 69abda97a09b2 ("mm/slab:
extend deferred free mechanism to handle rcu sheaves") added a fallback:
when !allow_spin and IRQs are disabled, the full rcu sheaf is put on a
per-CPU llist and submitted to call_rcu() later from irq_work.

call_rcu() now handles this itself. When it is called with IRQs disabled
(from NMI, or re-entered from instrumentation), it stages the callback on
a per-CPU list and re-issues it from irq_work. Callers no longer need to
guard against that, so always call call_rcu() directly and remove the
rcu_sheaves llist and its irq_work handling.

Signed-off-by: Puranjay Mohan <puranjay@kernel.org>
---
 mm/slub.c | 28 +++-------------------------
 1 file changed, 3 insertions(+), 25 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index 8fbc6bf8ebc51..6c44f4495db5a 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -4145,7 +4145,6 @@ static void flush_all(struct kmem_cache *s)
 struct deferred_percpu_work {
 	struct llist_head objects;
 	struct llist_head objects_by_rcu;
-	struct llist_head rcu_sheaves;
 	struct irq_work work;
 };
 
@@ -4154,7 +4153,6 @@ static void deferred_percpu_work_fn(struct irq_work *work);
 static DEFINE_PER_CPU(struct deferred_percpu_work, deferred_percpu_work) = {
 	.objects = LLIST_HEAD_INIT(objects),
 	.objects_by_rcu = LLIST_HEAD_INIT(objects_by_rcu),
-	.rcu_sheaves = LLIST_HEAD_INIT(rcu_sheaves),
 	.work = IRQ_WORK_INIT(deferred_percpu_work_fn),
 };
 
@@ -6308,22 +6306,8 @@ bool __kfree_rcu_sheaf(struct kmem_cache *s, void *obj, unsigned int free_flags)
 	 * we flush before local_unlock to make sure a racing
 	 * flush_all_rcu_sheaves() doesn't miss this sheaf
 	 */
-	if (rcu_sheaf) {
-		/*
-		 * With !allow_spin, we might have interrupted call_rcu()'s
-		 * IRQ-disabled critical section. If IRQs are not disabled,
-		 * we know that's not the case.
-		 */
-		if (unlikely(!allow_spin && irqs_disabled())) {
-			struct deferred_percpu_work *dpw;
-
-			dpw = this_cpu_ptr(&deferred_percpu_work);
-			if (llist_add(&rcu_sheaf->llnode, &dpw->rcu_sheaves))
-				irq_work_queue(&dpw->work);
-		} else {
-			call_rcu(&rcu_sheaf->rcu_head, rcu_free_sheaf);
-		}
-	}
+	if (rcu_sheaf)
+		call_rcu(&rcu_sheaf->rcu_head, rcu_free_sheaf);
 
 	local_unlock(&s->cpu_sheaves->lock);
 
@@ -6532,12 +6516,10 @@ static void free_to_pcs_bulk(struct kmem_cache *s, size_t size, void **p)
 static void deferred_percpu_work_fn(struct irq_work *work)
 {
 	struct deferred_percpu_work *dpw;
-	struct llist_head *objs, *objs_by_rcu, *rcu_sheaves;
+	struct llist_head *objs, *objs_by_rcu;
 	struct llist_node *llnode, *pos, *t;
-	struct slab_sheaf *sheaf, *next;
 
 	dpw = container_of(work, struct deferred_percpu_work, work);
-	rcu_sheaves = &dpw->rcu_sheaves;
 	objs = &dpw->objects;
 	objs_by_rcu = &dpw->objects_by_rcu;
 
@@ -6571,10 +6553,6 @@ static void deferred_percpu_work_fn(struct irq_work *work)
 
 		kvfree_call_rcu(head, objp);
 	}
-
-	llnode = llist_del_all(rcu_sheaves);
-	llist_for_each_entry_safe(sheaf, next, llnode, llnode)
-		call_rcu(&sheaf->rcu_head, rcu_free_sheaf);
 }
 
 static void defer_free(struct kmem_cache *s, void *head)

base-commit: e52c7c28dbece00f54dff26e1554df2fe59b02f3
-- 
2.53.0-Meta

Re: [PATCH] mm/slab: Call call_rcu() directly from __kfree_rcu_sheaf() in any context

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] mm: call_rcu() list corruption due to reentrancy from NMI
- [Low] mm: unused llnode field in struct slab_sheaf
--

commit 81da114bf5411864e59d873df170399156acac6d
Author: Puranjay Mohan <puranjay@kernel.org>

mm/slab: Call call_rcu() directly from __kfree_rcu_sheaf() in any context

This patch removes the deferred irq_work fallback for rcu_sheaves and
instead calls call_rcu() directly from __kfree_rcu_sheaf() in all contexts.
The commit message states that call_rcu() now natively handles reentrancy
from NMIs.

> diff --git a/mm/slub.c b/mm/slub.c
> index 8fbc6bf8ebc51..6c44f4495db5a 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -4145,7 +4145,6 @@ static void flush_all(struct kmem_cache *s)
>  struct deferred_percpu_work {
>  	struct llist_head objects;
>  	struct llist_head objects_by_rcu;
> -	struct llist_head rcu_sheaves;
>  	struct irq_work work;
>  };

[Severity: Low]
This isn't a bug, but since this patch removes the rcu_sheaves list and all
its associated logic, should the llnode field in struct slab_sheaf also be
removed?

It appears the llnode field in struct slab_sheaf was only used to defer
this call_rcu() invocation.

[ ... ]

> @@ -6308,22 +6306,8 @@ bool __kfree_rcu_sheaf(struct kmem_cache *s, void *obj, unsigned int free_flags)
>  	 * we flush before local_unlock to make sure a racing
>  	 * flush_all_rcu_sheaves() doesn't miss this sheaf
>  	 */
> -	if (rcu_sheaf) {
> -		/*
> -		 * With !allow_spin, we might have interrupted call_rcu()'s
> -		 * IRQ-disabled critical section. If IRQs are not disabled,
> -		 * we know that's not the case.
> -		 */
> -		if (unlikely(!allow_spin && irqs_disabled())) {
> -			struct deferred_percpu_work *dpw;
> -
> -			dpw = this_cpu_ptr(&deferred_percpu_work);
> -			if (llist_add(&rcu_sheaf->llnode, &dpw->rcu_sheaves))
> -				irq_work_queue(&dpw->work);
> -		} else {
> -			call_rcu(&rcu_sheaf->rcu_head, rcu_free_sheaf);
> -		}
> -	}
> +	if (rcu_sheaf)
> +		call_rcu(&rcu_sheaf->rcu_head, rcu_free_sheaf);

[Severity: High]
The commit message states: "When it is called with IRQs disabled (from NMI,
or re-entered from instrumentation), it stages the callback on a per-CPU
list and re-issues it from irq_work."

Looking at the implementation of call_rcu() and __call_rcu_common() in
kernel/rcu/tree.c, it disables IRQs and synchronously manipulates the
callback list. It does not appear to contain an NMI/irq_work fallback.

If __kfree_rcu_sheaf() is called from an NMI or tracing context (such as
from bpf or perf) while the CPU is already executing call_rcu() or
rcu_do_batch(), could this direct call to call_rcu() re-enter the list
manipulation and corrupt the RCU callback lists?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016155340.3304319-1-puranjay@kernel.org?part=1