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

[Boqun Feng <boqun@kernel.org>]

On Sun, Mar 22, 2026 at 03:09:19AM -0700, Paul E. McKenney wrote:
> On Sat, Mar 21, 2026 at 01:08:59PM -0700, Boqun Feng wrote:
> > On Sat, Mar 21, 2026 at 01:07:45PM -0700, Paul E. McKenney wrote:
> > > On Sat, Mar 21, 2026 at 12:45:27PM -0700, Boqun Feng wrote:
> > > > On Sat, Mar 21, 2026 at 12:31:04PM -0700, Paul E. McKenney wrote:
> > > > > On Sat, Mar 21, 2026 at 11:06:59AM -0700, Boqun Feng wrote:
> > > > > > On Sat, Mar 21, 2026 at 10:41:47AM -0700, Paul E. McKenney wrote:
> > > > > > [...]
> > > > > > > > > +	raw_spin_lock_rcu_node(ssp->srcu_sup);
> > > > > > > > > +	delay = srcu_get_delay(ssp);
> > > > > > > > > +	raw_spin_unlock_rcu_node(ssp->srcu_sup);
> > > > > > > > > 
> > > > > > > > 
> > > > > > > > It was fixed differently in v2:
> > > > > > > > 
> > > > > > > > 	https://lore.kernel.org/rcu/20260320222916.19987-1-boqun@kernel.org/
> > > > > > > > 
> > > > > > > > I used _irqsave/_irqrestore just in case. Given it's an urgent fix,
> > > > > > > > overly careful code is probably fine ;-)
> > > > > > > > 
> > > > > > > > Thanks for the testing and feedback.
> > > > > > > 
> > > > > > > OK, I will try that one, thank you!
> > > > > > > 
> > > > > > > FYI, with my change on your earlier version, SRCU-T got deadlocks between
> > > > > > > the pi-lock and the workqueue pool lock.  Which might or might not be
> > > > > > > particularly urgent.
> > > > > > > 
> > > > > > 
> > > > > > I just checked my run yesterday, I also hit it. It's probably what
> > > > > > Zqiang has found:
> > > > > > 
> > > > > > 	https://lore.kernel.org/rcu/4c23c66f86a2aff8f2d7b759f9dd257b82147a17@linux.dev/
> > > > > > 
> > > > > > We have a queue_work_on() in srcu_schedule_cbs_sdp(), so
> > > > > > 
> > > > > >     srcu_torture_deferred_free():
> > > > > >       raw_spin_lock_irqsave(->pi_lock,...);
> > > > > >       call_srcu():
> > > > > >         if (snp == snp_leaf && snp_seq != s) {
> > > > > >            srcu_schedule_cbs_sdp(sdp, do_norm ? SRCU_INTERVAL : 0):
> > > > > > 	     if (!delay)
> > > > > >                queue_work_on(...)
> > > > > > 
> > > > > > I was about to reply to Zqiang, fixing that could be a touch design
> > > > > > decision. Since it's a per srcu_data work ;-) NR_CPUS x irq_work
> > > > > > incoming.
> > > > > 
> > > > > Just to be clear, SRCU-T is Tiny SRCU rather than Tree SRCU.  So perhaps
> > > > > lower priority, though perhaps not lower irritation.  ;-)
> > > > 
> > > > I see, there is a schedule_work() in srcutiny's
> > > > srcu_gp_start_if_needed(). But it couldn't cause deadlock on UP since
> > > > locks are (almost) no-op. Maybe we can make RCU torture only test it on
> > > > SMP?
> > > 
> > > Like this, you mean?  I will give it a shot tomorrow.
> > 
> > Yes, thanks!
> 
> OK, the previous patch did fine on short rcutorture testing aside from
> the !SMP lockdep splat, so I have started the test without pi_lock.
> 
> Longer term, shouldn't lockdep take into account the fact that on !SMP,
> the disabling of preemption (or interrupts or...) is essentially the same
> as acquiring a global lock?  This means that only one task at a time can
> be acquiring a raw spinlock on !SMP, so that the order of acquisition
> of raw spinlocks on !SMP is irrelevant?  (Aside from self-deadlocking
> double acquisitions, perhaps.)  In other words, shouldn't lockdep leave
> raw spinlocks out of lockdep's cycle-detection data structure?
> 

Lockdep doesn't know whether a code path is UP-only, so it'll apply the
general locking rule to check. Similar as lockdep still detect
PREEMPT_RT locking issue for !PREEMPT_RT kernel. Maybe we can add a
separate kconfig to narrow down lockdep detection for UP-only if UP=y.

Regards,
Boqun

> 							Thanx, Paul