* [PATCH] srcutree: Add lockdep and early-boot checks to synchronize_srcu_atomic()
@ 2026-09-11 9:35 Kunwu Chan
2026-09-11 16:47 ` Paul E. McKenney
0 siblings, 1 reply; 11+ messages in thread
From: Kunwu Chan @ 2026-09-11 9:35 UTC (permalink / raw)
To: jiangshanlai, paulmck
Cc: josh, rostedt, mathieu.desnoyers, rcu, linux-kernel, Kunwu Chan, Zqiang
Add the lockdep annotation, same-type SRCU nesting warning, and
early-boot check used by __synchronize_srcu().
Suggested-by: Zqiang <qiang.zhang@linux.dev>
Signed-off-by: Kunwu Chan <kunwu.chan@gmail.com>
---
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] srcutree: Add lockdep and early-boot checks to synchronize_srcu_atomic()
2026-09-11 9:35 [PATCH] srcutree: Add lockdep and early-boot checks to synchronize_srcu_atomic() Kunwu Chan
@ 2026-09-11 16:47 ` Paul E. McKenney
2026-09-12 12:37 ` KunWu Chan
0 siblings, 1 reply; 11+ messages in thread
From: Paul E. McKenney @ 2026-09-11 16:47 UTC (permalink / raw)
To: Kunwu Chan
Cc: jiangshanlai, josh, rostedt, mathieu.desnoyers, rcu,
linux-kernel, Zqiang
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 <qiang.zhang@linux.dev>
> Signed-off-by: Kunwu Chan <kunwu.chan@gmail.com>
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?
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
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] srcutree: Add lockdep and early-boot checks to synchronize_srcu_atomic()
2026-09-11 16:47 ` Paul E. McKenney
@ 2026-09-12 12:37 ` KunWu Chan
2026-09-12 18:50 ` Paul E. McKenney
0 siblings, 1 reply; 11+ messages in thread
From: KunWu Chan @ 2026-09-12 12:37 UTC (permalink / raw)
To: paulmck
Cc: jiangshanlai, josh, rostedt, mathieu.desnoyers, rcu,
linux-kernel, Zqiang
On Sat, Sep 12, 2026 at 12:47 AM Paul E. McKenney <paulmck@kernel.org> 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 <qiang.zhang@linux.dev>
> > Signed-off-by: Kunwu Chan <kunwu.chan@gmail.com>
>
> 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.
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
> >
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] srcutree: Add lockdep and early-boot checks to synchronize_srcu_atomic()
2026-09-12 12:37 ` KunWu Chan
@ 2026-09-12 18:50 ` Paul E. McKenney
2026-09-13 6:21 ` Zqiang
2026-09-13 10:23 ` KunWu Chan
0 siblings, 2 replies; 11+ messages in thread
From: Paul E. McKenney @ 2026-09-12 18:50 UTC (permalink / raw)
To: KunWu Chan
Cc: jiangshanlai, josh, rostedt, mathieu.desnoyers, rcu,
linux-kernel, Zqiang
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 <paulmck@kernel.org> 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 <qiang.zhang@linux.dev>
> > > Signed-off-by: Kunwu Chan <kunwu.chan@gmail.com>
> >
> > 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.
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
> > >
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] srcutree: Add lockdep and early-boot checks to synchronize_srcu_atomic()
2026-09-12 18:50 ` Paul E. McKenney
@ 2026-09-13 6:21 ` Zqiang
2026-09-13 10:36 ` KunWu Chan
2026-09-13 10:23 ` KunWu Chan
1 sibling, 1 reply; 11+ messages in thread
From: Zqiang @ 2026-09-13 6:21 UTC (permalink / raw)
To: paulmck, KunWu Chan
Cc: jiangshanlai, josh, rostedt, mathieu.desnoyers, rcu, linux-kernel
>
> 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 <paulmck@kernel.org> 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 <qiang.zhang@linux.dev>
> > > Signed-off-by: Kunwu Chan <kunwu.chan@gmail.com>
> >
> > 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
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)
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)
Add WARN_ON(irqs_disabled()) to synchronize_rcu_atomic() ?
Any thoughts?
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
> > >
> >
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] srcutree: Add lockdep and early-boot checks to synchronize_srcu_atomic()
2026-09-12 18:50 ` Paul E. McKenney
2026-09-13 6:21 ` Zqiang
@ 2026-09-13 10:23 ` KunWu Chan
1 sibling, 0 replies; 11+ messages in thread
From: KunWu Chan @ 2026-09-13 10:23 UTC (permalink / raw)
To: paulmck, Zqiang
Cc: jiangshanlai, josh, rostedt, mathieu.desnoyers, rcu, linux-kernel
On Sun, Sep 13, 2026 at 2:50 AM Paul E. McKenney <paulmck@kernel.org> 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 <paulmck@kernel.org> 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 <qiang.zhang@linux.dev>
> > > > Signed-off-by: Kunwu Chan <kunwu.chan@gmail.com>
> > >
> > > 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.
Thanks, Paul. I'am happy to do that.
@Zqiang, your suggestions on the additional lockdep cases got me
started, and these are now incorporated as testtypes 4 and 5.
Paul's observation that synchronize_srcu_atomic() can legitimately
be called from a normal RCU read-side critical section led to an
additional regression test (testtype 6).
In particular, testtype 6 verifies that calling
synchronize_srcu_atomic() from within rcu_read_lock()/rcu_read_unlock()
does not produce a false-positive lockdep warning.
The atomic SRCU same-type nesting cases are covered by testtype 4
and are expected to trigger the RCU_LOCKDEP_WARN() in
synchronize_srcu_atomic().
The changes are in rcutorture.c and srcu_lockdep.sh; torture.sh
does not need to change. The testing changes are basically complete.
@Zqiang, I'm happy to add:
Co-developed-by: Zqiang <qiang.zhang@linux.dev>
Signed-off-by: Zqiang <qiang.zhang@linux.dev>
if you're OK with that.
Thanks,
Kunwu
>
> 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
> > > >
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] srcutree: Add lockdep and early-boot checks to synchronize_srcu_atomic()
2026-09-13 6:21 ` Zqiang
@ 2026-09-13 10:36 ` KunWu Chan
2026-09-13 18:18 ` Paul E. McKenney
2026-09-14 0:12 ` Zqiang
0 siblings, 2 replies; 11+ messages in thread
From: KunWu Chan @ 2026-09-13 10:36 UTC (permalink / raw)
To: Zqiang
Cc: paulmck, jiangshanlai, josh, rostedt, mathieu.desnoyers, rcu,
linux-kernel
On Sun, Sep 13, 2026 at 2:21 PM Zqiang <qiang.zhang@linux.dev> 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 <paulmck@kernel.org> 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 <qiang.zhang@linux.dev>
> > > > Signed-off-by: Kunwu Chan <kunwu.chan@gmail.com>
> > >
> > > 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.
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
> > > >
> > >
> >
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] srcutree: Add lockdep and early-boot checks to synchronize_srcu_atomic()
2026-09-13 10:36 ` KunWu Chan
@ 2026-09-13 18:18 ` Paul E. McKenney
2026-09-14 2:55 ` KunWu Chan
2026-09-14 0:12 ` Zqiang
1 sibling, 1 reply; 11+ messages in thread
From: Paul E. McKenney @ 2026-09-13 18:18 UTC (permalink / raw)
To: KunWu Chan
Cc: Zqiang, jiangshanlai, josh, rostedt, mathieu.desnoyers, rcu,
linux-kernel
On Sun, Sep 13, 2026 at 06:36:56PM +0800, KunWu Chan wrote:
> On Sun, Sep 13, 2026 at 2:21 PM Zqiang <qiang.zhang@linux.dev> 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 <paulmck@kernel.org> 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 <qiang.zhang@linux.dev>
> > > > > Signed-off-by: Kunwu Chan <kunwu.chan@gmail.com>
> > > >
> > > > 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().
I am not sure whether or not it is worth checking in lockdep, but the
general rule is (almost) that if synchronize_rcu_atomic() runs in a given
context, then srcu_read_lock_atomic() must be invoked from a context
that is at least as strict. So if synchronize_srcu_atomic() is invoked
with interrupts disabled, then all of the SRCU-atomic readers for that
same srcu_struct structure must also be invoked with interrupts disabled.
For the "almost" part, note that preemption-enabled context is
treated the same as is preemption-disabled context because both
synchronize_srcu_atomic() and srcu_read_lock_atomic() disable preemption.
> > 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.
The rule stated above also prevents this situation. The problem in
the above scenario is that srcu_read_lock_atomic() was invoked with
interrupts enabled, which means that invoking synchronize_srcu_atomic()
for that same srcu_struct structure from the interrupts-disabled IPI
handler is a usage bug.
> > 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.
>
> 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.
If lockdep could check for the rule stated above, that would be quite
nice. ;-)
Thanx, Paul
> 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
> > > > >
> > > >
> > >
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] srcutree: Add lockdep and early-boot checks to synchronize_srcu_atomic()
2026-09-13 10:36 ` KunWu Chan
2026-09-13 18:18 ` Paul E. McKenney
@ 2026-09-14 0:12 ` Zqiang
2026-09-14 2:56 ` KunWu Chan
1 sibling, 1 reply; 11+ messages in thread
From: Zqiang @ 2026-09-14 0:12 UTC (permalink / raw)
To: KunWu Chan, paulmck
Cc: jiangshanlai, josh, rostedt, mathieu.desnoyers, rcu, linux-kernel
>
> On Sun, Sep 13, 2026 at 2:21 PM Zqiang <qiang.zhang@linux.dev> 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 <paulmck@kernel.org> 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 <qiang.zhang@linux.dev>
> > > > Signed-off-by: Kunwu Chan <kunwu.chan@gmail.com>
> > >
> > > 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
>
> 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
> > > >
> > >
> >
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] srcutree: Add lockdep and early-boot checks to synchronize_srcu_atomic()
2026-09-13 18:18 ` Paul E. McKenney
@ 2026-09-14 2:55 ` KunWu Chan
0 siblings, 0 replies; 11+ messages in thread
From: KunWu Chan @ 2026-09-14 2:55 UTC (permalink / raw)
To: paulmck
Cc: Zqiang, jiangshanlai, josh, rostedt, mathieu.desnoyers, rcu,
linux-kernel
On Mon, Sep 14, 2026 at 2:18 AM Paul E. McKenney <paulmck@kernel.org> wrote:
>
> On Sun, Sep 13, 2026 at 06:36:56PM +0800, KunWu Chan wrote:
> > On Sun, Sep 13, 2026 at 2:21 PM Zqiang <qiang.zhang@linux.dev> 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 <paulmck@kernel.org> 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 <qiang.zhang@linux.dev>
> > > > > > Signed-off-by: Kunwu Chan <kunwu.chan@gmail.com>
> > > > >
> > > > > 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().
>
> I am not sure whether or not it is worth checking in lockdep, but the
> general rule is (almost) that if synchronize_rcu_atomic() runs in a given
> context, then srcu_read_lock_atomic() must be invoked from a context
> that is at least as strict. So if synchronize_srcu_atomic() is invoked
> with interrupts disabled, then all of the SRCU-atomic readers for that
> same srcu_struct structure must also be invoked with interrupts disabled.
>
> For the "almost" part, note that preemption-enabled context is
> treated the same as is preemption-disabled context because both
> synchronize_srcu_atomic() and srcu_read_lock_atomic() disable preemption.
>
Thanks, Paul, for the detailed explanation. The context rule makes
the two cases much clearer.
> > > 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.
>
> The rule stated above also prevents this situation. The problem in
> the above scenario is that srcu_read_lock_atomic() was invoked with
> interrupts enabled, which means that invoking synchronize_srcu_atomic()
> for that same srcu_struct structure from the interrupts-disabled IPI
> handler is a usage bug.
The cross-CPU case initially looked like a lockdep limitation to me.
Your context rule clarifies that the usage itself is invalid
regardless:
the reader is in a less strict context than synchronize_srcu_atomic().
The rule is a more direct way of catching the problem than trying to
make lockdep see across CPUs.
The preemption detail is also helpful here, since both
srcu_read_lock_atomic() and synchronize_srcu_atomic() disable
preemption.
>
> > > 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.
> >
> > 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.
>
> If lockdep could check for the rule stated above, that would be quite
> nice. ;-)
I'll investigate how this context rule could be checked by lockdep.
I'll also check the IRQ-state handling in synchronize_srcu_atomic()
based on Zqiang's suggestion.
Thanks for the clarification and for pointing out the direction here.
Thanks,
Kunwu
>
> Thanx, Paul
>
> > 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
> > > > > >
> > > > >
> > > >
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] srcutree: Add lockdep and early-boot checks to synchronize_srcu_atomic()
2026-09-14 0:12 ` Zqiang
@ 2026-09-14 2:56 ` KunWu Chan
0 siblings, 0 replies; 11+ messages in thread
From: KunWu Chan @ 2026-09-14 2:56 UTC (permalink / raw)
To: Zqiang
Cc: paulmck, jiangshanlai, josh, rostedt, mathieu.desnoyers, rcu,
linux-kernel
On Mon, Sep 14, 2026 at 8:12 AM Zqiang <qiang.zhang@linux.dev> wrote:
>
> >
> > On Sun, Sep 13, 2026 at 2:21 PM Zqiang <qiang.zhang@linux.dev> 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 <paulmck@kernel.org> 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 <qiang.zhang@linux.dev>
> > > > > Signed-off-by: Kunwu Chan <kunwu.chan@gmail.com>
> > > >
> > > > 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
> > > > >
> > > >
> > >
> >
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-09-14 2:56 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-11 9:35 [PATCH] srcutree: Add lockdep and early-boot checks to synchronize_srcu_atomic() Kunwu Chan
2026-09-11 16:47 ` Paul E. McKenney
2026-09-12 12:37 ` KunWu Chan
2026-09-12 18:50 ` Paul E. McKenney
2026-09-13 6:21 ` Zqiang
2026-09-13 10:36 ` KunWu Chan
2026-09-13 18:18 ` Paul E. McKenney
2026-09-14 2:55 ` KunWu Chan
2026-09-14 0:12 ` Zqiang
2026-09-14 2:56 ` KunWu Chan
2026-09-13 10:23 ` KunWu Chan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®