kernel/rcu/tree_plugin.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
For Preempt-RT kernel, when enable CONFIG_PROVE_RCU Kconfig,
disable local bh in rcuc kthreads will not affect preempt_count(),
this resulted in the following splat:
WARNING: suspicious RCU usage
kernel/rcu/tree_plugin.h:36 Unsafe read of RCU_NOCB offloaded state!
stack backtrace:
CPU: 0 UID: 0 PID: 22 Comm: rcuc/0
Call Trace:
[ 0.407907] <TASK>
[ 0.407910] dump_stack_lvl+0xbb/0xd0
[ 0.407917] dump_stack+0x14/0x20
[ 0.407920] lockdep_rcu_suspicious+0x133/0x210
[ 0.407932] rcu_rdp_is_offloaded+0x1c3/0x270
[ 0.407939] rcu_core+0x471/0x900
[ 0.407942] ? lockdep_hardirqs_on+0xd5/0x160
[ 0.407954] rcu_cpu_kthread+0x25f/0x870
[ 0.407959] ? __pfx_rcu_cpu_kthread+0x10/0x10
[ 0.407966] smpboot_thread_fn+0x34c/0xa50
[ 0.407970] ? trace_preempt_on+0x54/0x120
[ 0.407977] ? __pfx_smpboot_thread_fn+0x10/0x10
[ 0.407982] kthread+0x40e/0x840
[ 0.407990] ? __pfx_kthread+0x10/0x10
[ 0.407994] ? rt_spin_unlock+0x4e/0xb0
[ 0.407997] ? rt_spin_unlock+0x4e/0xb0
[ 0.408000] ? __pfx_kthread+0x10/0x10
[ 0.408006] ? __pfx_kthread+0x10/0x10
[ 0.408011] ret_from_fork+0x40/0x70
[ 0.408013] ? __pfx_kthread+0x10/0x10
[ 0.408018] ret_from_fork_asm+0x1a/0x30
[ 0.408042] </TASK>
Currently, triggering an rdp offloaded state change need the
corresponding rdp's CPU goes offline, and at this time the rcuc
kthreads has already in parking state. this means the corresponding
rcuc kthreads can safely read offloaded state of rdp while it's
corresponding cpu is online.
This commit therefore add rdp->rcu_cpu_kthread_task check for
Preempt-RT kernels.
Signed-off-by: Zqiang <qiang.zhang1211@gmail.com>
---
kernel/rcu/tree_plugin.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index 003e549f6514..fe728eded36e 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -31,7 +31,9 @@ static bool rcu_rdp_is_offloaded(struct rcu_data *rdp)
lockdep_is_held(&rcu_state.nocb_mutex) ||
(!(IS_ENABLED(CONFIG_PREEMPT_COUNT) && preemptible()) &&
rdp == this_cpu_ptr(&rcu_data)) ||
- rcu_current_is_nocb_kthread(rdp)),
+ rcu_current_is_nocb_kthread(rdp) ||
+ (IS_ENABLED(CONFIG_PREEMPT_RT) &&
+ current == rdp->rcu_cpu_kthread_task)),
"Unsafe read of RCU_NOCB offloaded state"
);
--
2.17.1Le Mon, Apr 28, 2025 at 05:54:03PM +0800, Zqiang a écrit : > For Preempt-RT kernel, when enable CONFIG_PROVE_RCU Kconfig, > disable local bh in rcuc kthreads will not affect preempt_count(), > this resulted in the following splat: > > WARNING: suspicious RCU usage > kernel/rcu/tree_plugin.h:36 Unsafe read of RCU_NOCB offloaded state! > stack backtrace: > CPU: 0 UID: 0 PID: 22 Comm: rcuc/0 > Call Trace: > [ 0.407907] <TASK> > [ 0.407910] dump_stack_lvl+0xbb/0xd0 > [ 0.407917] dump_stack+0x14/0x20 > [ 0.407920] lockdep_rcu_suspicious+0x133/0x210 > [ 0.407932] rcu_rdp_is_offloaded+0x1c3/0x270 > [ 0.407939] rcu_core+0x471/0x900 > [ 0.407942] ? lockdep_hardirqs_on+0xd5/0x160 > [ 0.407954] rcu_cpu_kthread+0x25f/0x870 > [ 0.407959] ? __pfx_rcu_cpu_kthread+0x10/0x10 > [ 0.407966] smpboot_thread_fn+0x34c/0xa50 > [ 0.407970] ? trace_preempt_on+0x54/0x120 > [ 0.407977] ? __pfx_smpboot_thread_fn+0x10/0x10 > [ 0.407982] kthread+0x40e/0x840 > [ 0.407990] ? __pfx_kthread+0x10/0x10 > [ 0.407994] ? rt_spin_unlock+0x4e/0xb0 > [ 0.407997] ? rt_spin_unlock+0x4e/0xb0 > [ 0.408000] ? __pfx_kthread+0x10/0x10 > [ 0.408006] ? __pfx_kthread+0x10/0x10 > [ 0.408011] ret_from_fork+0x40/0x70 > [ 0.408013] ? __pfx_kthread+0x10/0x10 > [ 0.408018] ret_from_fork_asm+0x1a/0x30 > [ 0.408042] </TASK> > > Currently, triggering an rdp offloaded state change need the > corresponding rdp's CPU goes offline, and at this time the rcuc > kthreads has already in parking state. this means the corresponding > rcuc kthreads can safely read offloaded state of rdp while it's > corresponding cpu is online. > > This commit therefore add rdp->rcu_cpu_kthread_task check for > Preempt-RT kernels. > > Signed-off-by: Zqiang <qiang.zhang1211@gmail.com> > --- > kernel/rcu/tree_plugin.h | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h > index 003e549f6514..fe728eded36e 100644 > --- a/kernel/rcu/tree_plugin.h > +++ b/kernel/rcu/tree_plugin.h > @@ -31,7 +31,9 @@ static bool rcu_rdp_is_offloaded(struct rcu_data *rdp) > lockdep_is_held(&rcu_state.nocb_mutex) || > (!(IS_ENABLED(CONFIG_PREEMPT_COUNT) && preemptible()) && > rdp == this_cpu_ptr(&rcu_data)) || > - rcu_current_is_nocb_kthread(rdp)), > + rcu_current_is_nocb_kthread(rdp) || > + (IS_ENABLED(CONFIG_PREEMPT_RT) && > + current == rdp->rcu_cpu_kthread_task)), Isn't it safe also on !CONFIG_PREEMPT_RT ? Thanks. > "Unsafe read of RCU_NOCB offloaded state" > ); > > -- > 2.17.1 > -- Frederic Weisbecker SUSE Labs
>
> Le Mon, Apr 28, 2025 at 05:54:03PM +0800, Zqiang a écrit :
> > For Preempt-RT kernel, when enable CONFIG_PROVE_RCU Kconfig,
> > disable local bh in rcuc kthreads will not affect preempt_count(),
> > this resulted in the following splat:
> >
> > WARNING: suspicious RCU usage
> > kernel/rcu/tree_plugin.h:36 Unsafe read of RCU_NOCB offloaded state!
> > stack backtrace:
> > CPU: 0 UID: 0 PID: 22 Comm: rcuc/0
> > Call Trace:
> > [ 0.407907] <TASK>
> > [ 0.407910] dump_stack_lvl+0xbb/0xd0
> > [ 0.407917] dump_stack+0x14/0x20
> > [ 0.407920] lockdep_rcu_suspicious+0x133/0x210
> > [ 0.407932] rcu_rdp_is_offloaded+0x1c3/0x270
> > [ 0.407939] rcu_core+0x471/0x900
> > [ 0.407942] ? lockdep_hardirqs_on+0xd5/0x160
> > [ 0.407954] rcu_cpu_kthread+0x25f/0x870
> > [ 0.407959] ? __pfx_rcu_cpu_kthread+0x10/0x10
> > [ 0.407966] smpboot_thread_fn+0x34c/0xa50
> > [ 0.407970] ? trace_preempt_on+0x54/0x120
> > [ 0.407977] ? __pfx_smpboot_thread_fn+0x10/0x10
> > [ 0.407982] kthread+0x40e/0x840
> > [ 0.407990] ? __pfx_kthread+0x10/0x10
> > [ 0.407994] ? rt_spin_unlock+0x4e/0xb0
> > [ 0.407997] ? rt_spin_unlock+0x4e/0xb0
> > [ 0.408000] ? __pfx_kthread+0x10/0x10
> > [ 0.408006] ? __pfx_kthread+0x10/0x10
> > [ 0.408011] ret_from_fork+0x40/0x70
> > [ 0.408013] ? __pfx_kthread+0x10/0x10
> > [ 0.408018] ret_from_fork_asm+0x1a/0x30
> > [ 0.408042] </TASK>
> >
> > Currently, triggering an rdp offloaded state change need the
> > corresponding rdp's CPU goes offline, and at this time the rcuc
> > kthreads has already in parking state. this means the corresponding
> > rcuc kthreads can safely read offloaded state of rdp while it's
> > corresponding cpu is online.
> >
> > This commit therefore add rdp->rcu_cpu_kthread_task check for
> > Preempt-RT kernels.
> >
> > Signed-off-by: Zqiang <qiang.zhang1211@gmail.com>
> > ---
> > kernel/rcu/tree_plugin.h | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
> > index 003e549f6514..fe728eded36e 100644
> > --- a/kernel/rcu/tree_plugin.h
> > +++ b/kernel/rcu/tree_plugin.h
> > @@ -31,7 +31,9 @@ static bool rcu_rdp_is_offloaded(struct rcu_data *rdp)
> > lockdep_is_held(&rcu_state.nocb_mutex) ||
> > (!(IS_ENABLED(CONFIG_PREEMPT_COUNT) && preemptible()) &&
> > rdp == this_cpu_ptr(&rcu_data)) ||
> > - rcu_current_is_nocb_kthread(rdp)),
> > + rcu_current_is_nocb_kthread(rdp) ||
> > + (IS_ENABLED(CONFIG_PREEMPT_RT) &&
> > + current == rdp->rcu_cpu_kthread_task)),
>
> Isn't it safe also on !CONFIG_PREEMPT_RT ?
For !CONFIG_PREEMPT_RT and in rcuc kthreads, it's also safe,
but the following check will passed :
(!(IS_ENABLED(CONFIG_PREEMPT_COUNT) && preemptible()) &&
rdp == this_cpu_ptr(&rcu_data))
Thanks
Zqiang
>
> Thanks.
>
> > "Unsafe read of RCU_NOCB offloaded state"
> > );
> >
> > --
> > 2.17.1
> >
>
> --
> Frederic Weisbecker
> SUSE Labs
> > > > > Le Mon, Apr 28, 2025 at 05:54:03PM +0800, Zqiang a écrit : > > > For Preempt-RT kernel, when enable CONFIG_PROVE_RCU Kconfig, > > > disable local bh in rcuc kthreads will not affect preempt_count(), > > > this resulted in the following splat: > > > > > > WARNING: suspicious RCU usage > > > kernel/rcu/tree_plugin.h:36 Unsafe read of RCU_NOCB offloaded state! > > > stack backtrace: > > > CPU: 0 UID: 0 PID: 22 Comm: rcuc/0 > > > Call Trace: > > > [ 0.407907] <TASK> > > > [ 0.407910] dump_stack_lvl+0xbb/0xd0 > > > [ 0.407917] dump_stack+0x14/0x20 > > > [ 0.407920] lockdep_rcu_suspicious+0x133/0x210 > > > [ 0.407932] rcu_rdp_is_offloaded+0x1c3/0x270 > > > [ 0.407939] rcu_core+0x471/0x900 > > > [ 0.407942] ? lockdep_hardirqs_on+0xd5/0x160 > > > [ 0.407954] rcu_cpu_kthread+0x25f/0x870 > > > [ 0.407959] ? __pfx_rcu_cpu_kthread+0x10/0x10 > > > [ 0.407966] smpboot_thread_fn+0x34c/0xa50 > > > [ 0.407970] ? trace_preempt_on+0x54/0x120 > > > [ 0.407977] ? __pfx_smpboot_thread_fn+0x10/0x10 > > > [ 0.407982] kthread+0x40e/0x840 > > > [ 0.407990] ? __pfx_kthread+0x10/0x10 > > > [ 0.407994] ? rt_spin_unlock+0x4e/0xb0 > > > [ 0.407997] ? rt_spin_unlock+0x4e/0xb0 > > > [ 0.408000] ? __pfx_kthread+0x10/0x10 > > > [ 0.408006] ? __pfx_kthread+0x10/0x10 > > > [ 0.408011] ret_from_fork+0x40/0x70 > > > [ 0.408013] ? __pfx_kthread+0x10/0x10 > > > [ 0.408018] ret_from_fork_asm+0x1a/0x30 > > > [ 0.408042] </TASK> > > > > > > Currently, triggering an rdp offloaded state change need the > > > corresponding rdp's CPU goes offline, and at this time the rcuc > > > kthreads has already in parking state. this means the corresponding > > > rcuc kthreads can safely read offloaded state of rdp while it's > > > corresponding cpu is online. > > > > > > This commit therefore add rdp->rcu_cpu_kthread_task check for > > > Preempt-RT kernels. > > > > > > Signed-off-by: Zqiang <qiang.zhang1211@gmail.com> > > > --- > > > kernel/rcu/tree_plugin.h | 4 +++- > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h > > > index 003e549f6514..fe728eded36e 100644 > > > --- a/kernel/rcu/tree_plugin.h > > > +++ b/kernel/rcu/tree_plugin.h > > > @@ -31,7 +31,9 @@ static bool rcu_rdp_is_offloaded(struct rcu_data *rdp) > > > lockdep_is_held(&rcu_state.nocb_mutex) || > > > (!(IS_ENABLED(CONFIG_PREEMPT_COUNT) && preemptible()) && > > > rdp == this_cpu_ptr(&rcu_data)) || > > > - rcu_current_is_nocb_kthread(rdp)), > > > + rcu_current_is_nocb_kthread(rdp) || > > > + (IS_ENABLED(CONFIG_PREEMPT_RT) && > > > + current == rdp->rcu_cpu_kthread_task)), > > > > Isn't it safe also on !CONFIG_PREEMPT_RT ? How about the following? (current == rdp->rcu_cpu_kthread_task && in_task()) Thanks Zqiang > > For !CONFIG_PREEMPT_RT and in rcuc kthreads, it's also safe, > but the following check will passed : > > (!(IS_ENABLED(CONFIG_PREEMPT_COUNT) && preemptible()) && > rdp == this_cpu_ptr(&rcu_data)) > > Thanks > Zqiang > > > > > > Thanks. > > > > > "Unsafe read of RCU_NOCB offloaded state" > > > ); > > > > > > -- > > > 2.17.1 > > > > > > > -- > > Frederic Weisbecker > > SUSE Labs
On 4/28/2025 6:59 AM, Z qiang wrote: >> >> Le Mon, Apr 28, 2025 at 05:54:03PM +0800, Zqiang a écrit : >>> For Preempt-RT kernel, when enable CONFIG_PROVE_RCU Kconfig, >>> disable local bh in rcuc kthreads will not affect preempt_count(), >>> this resulted in the following splat: >>> >>> WARNING: suspicious RCU usage >>> kernel/rcu/tree_plugin.h:36 Unsafe read of RCU_NOCB offloaded state! >>> stack backtrace: >>> CPU: 0 UID: 0 PID: 22 Comm: rcuc/0 >>> Call Trace: >>> [ 0.407907] <TASK> >>> [ 0.407910] dump_stack_lvl+0xbb/0xd0 >>> [ 0.407917] dump_stack+0x14/0x20 >>> [ 0.407920] lockdep_rcu_suspicious+0x133/0x210 >>> [ 0.407932] rcu_rdp_is_offloaded+0x1c3/0x270 >>> [ 0.407939] rcu_core+0x471/0x900 >>> [ 0.407942] ? lockdep_hardirqs_on+0xd5/0x160 >>> [ 0.407954] rcu_cpu_kthread+0x25f/0x870 >>> [ 0.407959] ? __pfx_rcu_cpu_kthread+0x10/0x10 >>> [ 0.407966] smpboot_thread_fn+0x34c/0xa50 >>> [ 0.407970] ? trace_preempt_on+0x54/0x120 >>> [ 0.407977] ? __pfx_smpboot_thread_fn+0x10/0x10 >>> [ 0.407982] kthread+0x40e/0x840 >>> [ 0.407990] ? __pfx_kthread+0x10/0x10 >>> [ 0.407994] ? rt_spin_unlock+0x4e/0xb0 >>> [ 0.407997] ? rt_spin_unlock+0x4e/0xb0 >>> [ 0.408000] ? __pfx_kthread+0x10/0x10 >>> [ 0.408006] ? __pfx_kthread+0x10/0x10 >>> [ 0.408011] ret_from_fork+0x40/0x70 >>> [ 0.408013] ? __pfx_kthread+0x10/0x10 >>> [ 0.408018] ret_from_fork_asm+0x1a/0x30 >>> [ 0.408042] </TASK> >>> >>> Currently, triggering an rdp offloaded state change need the >>> corresponding rdp's CPU goes offline, and at this time the rcuc >>> kthreads has already in parking state. this means the corresponding >>> rcuc kthreads can safely read offloaded state of rdp while it's >>> corresponding cpu is online. >>> >>> This commit therefore add rdp->rcu_cpu_kthread_task check for >>> Preempt-RT kernels. >>> >>> Signed-off-by: Zqiang <qiang.zhang1211@gmail.com> >>> --- >>> kernel/rcu/tree_plugin.h | 4 +++- >>> 1 file changed, 3 insertions(+), 1 deletion(-) >>> >>> diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h >>> index 003e549f6514..fe728eded36e 100644 >>> --- a/kernel/rcu/tree_plugin.h >>> +++ b/kernel/rcu/tree_plugin.h >>> @@ -31,7 +31,9 @@ static bool rcu_rdp_is_offloaded(struct rcu_data *rdp) >>> lockdep_is_held(&rcu_state.nocb_mutex) || >>> (!(IS_ENABLED(CONFIG_PREEMPT_COUNT) && preemptible()) && >>> rdp == this_cpu_ptr(&rcu_data)) || >>> - rcu_current_is_nocb_kthread(rdp)), >>> + rcu_current_is_nocb_kthread(rdp) || >>> + (IS_ENABLED(CONFIG_PREEMPT_RT) && >>> + current == rdp->rcu_cpu_kthread_task)), >> >> Isn't it safe also on !CONFIG_PREEMPT_RT ? > > For !CONFIG_PREEMPT_RT and in rcuc kthreads, it's also safe, > but the following check will passed : > > (!(IS_ENABLED(CONFIG_PREEMPT_COUNT) && preemptible()) && > rdp == this_cpu_ptr(&rcu_data)) I think the fact that it already passes for !PREEMPT_RT does not matter, because it simplifies the code so drop the PREEMPT_RT check? Or will softirq_count() not work? It appears to have special casing for PREEMPT_RT's local_bh_disable(): ( ( !(IS_ENABLED(CONFIG_PREEMPT_COUNT) && preemptible()) || softirq_count() ) && rdp == this_cpu_ptr(&rcu_data)) ) thanks, - Joel
> > > > On 4/28/2025 6:59 AM, Z qiang wrote: > >> > >> Le Mon, Apr 28, 2025 at 05:54:03PM +0800, Zqiang a écrit : > >>> For Preempt-RT kernel, when enable CONFIG_PROVE_RCU Kconfig, > >>> disable local bh in rcuc kthreads will not affect preempt_count(), > >>> this resulted in the following splat: > >>> > >>> WARNING: suspicious RCU usage > >>> kernel/rcu/tree_plugin.h:36 Unsafe read of RCU_NOCB offloaded state! > >>> stack backtrace: > >>> CPU: 0 UID: 0 PID: 22 Comm: rcuc/0 > >>> Call Trace: > >>> [ 0.407907] <TASK> > >>> [ 0.407910] dump_stack_lvl+0xbb/0xd0 > >>> [ 0.407917] dump_stack+0x14/0x20 > >>> [ 0.407920] lockdep_rcu_suspicious+0x133/0x210 > >>> [ 0.407932] rcu_rdp_is_offloaded+0x1c3/0x270 > >>> [ 0.407939] rcu_core+0x471/0x900 > >>> [ 0.407942] ? lockdep_hardirqs_on+0xd5/0x160 > >>> [ 0.407954] rcu_cpu_kthread+0x25f/0x870 > >>> [ 0.407959] ? __pfx_rcu_cpu_kthread+0x10/0x10 > >>> [ 0.407966] smpboot_thread_fn+0x34c/0xa50 > >>> [ 0.407970] ? trace_preempt_on+0x54/0x120 > >>> [ 0.407977] ? __pfx_smpboot_thread_fn+0x10/0x10 > >>> [ 0.407982] kthread+0x40e/0x840 > >>> [ 0.407990] ? __pfx_kthread+0x10/0x10 > >>> [ 0.407994] ? rt_spin_unlock+0x4e/0xb0 > >>> [ 0.407997] ? rt_spin_unlock+0x4e/0xb0 > >>> [ 0.408000] ? __pfx_kthread+0x10/0x10 > >>> [ 0.408006] ? __pfx_kthread+0x10/0x10 > >>> [ 0.408011] ret_from_fork+0x40/0x70 > >>> [ 0.408013] ? __pfx_kthread+0x10/0x10 > >>> [ 0.408018] ret_from_fork_asm+0x1a/0x30 > >>> [ 0.408042] </TASK> > >>> > >>> Currently, triggering an rdp offloaded state change need the > >>> corresponding rdp's CPU goes offline, and at this time the rcuc > >>> kthreads has already in parking state. this means the corresponding > >>> rcuc kthreads can safely read offloaded state of rdp while it's > >>> corresponding cpu is online. > >>> > >>> This commit therefore add rdp->rcu_cpu_kthread_task check for > >>> Preempt-RT kernels. > >>> > >>> Signed-off-by: Zqiang <qiang.zhang1211@gmail.com> > >>> --- > >>> kernel/rcu/tree_plugin.h | 4 +++- > >>> 1 file changed, 3 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h > >>> index 003e549f6514..fe728eded36e 100644 > >>> --- a/kernel/rcu/tree_plugin.h > >>> +++ b/kernel/rcu/tree_plugin.h > >>> @@ -31,7 +31,9 @@ static bool rcu_rdp_is_offloaded(struct rcu_data *rdp) > >>> lockdep_is_held(&rcu_state.nocb_mutex) || > >>> (!(IS_ENABLED(CONFIG_PREEMPT_COUNT) && preemptible()) && > >>> rdp == this_cpu_ptr(&rcu_data)) || > >>> - rcu_current_is_nocb_kthread(rdp)), > >>> + rcu_current_is_nocb_kthread(rdp) || > >>> + (IS_ENABLED(CONFIG_PREEMPT_RT) && > >>> + current == rdp->rcu_cpu_kthread_task)), > >> > >> Isn't it safe also on !CONFIG_PREEMPT_RT ? > > > > For !CONFIG_PREEMPT_RT and in rcuc kthreads, it's also safe, > > but the following check will passed : > > > > (!(IS_ENABLED(CONFIG_PREEMPT_COUNT) && preemptible()) && > > rdp == this_cpu_ptr(&rcu_data)) > > I think the fact that it already passes for !PREEMPT_RT does not matter, because > it simplifies the code so drop the PREEMPT_RT check? > > Or will softirq_count() not work? It appears to have special casing for > PREEMPT_RT's local_bh_disable(): > > ( ( !(IS_ENABLED(CONFIG_PREEMPT_COUNT) && preemptible()) || softirq_count() ) > && rdp == this_cpu_ptr(&rcu_data)) ) Thank you for Joel's reply, I also willing to accept such modifications and resend :) . Thanks Zqiang > > thanks, > > - Joel > > > > >
On 4/30/2025 10:57 AM, Z qiang wrote: >> >> >> >> On 4/28/2025 6:59 AM, Z qiang wrote: >>>> >>>> Le Mon, Apr 28, 2025 at 05:54:03PM +0800, Zqiang a écrit : >>>>> For Preempt-RT kernel, when enable CONFIG_PROVE_RCU Kconfig, >>>>> disable local bh in rcuc kthreads will not affect preempt_count(), >>>>> this resulted in the following splat: >>>>> >>>>> WARNING: suspicious RCU usage >>>>> kernel/rcu/tree_plugin.h:36 Unsafe read of RCU_NOCB offloaded state! >>>>> stack backtrace: >>>>> CPU: 0 UID: 0 PID: 22 Comm: rcuc/0 >>>>> Call Trace: >>>>> [ 0.407907] <TASK> >>>>> [ 0.407910] dump_stack_lvl+0xbb/0xd0 >>>>> [ 0.407917] dump_stack+0x14/0x20 >>>>> [ 0.407920] lockdep_rcu_suspicious+0x133/0x210 >>>>> [ 0.407932] rcu_rdp_is_offloaded+0x1c3/0x270 >>>>> [ 0.407939] rcu_core+0x471/0x900 >>>>> [ 0.407942] ? lockdep_hardirqs_on+0xd5/0x160 >>>>> [ 0.407954] rcu_cpu_kthread+0x25f/0x870 >>>>> [ 0.407959] ? __pfx_rcu_cpu_kthread+0x10/0x10 >>>>> [ 0.407966] smpboot_thread_fn+0x34c/0xa50 >>>>> [ 0.407970] ? trace_preempt_on+0x54/0x120 >>>>> [ 0.407977] ? __pfx_smpboot_thread_fn+0x10/0x10 >>>>> [ 0.407982] kthread+0x40e/0x840 >>>>> [ 0.407990] ? __pfx_kthread+0x10/0x10 >>>>> [ 0.407994] ? rt_spin_unlock+0x4e/0xb0 >>>>> [ 0.407997] ? rt_spin_unlock+0x4e/0xb0 >>>>> [ 0.408000] ? __pfx_kthread+0x10/0x10 >>>>> [ 0.408006] ? __pfx_kthread+0x10/0x10 >>>>> [ 0.408011] ret_from_fork+0x40/0x70 >>>>> [ 0.408013] ? __pfx_kthread+0x10/0x10 >>>>> [ 0.408018] ret_from_fork_asm+0x1a/0x30 >>>>> [ 0.408042] </TASK> >>>>> >>>>> Currently, triggering an rdp offloaded state change need the >>>>> corresponding rdp's CPU goes offline, and at this time the rcuc >>>>> kthreads has already in parking state. this means the corresponding >>>>> rcuc kthreads can safely read offloaded state of rdp while it's >>>>> corresponding cpu is online. >>>>> >>>>> This commit therefore add rdp->rcu_cpu_kthread_task check for >>>>> Preempt-RT kernels. >>>>> >>>>> Signed-off-by: Zqiang <qiang.zhang1211@gmail.com> >>>>> --- >>>>> kernel/rcu/tree_plugin.h | 4 +++- >>>>> 1 file changed, 3 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h >>>>> index 003e549f6514..fe728eded36e 100644 >>>>> --- a/kernel/rcu/tree_plugin.h >>>>> +++ b/kernel/rcu/tree_plugin.h >>>>> @@ -31,7 +31,9 @@ static bool rcu_rdp_is_offloaded(struct rcu_data *rdp) >>>>> lockdep_is_held(&rcu_state.nocb_mutex) || >>>>> (!(IS_ENABLED(CONFIG_PREEMPT_COUNT) && preemptible()) && >>>>> rdp == this_cpu_ptr(&rcu_data)) || >>>>> - rcu_current_is_nocb_kthread(rdp)), >>>>> + rcu_current_is_nocb_kthread(rdp) || >>>>> + (IS_ENABLED(CONFIG_PREEMPT_RT) && >>>>> + current == rdp->rcu_cpu_kthread_task)), >>>> >>>> Isn't it safe also on !CONFIG_PREEMPT_RT ? >>> >>> For !CONFIG_PREEMPT_RT and in rcuc kthreads, it's also safe, >>> but the following check will passed : >>> >>> (!(IS_ENABLED(CONFIG_PREEMPT_COUNT) && preemptible()) && >>> rdp == this_cpu_ptr(&rcu_data)) >> >> I think the fact that it already passes for !PREEMPT_RT does not matter, because >> it simplifies the code so drop the PREEMPT_RT check? >> >> Or will softirq_count() not work? It appears to have special casing for >> PREEMPT_RT's local_bh_disable(): >> >> ( ( !(IS_ENABLED(CONFIG_PREEMPT_COUNT) && preemptible()) || softirq_count() ) >> && rdp == this_cpu_ptr(&rcu_data)) ) > > Thank you for Joel's reply, I also willing to accept such > modifications and resend :) . Thanks, I am Ok with either approach whichever you and Frederic together decide. I can then pull this in for the v6.16 merge window once you resend, thanks! - Joel
On 4/30/2025 12:14 PM, Joel Fernandes wrote: > > > On 4/30/2025 10:57 AM, Z qiang wrote: >>> >>> >>> >>> On 4/28/2025 6:59 AM, Z qiang wrote: >>>>> >>>>> Le Mon, Apr 28, 2025 at 05:54:03PM +0800, Zqiang a écrit : >>>>>> For Preempt-RT kernel, when enable CONFIG_PROVE_RCU Kconfig, >>>>>> disable local bh in rcuc kthreads will not affect preempt_count(), >>>>>> this resulted in the following splat: >>>>>> >>>>>> WARNING: suspicious RCU usage >>>>>> kernel/rcu/tree_plugin.h:36 Unsafe read of RCU_NOCB offloaded state! >>>>>> stack backtrace: >>>>>> CPU: 0 UID: 0 PID: 22 Comm: rcuc/0 >>>>>> Call Trace: >>>>>> [ 0.407907] <TASK> >>>>>> [ 0.407910] dump_stack_lvl+0xbb/0xd0 >>>>>> [ 0.407917] dump_stack+0x14/0x20 >>>>>> [ 0.407920] lockdep_rcu_suspicious+0x133/0x210 >>>>>> [ 0.407932] rcu_rdp_is_offloaded+0x1c3/0x270 >>>>>> [ 0.407939] rcu_core+0x471/0x900 >>>>>> [ 0.407942] ? lockdep_hardirqs_on+0xd5/0x160 >>>>>> [ 0.407954] rcu_cpu_kthread+0x25f/0x870 >>>>>> [ 0.407959] ? __pfx_rcu_cpu_kthread+0x10/0x10 >>>>>> [ 0.407966] smpboot_thread_fn+0x34c/0xa50 >>>>>> [ 0.407970] ? trace_preempt_on+0x54/0x120 >>>>>> [ 0.407977] ? __pfx_smpboot_thread_fn+0x10/0x10 >>>>>> [ 0.407982] kthread+0x40e/0x840 >>>>>> [ 0.407990] ? __pfx_kthread+0x10/0x10 >>>>>> [ 0.407994] ? rt_spin_unlock+0x4e/0xb0 >>>>>> [ 0.407997] ? rt_spin_unlock+0x4e/0xb0 >>>>>> [ 0.408000] ? __pfx_kthread+0x10/0x10 >>>>>> [ 0.408006] ? __pfx_kthread+0x10/0x10 >>>>>> [ 0.408011] ret_from_fork+0x40/0x70 >>>>>> [ 0.408013] ? __pfx_kthread+0x10/0x10 >>>>>> [ 0.408018] ret_from_fork_asm+0x1a/0x30 >>>>>> [ 0.408042] </TASK> >>>>>> >>>>>> Currently, triggering an rdp offloaded state change need the >>>>>> corresponding rdp's CPU goes offline, and at this time the rcuc >>>>>> kthreads has already in parking state. this means the corresponding >>>>>> rcuc kthreads can safely read offloaded state of rdp while it's >>>>>> corresponding cpu is online. >>>>>> >>>>>> This commit therefore add rdp->rcu_cpu_kthread_task check for >>>>>> Preempt-RT kernels. >>>>>> >>>>>> Signed-off-by: Zqiang <qiang.zhang1211@gmail.com> >>>>>> --- >>>>>> kernel/rcu/tree_plugin.h | 4 +++- >>>>>> 1 file changed, 3 insertions(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h >>>>>> index 003e549f6514..fe728eded36e 100644 >>>>>> --- a/kernel/rcu/tree_plugin.h >>>>>> +++ b/kernel/rcu/tree_plugin.h >>>>>> @@ -31,7 +31,9 @@ static bool rcu_rdp_is_offloaded(struct rcu_data *rdp) >>>>>> lockdep_is_held(&rcu_state.nocb_mutex) || >>>>>> (!(IS_ENABLED(CONFIG_PREEMPT_COUNT) && preemptible()) && >>>>>> rdp == this_cpu_ptr(&rcu_data)) || >>>>>> - rcu_current_is_nocb_kthread(rdp)), >>>>>> + rcu_current_is_nocb_kthread(rdp) || >>>>>> + (IS_ENABLED(CONFIG_PREEMPT_RT) && >>>>>> + current == rdp->rcu_cpu_kthread_task)), >>>>> >>>>> Isn't it safe also on !CONFIG_PREEMPT_RT ? >>>> >>>> For !CONFIG_PREEMPT_RT and in rcuc kthreads, it's also safe, >>>> but the following check will passed : >>>> >>>> (!(IS_ENABLED(CONFIG_PREEMPT_COUNT) && preemptible()) && >>>> rdp == this_cpu_ptr(&rcu_data)) >>> >>> I think the fact that it already passes for !PREEMPT_RT does not matter, because >>> it simplifies the code so drop the PREEMPT_RT check? >>> >>> Or will softirq_count() not work? It appears to have special casing for >>> PREEMPT_RT's local_bh_disable(): >>> >>> ( ( !(IS_ENABLED(CONFIG_PREEMPT_COUNT) && preemptible()) || softirq_count() ) >>> && rdp == this_cpu_ptr(&rcu_data)) ) >> >> Thank you for Joel's reply, I also willing to accept such >> modifications and resend :) . > Thanks, I am Ok with either approach whichever you and Frederic together decide. > I can then pull this in for the v6.16 merge window once you resend, thanks! > Frederic, there are a couple of ways we can move forward hear. Does the softirq_count() approach sound good to you? If yes, I can fixup the patch myself. I am also Ok at this point to take it in for 6.16, though I've also stored it in my rcu/dev branch for Neeraj's 6.17 PR, just in case :) - Joel
> > > > On 4/30/2025 12:14 PM, Joel Fernandes wrote: > > > > > > On 4/30/2025 10:57 AM, Z qiang wrote: > >>> > >>> > >>> > >>> On 4/28/2025 6:59 AM, Z qiang wrote: > >>>>> > >>>>> Le Mon, Apr 28, 2025 at 05:54:03PM +0800, Zqiang a écrit : > >>>>>> For Preempt-RT kernel, when enable CONFIG_PROVE_RCU Kconfig, > >>>>>> disable local bh in rcuc kthreads will not affect preempt_count(), > >>>>>> this resulted in the following splat: > >>>>>> > >>>>>> WARNING: suspicious RCU usage > >>>>>> kernel/rcu/tree_plugin.h:36 Unsafe read of RCU_NOCB offloaded state! > >>>>>> stack backtrace: > >>>>>> CPU: 0 UID: 0 PID: 22 Comm: rcuc/0 > >>>>>> Call Trace: > >>>>>> [ 0.407907] <TASK> > >>>>>> [ 0.407910] dump_stack_lvl+0xbb/0xd0 > >>>>>> [ 0.407917] dump_stack+0x14/0x20 > >>>>>> [ 0.407920] lockdep_rcu_suspicious+0x133/0x210 > >>>>>> [ 0.407932] rcu_rdp_is_offloaded+0x1c3/0x270 > >>>>>> [ 0.407939] rcu_core+0x471/0x900 > >>>>>> [ 0.407942] ? lockdep_hardirqs_on+0xd5/0x160 > >>>>>> [ 0.407954] rcu_cpu_kthread+0x25f/0x870 > >>>>>> [ 0.407959] ? __pfx_rcu_cpu_kthread+0x10/0x10 > >>>>>> [ 0.407966] smpboot_thread_fn+0x34c/0xa50 > >>>>>> [ 0.407970] ? trace_preempt_on+0x54/0x120 > >>>>>> [ 0.407977] ? __pfx_smpboot_thread_fn+0x10/0x10 > >>>>>> [ 0.407982] kthread+0x40e/0x840 > >>>>>> [ 0.407990] ? __pfx_kthread+0x10/0x10 > >>>>>> [ 0.407994] ? rt_spin_unlock+0x4e/0xb0 > >>>>>> [ 0.407997] ? rt_spin_unlock+0x4e/0xb0 > >>>>>> [ 0.408000] ? __pfx_kthread+0x10/0x10 > >>>>>> [ 0.408006] ? __pfx_kthread+0x10/0x10 > >>>>>> [ 0.408011] ret_from_fork+0x40/0x70 > >>>>>> [ 0.408013] ? __pfx_kthread+0x10/0x10 > >>>>>> [ 0.408018] ret_from_fork_asm+0x1a/0x30 > >>>>>> [ 0.408042] </TASK> > >>>>>> > >>>>>> Currently, triggering an rdp offloaded state change need the > >>>>>> corresponding rdp's CPU goes offline, and at this time the rcuc > >>>>>> kthreads has already in parking state. this means the corresponding > >>>>>> rcuc kthreads can safely read offloaded state of rdp while it's > >>>>>> corresponding cpu is online. > >>>>>> > >>>>>> This commit therefore add rdp->rcu_cpu_kthread_task check for > >>>>>> Preempt-RT kernels. > >>>>>> > >>>>>> Signed-off-by: Zqiang <qiang.zhang1211@gmail.com> > >>>>>> --- > >>>>>> kernel/rcu/tree_plugin.h | 4 +++- > >>>>>> 1 file changed, 3 insertions(+), 1 deletion(-) > >>>>>> > >>>>>> diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h > >>>>>> index 003e549f6514..fe728eded36e 100644 > >>>>>> --- a/kernel/rcu/tree_plugin.h > >>>>>> +++ b/kernel/rcu/tree_plugin.h > >>>>>> @@ -31,7 +31,9 @@ static bool rcu_rdp_is_offloaded(struct rcu_data *rdp) > >>>>>> lockdep_is_held(&rcu_state.nocb_mutex) || > >>>>>> (!(IS_ENABLED(CONFIG_PREEMPT_COUNT) && preemptible()) && > >>>>>> rdp == this_cpu_ptr(&rcu_data)) || > >>>>>> - rcu_current_is_nocb_kthread(rdp)), > >>>>>> + rcu_current_is_nocb_kthread(rdp) || > >>>>>> + (IS_ENABLED(CONFIG_PREEMPT_RT) && > >>>>>> + current == rdp->rcu_cpu_kthread_task)), > >>>>> > >>>>> Isn't it safe also on !CONFIG_PREEMPT_RT ? > >>>> > >>>> For !CONFIG_PREEMPT_RT and in rcuc kthreads, it's also safe, > >>>> but the following check will passed : > >>>> > >>>> (!(IS_ENABLED(CONFIG_PREEMPT_COUNT) && preemptible()) && > >>>> rdp == this_cpu_ptr(&rcu_data)) > >>> > >>> I think the fact that it already passes for !PREEMPT_RT does not matter, because > >>> it simplifies the code so drop the PREEMPT_RT check? > >>> > >>> Or will softirq_count() not work? It appears to have special casing for > >>> PREEMPT_RT's local_bh_disable(): > >>> > >>> ( ( !(IS_ENABLED(CONFIG_PREEMPT_COUNT) && preemptible()) || softirq_count() ) > >>> && rdp == this_cpu_ptr(&rcu_data)) ) > >> > >> Thank you for Joel's reply, I also willing to accept such > >> modifications and resend :) . > > Thanks, I am Ok with either approach whichever you and Frederic together decide. > > I can then pull this in for the v6.16 merge window once you resend, thanks! > > > > Frederic, there are a couple of ways we can move forward hear. Does the > softirq_count() approach sound good to you? If yes, I can fixup the patch myself. Hello, Joel If you send a patch to fix it, I'd be happy, you can add me as the Reported-by ;) Thanks Zqiang > > I am also Ok at this point to take it in for 6.16, though I've also stored it in > my rcu/dev branch for Neeraj's 6.17 PR, just in case :) > > - Joel > >
> On May 6, 2025, at 2:26 AM, Z qiang <qiang.zhang1211@gmail.com> wrote: > > >> >> >> >> >>> On 4/30/2025 12:14 PM, Joel Fernandes wrote: >>> >>> >>> On 4/30/2025 10:57 AM, Z qiang wrote: >>>>> >>>>> >>>>> >>>>> On 4/28/2025 6:59 AM, Z qiang wrote: >>>>>>> >>>>>>> Le Mon, Apr 28, 2025 at 05:54:03PM +0800, Zqiang a écrit : >>>>>>>> For Preempt-RT kernel, when enable CONFIG_PROVE_RCU Kconfig, >>>>>>>> disable local bh in rcuc kthreads will not affect preempt_count(), >>>>>>>> this resulted in the following splat: >>>>>>>> >>>>>>>> WARNING: suspicious RCU usage >>>>>>>> kernel/rcu/tree_plugin.h:36 Unsafe read of RCU_NOCB offloaded state! >>>>>>>> stack backtrace: >>>>>>>> CPU: 0 UID: 0 PID: 22 Comm: rcuc/0 >>>>>>>> Call Trace: >>>>>>>> [ 0.407907] <TASK> >>>>>>>> [ 0.407910] dump_stack_lvl+0xbb/0xd0 >>>>>>>> [ 0.407917] dump_stack+0x14/0x20 >>>>>>>> [ 0.407920] lockdep_rcu_suspicious+0x133/0x210 >>>>>>>> [ 0.407932] rcu_rdp_is_offloaded+0x1c3/0x270 >>>>>>>> [ 0.407939] rcu_core+0x471/0x900 >>>>>>>> [ 0.407942] ? lockdep_hardirqs_on+0xd5/0x160 >>>>>>>> [ 0.407954] rcu_cpu_kthread+0x25f/0x870 >>>>>>>> [ 0.407959] ? __pfx_rcu_cpu_kthread+0x10/0x10 >>>>>>>> [ 0.407966] smpboot_thread_fn+0x34c/0xa50 >>>>>>>> [ 0.407970] ? trace_preempt_on+0x54/0x120 >>>>>>>> [ 0.407977] ? __pfx_smpboot_thread_fn+0x10/0x10 >>>>>>>> [ 0.407982] kthread+0x40e/0x840 >>>>>>>> [ 0.407990] ? __pfx_kthread+0x10/0x10 >>>>>>>> [ 0.407994] ? rt_spin_unlock+0x4e/0xb0 >>>>>>>> [ 0.407997] ? rt_spin_unlock+0x4e/0xb0 >>>>>>>> [ 0.408000] ? __pfx_kthread+0x10/0x10 >>>>>>>> [ 0.408006] ? __pfx_kthread+0x10/0x10 >>>>>>>> [ 0.408011] ret_from_fork+0x40/0x70 >>>>>>>> [ 0.408013] ? __pfx_kthread+0x10/0x10 >>>>>>>> [ 0.408018] ret_from_fork_asm+0x1a/0x30 >>>>>>>> [ 0.408042] </TASK> >>>>>>>> >>>>>>>> Currently, triggering an rdp offloaded state change need the >>>>>>>> corresponding rdp's CPU goes offline, and at this time the rcuc >>>>>>>> kthreads has already in parking state. this means the corresponding >>>>>>>> rcuc kthreads can safely read offloaded state of rdp while it's >>>>>>>> corresponding cpu is online. >>>>>>>> >>>>>>>> This commit therefore add rdp->rcu_cpu_kthread_task check for >>>>>>>> Preempt-RT kernels. >>>>>>>> >>>>>>>> Signed-off-by: Zqiang <qiang.zhang1211@gmail.com> >>>>>>>> --- >>>>>>>> kernel/rcu/tree_plugin.h | 4 +++- >>>>>>>> 1 file changed, 3 insertions(+), 1 deletion(-) >>>>>>>> >>>>>>>> diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h >>>>>>>> index 003e549f6514..fe728eded36e 100644 >>>>>>>> --- a/kernel/rcu/tree_plugin.h >>>>>>>> +++ b/kernel/rcu/tree_plugin.h >>>>>>>> @@ -31,7 +31,9 @@ static bool rcu_rdp_is_offloaded(struct rcu_data *rdp) >>>>>>>> lockdep_is_held(&rcu_state.nocb_mutex) || >>>>>>>> (!(IS_ENABLED(CONFIG_PREEMPT_COUNT) && preemptible()) && >>>>>>>> rdp == this_cpu_ptr(&rcu_data)) || >>>>>>>> - rcu_current_is_nocb_kthread(rdp)), >>>>>>>> + rcu_current_is_nocb_kthread(rdp) || >>>>>>>> + (IS_ENABLED(CONFIG_PREEMPT_RT) && >>>>>>>> + current == rdp->rcu_cpu_kthread_task)), >>>>>>> >>>>>>> Isn't it safe also on !CONFIG_PREEMPT_RT ? >>>>>> >>>>>> For !CONFIG_PREEMPT_RT and in rcuc kthreads, it's also safe, >>>>>> but the following check will passed : >>>>>> >>>>>> (!(IS_ENABLED(CONFIG_PREEMPT_COUNT) && preemptible()) && >>>>>> rdp == this_cpu_ptr(&rcu_data)) >>>>> >>>>> I think the fact that it already passes for !PREEMPT_RT does not matter, because >>>>> it simplifies the code so drop the PREEMPT_RT check? >>>>> >>>>> Or will softirq_count() not work? It appears to have special casing for >>>>> PREEMPT_RT's local_bh_disable(): >>>>> >>>>> ( ( !(IS_ENABLED(CONFIG_PREEMPT_COUNT) && preemptible()) || softirq_count() ) >>>>> && rdp == this_cpu_ptr(&rcu_data)) ) >>>> >>>> Thank you for Joel's reply, I also willing to accept such >>>> modifications and resend :) . >>> Thanks, I am Ok with either approach whichever you and Frederic together decide. >>> I can then pull this in for the v6.16 merge window once you resend, thanks! >>> >> >> Frederic, there are a couple of ways we can move forward hear. Does the >> softirq_count() approach sound good to you? If yes, I can fixup the patch myself. > > Hello, Joel > > If you send a patch to fix it, I'd be happy, you can add me as the > Reported-by ;) Actually Z, could you send the patch with the suggestion above after appropriate testing? That way I will be more comfortable applying it for 6.16. Sorry for any confusion, Thanks! - Joel > > Thanks > Zqiang > >> >> I am also Ok at this point to take it in for 6.16, though I've also stored it in >> my rcu/dev branch for Neeraj's 6.17 PR, just in case :) >> >> - Joel >> >>
Le Mon, May 05, 2025 at 09:38:02AM -0400, Joel Fernandes a écrit : > Frederic, there are a couple of ways we can move forward hear. Does the > softirq_count() approach sound good to you? If yes, I can fixup the patch > myself. That approach looks good yes. > > I am also Ok at this point to take it in for 6.16, though I've also stored it in > my rcu/dev branch for Neeraj's 6.17 PR, just in case :) I'm fine either way. To me it's neither too late nor too early :-) Thanks. > > - Joel > > -- Frederic Weisbecker SUSE Labs
© 2016 - 2026 Red Hat, Inc.