Re: [PATCH v3] 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:
- [Critical] __kfree_rcu_sheaf() allocates with GFP_NOWAIT on !PREEMPT_RT, which can trigger wakeup_kswapd() and deadlock against scheduler locks (e.g., pi_lock).
- [High] kfree_call_rcu_nolock() unconditionally calls __kfree_rcu_sheaf() and triggers lockdep wait context violations and circular dependencies on PREEMPT_RT when called from raw spinlock context.
--

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

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

This commit fixes a circular locking dependency on PREEMPT_RT by skipping
the kfree_rcu sheaves layer in kvfree_call_rcu() when called from contexts
that hold a raw spinlock. This safely falls back to the existing list.

Closes: https://syzkaller.appspot.com/bug?extid=acf142088e0182172e58
Fixes: 2a8bb29ec9b2 ("mm/slab: allow kfree_rcu_sheaf() on PREEMPT_RT")

> 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_t would
> hit the same lockdep ordering cycle through that path too.

[Severity: High]
This is a pre-existing issue, but since the patch leaves
kfree_call_rcu_nolock() untouched, does the API contract for the nolock
variant remain violated on PREEMPT_RT?

As noted in the commit message, calling kfree_call_rcu_nolock() still
unconditionally reaches __kfree_rcu_sheaf() with SLAB_FREE_NOLOCK, which
executes:

mm/slub.c:__kfree_rcu_sheaf() {
    ...
    if (!local_trylock(&s->cpu_sheaves->lock))
        goto fail;
    ...
    empty = alloc_empty_sheaf(s, gfp, alloc_flags);
    ...
}

While local_trylock() bypasses the Invalid wait context check, it still
creates a lock-class ordering dependency (raw_spinlock_t -> spinlock_t),
which causes circular dependency splats against other paths taking the lock.

Furthermore, if it needs an empty sheaf, it calls alloc_empty_sheaf(),
which enters the page allocator. The page allocator acquires sleeping locks
like zone->lock via spin_lock_irqsave. This triggers a hard Invalid wait
context violation and exposes the system to genuine deadlocks on PREEMPT_RT.

Could this be addressed to ensure kfree_call_rcu_nolock() is truly safe for
raw spinlock contexts as intended?

> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index b19ba1b31484c..015380ba8bcc6 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c

[ ... ]

> @@ -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_t here on PREEMPT_RT (e.g.
> +	 * set_cpus_allowed_force(), whose callers all hold
> +	 * task_struct::pi_lock), 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))

[Severity: Critical]
This isn't a bug introduced by this patch, but does falling back to
kfree_rcu_sheaf() on !PREEMPT_RT expose us to a deadlock when called under
scheduler locks?

When kvfree_call_rcu() is called holding pi_lock, like in
set_cpus_allowed_force(), on !PREEMPT_RT it enters __kfree_rcu_sheaf() with
allow_spin evaluating to true. This sets up the allocation:

mm/slub.c:__kfree_rcu_sheaf() {
    ...
    gfp_t gfp = allow_spin ? GFP_NOWAIT : __GFP_NOWARN;
    ...
    empty = alloc_empty_sheaf(s, gfp, alloc_flags);
    ...
}

GFP_NOWAIT includes __GFP_KSWAPD_RECLAIM. If alloc_empty_sheaf() needs to
be called, the page allocator will wake kswapd.

Waking kswapd calls try_to_wake_up(), which attempts to acquire scheduler
locks such as the runqueue lock or kswapd->pi_lock.

If the original caller already holds pi_lock or rq_lock, wouldn't this
cause lock recursion (e.g., rq_lock -> pi_lock -> rq_lock) and deadlock
the system?

>  		return;
>  
>  	// Queue the object but don't yet schedule the batch.

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