Re: [PATCH v2] mm/slab: don't use kfree_rcu sheaves on PREEMPT_RT in kvfree_call_rcu()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Lock recursion deadlock in kfree_rcu() on !PREEMPT_RT when caller holds scheduler locks
- [High] kfree_call_rcu_nolock() violates its 'any context' API guarantee on PREEMPT_RT, leading to Invalid Wait Context
--

commit 405a1ecd91cd7581e29421663a94a33714163104
Author: ThangNN99 <ngocthang2710.1999@gmail.com>

mm/slab: don't use kfree_rcu sheaves on PREEMPT_RT in kvfree_call_rcu()

This commit addresses a lockdep warning by skipping the sheaf fast path
in kvfree_call_rcu() on PREEMPT_RT, avoiding a local_trylock() under a
raw spinlock. It also notes that kfree_call_rcu_nolock() has a similar
problem but leaves it untouched.

Closes: https://syzkaller.appspot.com/bug?extid=acf142088e0182172e58

> kfree_call_rcu_nolock(), added later in commit 3bc999d944b3
> ("mm/slab: introduce kfree_rcu_nolock()"), is untouched by this
> patch. Note it would not be a safe substitute here either: it still
> reaches __kfree_rcu_sheaf()'s local_trylock() on &s->cpu_sheaves->lock
> unconditionally, so a caller already holding a raw spinlock would hit
> the same lockdep ordering cycle through that path too.

[Severity: High]
This is a pre-existing issue, but does leaving this unaddressed violate
the API guarantee of kfree_call_rcu_nolock() in mm/slab_common.c?

The documentation for kfree_call_rcu_nolock() states it is safe for any
context. However, on PREEMPT_RT it leads to this call chain:

kfree_call_rcu_nolock()
  __kfree_rcu_sheaf(..., SLAB_FREE_NOLOCK)
    local_trylock(&s->cpu_sheaves->lock)

Since local_lock maps to a sleepable rt_mutex on PREEMPT_RT, executing
this trylock from a raw spinlock or hardirq context violates wait-type
rules.

Won't any subsystem using kfree_call_rcu_nolock() in these contexts still
trigger an Invalid wait context lockdep splat on PREEMPT_RT?

> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index b19ba1b31484c..3de1eabe6c770 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -1667,15 +1667,8 @@ static bool kfree_rcu_sheaf(void *obj)
>  {
>  	struct kmem_cache *s;
>  	struct slab *slab;
> -	unsigned int free_flags = SLAB_FREE_DEFAULT;
> -
> -	/*
> -	 * It is not safe to spin on PREEMPT_RT because the kernel might be
> -	 * holding a raw spinlock and slab acquires sleeping locks.
> -	 */
> -	if (IS_ENABLED(CONFIG_PREEMPT_RT))
> -		free_flags = SLAB_FREE_NOLOCK;
>  
> +	/* Callers on PREEMPT_RT never reach here, see kvfree_call_rcu(). */
>  	if (is_vmalloc_addr(obj))
>  		return false;
>  
> @@ -1685,7 +1678,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, free_flags);
> +		return __kfree_rcu_sheaf(s, obj, SLAB_FREE_DEFAULT);
>  
>  	return false;
>  }
> @@ -2034,7 +2027,14 @@ void kvfree_call_rcu(struct kvfree_rcu_head *head, void *ptr)
>  	if (!head)
>  		might_sleep();
>  
> -	if (kfree_rcu_sheaf(ptr))
> +	/*
> +	 * Callers may hold a raw spinlock here on PREEMPT_RT (e.g.
> +	 * set_cpus_allowed_force() with p->pi_lock held), and the sheaf/barn
> +	 * locks are also taken as blocking locks elsewhere, so trying them
> +	 * here creates a lockdep-visible ordering conflict. Skip sheaves on
> +	 * PREEMPT_RT.
> +	 */
> +	if (!IS_ENABLED(CONFIG_PREEMPT_RT) && kfree_rcu_sheaf(ptr))
>  		return;

[Severity: High]
This isn't a bug introduced by this patch, but does this lead to a lock
recursion deadlock on !PREEMPT_RT?

On !PREEMPT_RT, this can result in the following call chain:

kvfree_call_rcu()
  kfree_rcu_sheaf()
    __kfree_rcu_sheaf(s, obj, SLAB_FREE_DEFAULT)
      alloc_empty_sheaf() with GFP_NOWAIT
        wakeup_kswapd()
          try_to_wake_up()

Since GFP_NOWAIT includes __GFP_KSWAPD_RECLAIM, the page allocator can call
wakeup_kswapd() if the zone watermark is low.

This enters the scheduler and attempts to acquire scheduler locks. If the
caller, like set_cpus_allowed_force() in the trace above, already holds a
scheduler raw spinlock, won't this result in a hard deadlock?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831060633.6831-1-ngocthang2710.1999@gmail.com?part=1