Re: [PATCH] rcu: Use an intermediate irq_work to start process_srcu()

[Andrea Righi <arighi@nvidia.com>]

On Fri, Mar 20, 2026 at 01:54:01PM -0700, Boqun Feng wrote:
> On Fri, Mar 20, 2026 at 09:47:51PM +0100, Andrea Righi wrote:
> > Hi Boqun,
> > 
> > On Fri, Mar 20, 2026 at 11:14:00AM -0700, Boqun Feng wrote:
> > > Since commit c27cea4416a3 ("rcu: Re-implement RCU Tasks Trace in terms
> > > of SRCU-fast") we switched to SRCU in BPF. However as BPF instrument can
> > > happen basically everywhere (including where a scheduler lock is held),
> > > call_srcu() now needs to avoid acquiring scheduler lock because
> > > otherwise it could cause deadlock [1]. Fix this by following what the
> > > previous RCU Tasks Trace did: using an irq_work to delay the queuing of
> > > the work to start process_srcu().
> > > 
> > > [boqun: Apply Joel's feedback]
> > > 
> > > Reported-by: Andrea Righi <arighi@nvidia.com>
> > > Closes: https://lore.kernel.org/all/abjzvz_tL_siV17s@gpd4/
> > > Fixes: commit c27cea4416a3 ("rcu: Re-implement RCU Tasks Trace in terms of SRCU-fast")
> > > Link: https://lore.kernel.org/rcu/3c4c5a29-24ea-492d-aeee-e0d9605b4183@nvidia.com/ [1]
> > > Suggested-by: Zqiang <qiang.zhang@linux.dev>
> > > Signed-off-by: Boqun Feng <boqun@kernel.org>
> > > ---
> > > @Zqiang, I put your name as Suggested-by because you proposed the same
> > > idea, let me know if you rather not have it.
> > > 
> > > @Joel, I did two updates (including your test feedback, other one is
> > > call irq_work_sync() when we clean the srcu_struct), please give it a
> > > try.
> > 
> > I'm getting this at boot with this patch applied (testing directly from
> > Joel's branch rcu/dev):
> > 
> > [    0.639477] DEBUG_LOCKS_WARN_ON(lockdep_hardirq_context())
> 
> My bad, this is missing:
> 
> diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
> index 73aef361a524..e08aaacad695 100644
> --- a/kernel/rcu/srcutree.c
> +++ b/kernel/rcu/srcutree.c
> @@ -1993,13 +1993,14 @@ static void srcu_irq_work(struct irq_work *work)
>         struct srcu_struct *ssp;
>         struct srcu_usage *sup;
>         unsigned long delay;
> +       unsigned long flags;
> 
>         sup = container_of(work, struct srcu_usage, irq_work);
>         ssp = sup->srcu_ssp;
> 
> -       raw_spin_lock_irq_rcu_node(ssp->srcu_sup);
> +       raw_spin_lock_irqsave_rcu_node(ssp->srcu_sup, flags);
>         delay = srcu_get_delay(ssp);
> -       raw_spin_unlock_irq_rcu_node(ssp->srcu_sup);
> +       raw_spin_unlock_irqrestore_rcu_node(ssp->srcu_sup, flags);
> 
>         queue_delayed_work(rcu_gp_wq, &sup->work, !!delay);
>  }

Ah yes, much better with this one. :) And I confirm that it fixes the
initial locking issue that I reported.

Thanks!
-Andrea