On Mon, Sep 14, 2026 at 8:12 AM Zqiang <[email protected]> wrote: > > > > > On Sun, Sep 13, 2026 at 2:21 PM Zqiang <[email protected]> wrote: > > > > > > > > On Sat, Sep 12, 2026 at 08:37:33PM +0800, KunWu Chan wrote: > > > > > > > > > > > On Sat, Sep 12, 2026 at 12:47 AM Paul E. McKenney <[email protected]> > > > wrote: > > > > > > > > On Fri, Sep 11, 2026 at 05:35:15PM +0800, Kunwu Chan wrote: > > > > > Add the lockdep annotation, same-type SRCU nesting warning, and > > > > > early-boot check used by __synchronize_srcu(). > > > > > > > > > > Suggested-by: Zqiang <[email protected]> > > > > > Signed-off-by: Kunwu Chan <[email protected]> > > > > > > > > Queued for review and testing, thank you both! > > > > > > > > Interestingly enough, it is now the case that there is a grace-period > > > > wait that can be placed in a normal RCU read-side critical section. > > > > Does this mean that we should also adjust the --do-srcu-lockdep testing > > > > in tools/testing/selftests/rcutorture/bin/torture.sh? > > > > > > > > Thanks, Paul. Good point. > > > > > > > > I’ll check the current `--do-srcu-lockdep` coverage, including the > > > > case where `synchronize_srcu_atomic()` is called from a normal RCU > > > > read-side critical section, and follow up with the necessary torture > > > > testing changes. > > > > > > > Sounds good! > > > > > > Perhaps you and Zqiang can work together on this. Co-developed-by, > > > for example. > > > > > > Hi, Paul and KunWu > > > > > Hi Zqiang, > > > > Thanks for pointing out these cases. > > > > > > > > Should we also consider the following situations ? > > > > > > idx = srcu_read_lock_atomic(srcu) > > > > > > by interrupt run hardirq context: > > > synchronize_rcu_atomic(srcu) > > > > > > srcu_read_unlock_atomic(srcu, idx) > > > > > For the same-CPU interrupt case, the existing check in > > synchronize_srcu_atomic() already catches it: > > > > synchronize_srcu_atomic(ssp) > > RCU_LOCKDEP_WARN(lockdep_is_held(ssp), > > "Illegal synchronize_srcu_atomic() in same-type SRCU ..."); > > > > lockdep_is_held() resolves to __lock_is_held() (lockdep.c:5612), > > which checks current->held_locks[]. The interrupt handler runs with > > the same current, so it sees the SRCU dep_map acquired by > > srcu_read_lock_atomic(). > > > > > > > > or: > > > > > > CPU0: CPU1: > > > > > > idx = srcu_read_lock_atomic(srcu) > > > > > > smp_call_function_single(CPU1, som_func, NULL, 1) > > > to send IPI to CPU1, and sync wait complete. > > > hardirq context or ide task context: > > > > > > some_func() > > > ->synchronize_rcu_atomic(srcu) > > > > > > srcu_read_unlock_atomic(srcu, idx) > > > > > The cross-CPU case is different. current->held_locks[] is part of > > struct task_struct (sched.h:1302), so the existing > > __lock_is_held() check can only see the current task's held locks. > > It cannot see the SRCU read-side lock held by the task running on > > another CPU. The same limitation applies to the > > lock_is_held(&rcu_lock_map) check in synchronize_srcu() at > > srcutree.c:1665. > > > > > > > > Add WARN_ON(irqs_disabled()) to synchronize_rcu_atomic() ? > > > > > > Any thoughts? > > > > > WARN_ON(irqs_disabled()) wouldn't help with the cross-CPU case: > > CPU1 could be running in process context with interrupts enabled, so > > the WARN would not trigger. It would also add a false positive for > > legitimate hardirq calls. synchronize_srcu_atomic() omits > > might_sleep() (compare __synchronize_srcu() at srcutree.c:1676) > > because it is designed to work in contexts where sleeping is not > > allowed, including hardirq context. > > If runing in hardirq context, we should use raw_spin_lock_irqsave_rcu_node() > and raw_spin_unlock_irqrestore_rcu_node() to replace > raw_spin_lock_irq_rcu_node() > and raw_spin_unlock_irq_rcu_node() in synchronize_srcu_atomic(). >
Thanks, Zqiang. I'll take a closer look at the IRQ-state handling and consider the change. If a patch comes out of it, I'll add the Co-developed-by tag. Thanks, Kunwu > Thanks > Zqiang > > > > > > > Whether a general cross-CPU read-side-hold check is feasible is an > > open question. It would need to account for the read-side state across > > CPUs without adding too much overhead to the SRCU read-side fast path. > > > > I'm happy to discuss and explore whether there is a reasonable way to > > handle this cross-CPU case. > > > > Thanks, > > Kunwu > > > > > > > > Thanks > > > Zqiang > > > > > > Thanx, Paul > > > > > > > > > > > Thanks, > > > > Kunwu > > > > > > > > > > > > Thanx, Paul > > > > > > > > > --- > > > > > kernel/rcu/srcutree.c | 8 ++++++++ > > > > > 1 file changed, 8 insertions(+) > > > > > > > > > > diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c > > > > > index 6a9c432a3bd0..6c729e805fb3 100644 > > > > > --- a/kernel/rcu/srcutree.c > > > > > +++ b/kernel/rcu/srcutree.c > > > > > @@ -2123,6 +2123,14 @@ void synchronize_srcu_atomic(struct > > > srcu_struct *ssp) > > > > > unsigned long rdm0, rdm1; > > > > > unsigned long unlocks0, unlocks1; > > > > > > > > > > + srcu_lock_sync(&ssp->dep_map); > > > > > + > > > > > + RCU_LOCKDEP_WARN(lockdep_is_held(ssp), > > > > > + "Illegal synchronize_srcu_atomic() in same-type SRCU read-side > > > critical section"); > > > > > + > > > > > + if (rcu_scheduler_active == RCU_SCHEDULER_INACTIVE) > > > > > + return; > > > > > + > > > > > // Initialize. Either init_srcu_struct() was invoked or > > > > > // DEFINE_SRCU() or similar was used. Therefore, no allocation > > > > > // will be done here. > > > > > -- > > > > > 2.43.0 > > > > > > > > > > > > > >
