> > > > 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.zhang1...@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 > >
