[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CALm+0cWyWrEcsYx+6nDT9XjMdLBDwx27SmAXTPUFLB59TbEc3Q@mail.gmail.com>
Date: Thu, 8 May 2025 14:43:11 +0800
From: Z qiang <qiang.zhang1211@...il.com>
To: Frederic Weisbecker <frederic@...nel.org>
Cc: paulmck@...nel.org, neeraj.upadhyay@...nel.org, joel@...lfernandes.org,
urezki@...il.com, boqun.feng@...il.com, rcu@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] rcu/nocb: Add Safe checks for access offloaded rdp
On Thu, May 8, 2025 at 12:25 AM Frederic Weisbecker <frederic@...nel.org> wrote:
>
> Le Wed, May 07, 2025 at 07:26:04PM +0800, Zqiang a écrit :
> > For built with CONFIG_PROVE_RCU=y and CONFIG_PREEMPT_RT=y kernels,
> > Disable BH does not change the SOFTIRQ corresponding bits in
> > preempt_count(), but change current->softirq_disable_cnt, 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 softirq_count() check for
> > Preempt-RT kernels.
> >
> > Suggested-by: Joel Fernandes <joelagnelf@...dia.com>
> > Signed-off-by: Zqiang <qiang.zhang1211@...il.com>
> > ---
> > kernel/rcu/tree_plugin.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
> > index 003e549f6514..a91b2322a0cd 100644
> > --- a/kernel/rcu/tree_plugin.h
> > +++ b/kernel/rcu/tree_plugin.h
> > @@ -29,7 +29,7 @@ static bool rcu_rdp_is_offloaded(struct rcu_data *rdp)
> > (IS_ENABLED(CONFIG_HOTPLUG_CPU) && lockdep_is_cpus_held()) ||
> > lockdep_is_held(&rdp->nocb_lock) ||
> > lockdep_is_held(&rcu_state.nocb_mutex) ||
> > - (!(IS_ENABLED(CONFIG_PREEMPT_COUNT) && preemptible()) &&
> > + ((!(IS_ENABLED(CONFIG_PREEMPT_COUNT) && preemptible()) || softirq_count()) &&
> > rdp == this_cpu_ptr(&rcu_data)) ||
>
> On a second thought, isn't "rdp == this_cpu_ptr(&rcu_data)" enough?
If the CONFIG_DEBUG_PREEMPT=y, the following code will cause
a warning in rcuop kthreads:
WARN_ON_ONCE(!rcu_rdp_is_offloaded(rdp))
> The offloaded state can only change if the CPU is completely offline.
> But if the current CPU is looking at the local rdp, it means it is online
> and the rdp can't be concurrently [de]offloaded, right?
yes
Thanks
Zqiang
>
> Thanks.
>
> > rcu_current_is_nocb_kthread(rdp)),
> > "Unsafe read of RCU_NOCB offloaded state"
> > --
> > 2.17.1
> >
> >
>
> --
> Frederic Weisbecker
> SUSE Labs
Powered by blists - more mailing lists