* [PATCH] rcu/nocb: Add Safe checks for access offloaded rdp @ 2025-04-28 9:54 Zqiang 2025-04-28 10:14 ` Frederic Weisbecker 0 siblings, 1 reply; 11+ messages in thread From: Zqiang @ 2025-04-28 9:54 UTC (permalink / raw) To: paulmck, frederic, neeraj.upadhyay, joel, urezki, boqun.feng Cc: qiang.zhang1211, rcu, linux-kernel 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.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] rcu/nocb: Add Safe checks for access offloaded rdp 2025-04-28 9:54 [PATCH] rcu/nocb: Add Safe checks for access offloaded rdp Zqiang @ 2025-04-28 10:14 ` Frederic Weisbecker 2025-04-28 10:59 ` Z qiang 0 siblings, 1 reply; 11+ messages in thread From: Frederic Weisbecker @ 2025-04-28 10:14 UTC (permalink / raw) To: Zqiang Cc: paulmck, neeraj.upadhyay, joel, urezki, boqun.feng, rcu, linux-kernel 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 ? Thanks. > "Unsafe read of RCU_NOCB offloaded state" > ); > > -- > 2.17.1 > -- Frederic Weisbecker SUSE Labs ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] rcu/nocb: Add Safe checks for access offloaded rdp 2025-04-28 10:14 ` Frederic Weisbecker @ 2025-04-28 10:59 ` Z qiang 2025-04-28 14:46 ` Joel Fernandes 2025-04-30 14:54 ` Z qiang 0 siblings, 2 replies; 11+ messages in thread From: Z qiang @ 2025-04-28 10:59 UTC (permalink / raw) To: Frederic Weisbecker Cc: paulmck, neeraj.upadhyay, joel, urezki, boqun.feng, rcu, linux-kernel > > 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] rcu/nocb: Add Safe checks for access offloaded rdp 2025-04-28 10:59 ` Z qiang @ 2025-04-28 14:46 ` Joel Fernandes 2025-04-30 14:57 ` Z qiang 2025-04-30 14:54 ` Z qiang 1 sibling, 1 reply; 11+ messages in thread From: Joel Fernandes @ 2025-04-28 14:46 UTC (permalink / raw) To: Z qiang, Frederic Weisbecker Cc: paulmck, neeraj.upadhyay, joel, urezki, boqun.feng, rcu, linux-kernel 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] rcu/nocb: Add Safe checks for access offloaded rdp 2025-04-28 14:46 ` Joel Fernandes @ 2025-04-30 14:57 ` Z qiang 2025-04-30 16:14 ` Joel Fernandes 0 siblings, 1 reply; 11+ messages in thread From: Z qiang @ 2025-04-30 14:57 UTC (permalink / raw) To: Joel Fernandes Cc: Frederic Weisbecker, paulmck, neeraj.upadhyay, joel, urezki, boqun.feng, rcu, linux-kernel > > > > 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 > > > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] rcu/nocb: Add Safe checks for access offloaded rdp 2025-04-30 14:57 ` Z qiang @ 2025-04-30 16:14 ` Joel Fernandes 2025-05-05 13:38 ` Joel Fernandes 0 siblings, 1 reply; 11+ messages in thread From: Joel Fernandes @ 2025-04-30 16:14 UTC (permalink / raw) To: Z qiang Cc: Frederic Weisbecker, paulmck, neeraj.upadhyay, joel, urezki, boqun.feng, rcu, linux-kernel 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] rcu/nocb: Add Safe checks for access offloaded rdp 2025-04-30 16:14 ` Joel Fernandes @ 2025-05-05 13:38 ` Joel Fernandes 2025-05-05 21:45 ` Frederic Weisbecker 2025-05-06 6:25 ` Z qiang 0 siblings, 2 replies; 11+ messages in thread From: Joel Fernandes @ 2025-05-05 13:38 UTC (permalink / raw) To: Z qiang, Frederic Weisbecker Cc: paulmck, neeraj.upadhyay, joel, urezki, boqun.feng, rcu, linux-kernel 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] rcu/nocb: Add Safe checks for access offloaded rdp 2025-05-05 13:38 ` Joel Fernandes @ 2025-05-05 21:45 ` Frederic Weisbecker 2025-05-06 6:25 ` Z qiang 1 sibling, 0 replies; 11+ messages in thread From: Frederic Weisbecker @ 2025-05-05 21:45 UTC (permalink / raw) To: Joel Fernandes Cc: Z qiang, paulmck, neeraj.upadhyay, joel, urezki, boqun.feng, rcu, linux-kernel 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] rcu/nocb: Add Safe checks for access offloaded rdp 2025-05-05 13:38 ` Joel Fernandes 2025-05-05 21:45 ` Frederic Weisbecker @ 2025-05-06 6:25 ` Z qiang 2025-05-06 17:49 ` Joel Fernandes 1 sibling, 1 reply; 11+ messages in thread From: Z qiang @ 2025-05-06 6:25 UTC (permalink / raw) To: Joel Fernandes Cc: Frederic Weisbecker, paulmck, neeraj.upadhyay, joel, urezki, boqun.feng, rcu, linux-kernel > > > > 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 > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] rcu/nocb: Add Safe checks for access offloaded rdp 2025-05-06 6:25 ` Z qiang @ 2025-05-06 17:49 ` Joel Fernandes 0 siblings, 0 replies; 11+ messages in thread From: Joel Fernandes @ 2025-05-06 17:49 UTC (permalink / raw) To: qiang Z Cc: Joel Fernandes, Frederic Weisbecker, paulmck, neeraj.upadhyay, urezki, boqun.feng, rcu, linux-kernel > 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 >> >> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] rcu/nocb: Add Safe checks for access offloaded rdp 2025-04-28 10:59 ` Z qiang 2025-04-28 14:46 ` Joel Fernandes @ 2025-04-30 14:54 ` Z qiang 1 sibling, 0 replies; 11+ messages in thread From: Z qiang @ 2025-04-30 14:54 UTC (permalink / raw) To: Frederic Weisbecker Cc: paulmck, neeraj.upadhyay, joel, urezki, boqun.feng, rcu, linux-kernel > > > > > 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-05-06 17:49 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-04-28 9:54 [PATCH] rcu/nocb: Add Safe checks for access offloaded rdp Zqiang 2025-04-28 10:14 ` Frederic Weisbecker 2025-04-28 10:59 ` Z qiang 2025-04-28 14:46 ` Joel Fernandes 2025-04-30 14:57 ` Z qiang 2025-04-30 16:14 ` Joel Fernandes 2025-05-05 13:38 ` Joel Fernandes 2025-05-05 21:45 ` Frederic Weisbecker 2025-05-06 6:25 ` Z qiang 2025-05-06 17:49 ` Joel Fernandes 2025-04-30 14:54 ` Z qiang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).