Re: [PATCH for-next v3 3/9] mm/slab: handle the !allow_spin case in kfree_rcu_sheaf()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Kmemleak tracking symmetry violation for empty sheaves allocated with !allow_spin.

Pre-existing issues:
- [High] Deadlock under scheduler locks due to GFP_NOWAIT allocation in __kfree_rcu_sheaf().
--

commit 0ddecba01f906bc55d1c11049f09813fcc05200e
Author: Harry Yoo (Oracle) <harry@xxxxxxxxxx>

mm/slab: handle the !allow_spin case in kfree_rcu_sheaf()

This commit updates kfree_rcu_sheaf() to handle cases where spinning is not
allowed, by allocating and freeing empty sheaves without taking locks. It also
modifies the allocation flags and fallback paths depending on the allow_spin
context.

> diff --git a/mm/slub.c b/mm/slub.c
> index 87ca154ccd80f..b0d38d5153865 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
[ ... ]
> @@ -2827,11 +2828,20 @@ static void free_empty_sheaf(struct kmem_cache *s, struct slab_sheaf *sheaf)
>  		mark_obj_codetag_empty(sheaf);
>  
>  	VM_WARN_ON_ONCE(sheaf->size > 0);
> -	kfree(sheaf);
> +
> +	if (likely(allow_spin))
> +		kfree(sheaf);
> +	else
> +		kfree_nolock(sheaf);
>  
>  	stat(s, SHEAF_FREE);
>  }
>  
> +static void free_empty_sheaf(struct kmem_cache *s, struct slab_sheaf *sheaf)
> +{
> +	__free_empty_sheaf(s, sheaf, /* allow_spin = */ true);
> +}

[Severity: High]
Does hardcoding allow_spin to true here cause a kmemleak tracking symmetry
violation for sheaves allocated with allow_spin = false?

In __kfree_rcu_sheaf(), if allow_spin is false, the empty sheaf is allocated
using SLAB_ALLOC_TRYLOCK and gfp = 0. Because __GFP_RECLAIM bits are missing,
SLUB skips kmemleak registration for the object.

When this sheaf becomes full and is processed by RCU, rcu_free_sheaf() handles
the cleanup and delegates to free_empty_sheaf(), which now unconditionally
calls kfree(sheaf) because allow_spin is hardcoded to true. Calling kfree()
on an object that wasn't registered with kmemleak triggers kmemleak warnings.

[ ... ]
> @@ -6112,20 +6122,25 @@ bool __kfree_rcu_sheaf(struct kmem_cache *s, void *obj)
>  
>  		local_unlock(&s->cpu_sheaves->lock);
>  
> -		empty = alloc_empty_sheaf(s, GFP_NOWAIT, SLAB_ALLOC_DEFAULT);
> +		if (unlikely(!allow_spin)) {
> +			alloc_flags = SLAB_ALLOC_TRYLOCK;
> +			gfp = 0;
> +		}
> +
> +		empty = alloc_empty_sheaf(s, gfp, alloc_flags);

[Severity: High]
This is a pre-existing issue, but does the use of GFP_NOWAIT when allow_spin
is true risk a deadlock?

Since GFP_NOWAIT contains the __GFP_KSWAPD_RECLAIM flag, the allocation can
trigger wakeup_kswapd(). This enters the scheduler and attempts to acquire
scheduler locks.

If kvfree_call_rcu() is invoked while a scheduler lock is already held,
wouldn't this cause a lock recursion deadlock?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260615-kfree_rcu_nolock-v3-0-70a54f3775bb@xxxxxxxxxx?part=3