Re: [PATCH slab/for-next v4 2/8] mm/slab: handle the !allow_spin case in kfree_rcu_sheaf()

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

New issues:
- [High] kfree_rcu_sheaf() hardcodes SLAB_FREE_DEFAULT, rendering the new !allow_spin fallback logic unreachable and leaving lock recursion bugs unfixed.

Pre-existing issues:
- [High] Calling kfree_nolock() can trigger RT-mutex deadlocks when executed in NMI context on PREEMPT_RT kernels.
--

--- Patch [2]: [PATCH slab/for-next v4 2/8] mm/slab: handle the !allow_spin case in kfree_rcu_sheaf() ---
commit 412874c34b3d9cc0ed877c03632bf3c3cd9499d6
Author: Harry Yoo (Oracle) <[email protected]>

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

This commit teaches kfree_rcu_sheaf() how to handle the allow_spin false
case by trying to get an empty sheaf from pcs->spare or the barn even when
spinning is not allowed. It introduces free_flags to describe the context
and converts them to alloc_flags when allocating memory in the free path.

Link: https://lore.kernel.org/linux-mm/[email protected]

> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index b6426d7ceec92..e07b4e6d6679c 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -1605,7 +1605,7 @@ static bool kfree_rcu_sheaf(void *obj)
>  
>  	s = slab->slab_cache;
>  	if (likely(!IS_ENABLED(CONFIG_NUMA) || slab_nid(slab) == numa_mem_id()))
> -		return __kfree_rcu_sheaf(s, obj);
> +		return __kfree_rcu_sheaf(s, obj, SLAB_FREE_DEFAULT);

[Severity: High]
Does this hardcoded SLAB_FREE_DEFAULT prevent the new allow_spin fallback
logic from ever being executed?

Since kfree_rcu_sheaf() is the sole caller of __kfree_rcu_sheaf(), passing
SLAB_FREE_DEFAULT unconditionally appears to make the new context-handling
logic unreachable.

>  
>  	return false;
>  }

[ ... ]

> diff --git a/mm/slub.c b/mm/slub.c
> index e32a686775374..0c350274fbff4 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -2829,11 +2833,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);

[Severity: High]
This is a pre-existing issue, but can calling kfree_nolock() here trigger
an RT-mutex deadlock when executed in NMI context on PREEMPT_RT kernels?

Looking at kfree_nolock() in mm/slub.c, it calls free_to_pcs() with the
following sequence:

    if (likely(can_free_to_pcs(slab)) && likely(free_to_pcs(s, x, false)))
        return;

And free_to_pcs() acquires the lock via local_trylock():

    if (!local_trylock(&s->cpu_sheaves->lock))
        return false;

On PREEMPT_RT, local_trylock() maps to an RT-mutex, which can deadlock if
the NMI interrupted a critical section holding the internal raw spinlock on
the same CPU.

>  
>  	stat(s, SHEAF_FREE);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2