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

[sashiko-bot@kernel.org]

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