diff mbox series

[3/3] rcu: Report callbacks enqueued on offline CPU blind spot

Message ID 20241002145738.38226-4-frederic@kernel.org (mailing list archive)
State New
Headers show
Series rcu: Fix yet another wake up from offline related issue | expand

Commit Message

Frederic Weisbecker Oct. 2, 2024, 2:57 p.m. UTC
Callbacks enqueued after rcutree_report_cpu_dead() fall into RCU barrier
blind spot. Report any potential misuse.

Reported-by: Paul E. McKenney <paulmck@kernel.org>
Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
---
 kernel/rcu/tree.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Frederic Weisbecker Oct. 2, 2024, 3 p.m. UTC | #1
Le Wed, Oct 02, 2024 at 04:57:38PM +0200, Frederic Weisbecker a écrit :
> Callbacks enqueued after rcutree_report_cpu_dead() fall into RCU barrier
> blind spot. Report any potential misuse.
> 
> Reported-by: Paul E. McKenney <paulmck@kernel.org>
> Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
> ---
>  kernel/rcu/tree.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index a60616e69b66..36070b6bf4a1 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -3084,8 +3084,11 @@ __call_rcu_common(struct rcu_head *head, rcu_callback_t func, bool lazy_in)
>  	head->func = func;
>  	head->next = NULL;
>  	kasan_record_aux_stack_noalloc(head);
> +
>  	local_irq_save(flags);
>  	rdp = this_cpu_ptr(&rcu_data);
> +	RCU_LOCKDEP_WARN(rcu_rdp_cpu_online(rdp), "Callback enqueued on offline
> CPU!");

This should be !rcu_rdp_cpu_online(rdp)

Sigh...

> +
>  	lazy = lazy_in && !rcu_async_should_hurry();
>  
>  	/* Add the callback to our list. */
> -- 
> 2.46.0
> 
>
Paul E. McKenney Oct. 9, 2024, 2:03 a.m. UTC | #2
On Wed, Oct 02, 2024 at 05:00:03PM +0200, Frederic Weisbecker wrote:
> Le Wed, Oct 02, 2024 at 04:57:38PM +0200, Frederic Weisbecker a écrit :
> > Callbacks enqueued after rcutree_report_cpu_dead() fall into RCU barrier
> > blind spot. Report any potential misuse.
> > 
> > Reported-by: Paul E. McKenney <paulmck@kernel.org>
> > Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
> > ---
> >  kernel/rcu/tree.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > index a60616e69b66..36070b6bf4a1 100644
> > --- a/kernel/rcu/tree.c
> > +++ b/kernel/rcu/tree.c
> > @@ -3084,8 +3084,11 @@ __call_rcu_common(struct rcu_head *head, rcu_callback_t func, bool lazy_in)
> >  	head->func = func;
> >  	head->next = NULL;
> >  	kasan_record_aux_stack_noalloc(head);
> > +
> >  	local_irq_save(flags);
> >  	rdp = this_cpu_ptr(&rcu_data);
> > +	RCU_LOCKDEP_WARN(rcu_rdp_cpu_online(rdp), "Callback enqueued on offline
> > CPU!");
> 
> This should be !rcu_rdp_cpu_online(rdp)
> 
> Sigh...

I am pulling this in for testing with this change, thank you!

							Thanx, Paul

> > +
> >  	lazy = lazy_in && !rcu_async_should_hurry();
> >  
> >  	/* Add the callback to our list. */
> > -- 
> > 2.46.0
> > 
> >
Neeraj Upadhyay Oct. 9, 2024, 2:24 a.m. UTC | #3
On 10/2/2024 8:30 PM, Frederic Weisbecker wrote:
> Le Wed, Oct 02, 2024 at 04:57:38PM +0200, Frederic Weisbecker a écrit :
>> Callbacks enqueued after rcutree_report_cpu_dead() fall into RCU barrier
>> blind spot. Report any potential misuse.
>>
>> Reported-by: Paul E. McKenney <paulmck@kernel.org>
>> Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
>> ---
>>  kernel/rcu/tree.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
>> index a60616e69b66..36070b6bf4a1 100644
>> --- a/kernel/rcu/tree.c
>> +++ b/kernel/rcu/tree.c
>> @@ -3084,8 +3084,11 @@ __call_rcu_common(struct rcu_head *head, rcu_callback_t func, bool lazy_in)
>>  	head->func = func;
>>  	head->next = NULL;
>>  	kasan_record_aux_stack_noalloc(head);
>> +
>>  	local_irq_save(flags);
>>  	rdp = this_cpu_ptr(&rcu_data);
>> +	RCU_LOCKDEP_WARN(rcu_rdp_cpu_online(rdp), "Callback enqueued on offline
>> CPU!");
> 
> This should be !rcu_rdp_cpu_online(rdp)
> 

With this patch series, 600 mins RCU torture overnight testing completed
without failures at my end.

- Neeraj

> Sigh...
> 
>> +
>>  	lazy = lazy_in && !rcu_async_should_hurry();
>>  
>>  	/* Add the callback to our list. */
>> -- 
>> 2.46.0
>>
>>
Paul E. McKenney Oct. 9, 2024, 3:13 p.m. UTC | #4
On Tue, Oct 08, 2024 at 07:03:50PM -0700, Paul E. McKenney wrote:
> On Wed, Oct 02, 2024 at 05:00:03PM +0200, Frederic Weisbecker wrote:
> > Le Wed, Oct 02, 2024 at 04:57:38PM +0200, Frederic Weisbecker a écrit :
> > > Callbacks enqueued after rcutree_report_cpu_dead() fall into RCU barrier
> > > blind spot. Report any potential misuse.
> > > 
> > > Reported-by: Paul E. McKenney <paulmck@kernel.org>
> > > Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
> > > ---
> > >  kernel/rcu/tree.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > > index a60616e69b66..36070b6bf4a1 100644
> > > --- a/kernel/rcu/tree.c
> > > +++ b/kernel/rcu/tree.c
> > > @@ -3084,8 +3084,11 @@ __call_rcu_common(struct rcu_head *head, rcu_callback_t func, bool lazy_in)
> > >  	head->func = func;
> > >  	head->next = NULL;
> > >  	kasan_record_aux_stack_noalloc(head);
> > > +
> > >  	local_irq_save(flags);
> > >  	rdp = this_cpu_ptr(&rcu_data);
> > > +	RCU_LOCKDEP_WARN(rcu_rdp_cpu_online(rdp), "Callback enqueued on offline
> > > CPU!");
> > 
> > This should be !rcu_rdp_cpu_online(rdp)
> > 
> > Sigh...
> 
> I am pulling this in for testing with this change, thank you!

And:

Tested-by: Paul E. McKenney <paulmck@kernel.org>

> 							Thanx, Paul
> 
> > > +
> > >  	lazy = lazy_in && !rcu_async_should_hurry();
> > >  
> > >  	/* Add the callback to our list. */
> > > -- 
> > > 2.46.0
> > > 
> > > 
>
Joel Fernandes Oct. 10, 2024, 2:30 p.m. UTC | #5
On Wed, Oct 9, 2024 at 11:13 AM Paul E. McKenney <paulmck@kernel.org> wrote:
>
> On Tue, Oct 08, 2024 at 07:03:50PM -0700, Paul E. McKenney wrote:
> > On Wed, Oct 02, 2024 at 05:00:03PM +0200, Frederic Weisbecker wrote:
> > > Le Wed, Oct 02, 2024 at 04:57:38PM +0200, Frederic Weisbecker a écrit :
> > > > Callbacks enqueued after rcutree_report_cpu_dead() fall into RCU barrier
> > > > blind spot. Report any potential misuse.
> > > >
> > > > Reported-by: Paul E. McKenney <paulmck@kernel.org>
> > > > Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
> > > > ---
> > > >  kernel/rcu/tree.c | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > > > index a60616e69b66..36070b6bf4a1 100644
> > > > --- a/kernel/rcu/tree.c
> > > > +++ b/kernel/rcu/tree.c
> > > > @@ -3084,8 +3084,11 @@ __call_rcu_common(struct rcu_head *head, rcu_callback_t func, bool lazy_in)
> > > >   head->func = func;
> > > >   head->next = NULL;
> > > >   kasan_record_aux_stack_noalloc(head);
> > > > +
> > > >   local_irq_save(flags);
> > > >   rdp = this_cpu_ptr(&rcu_data);
> > > > + RCU_LOCKDEP_WARN(rcu_rdp_cpu_online(rdp), "Callback enqueued on offline
> > > > CPU!");
> > >
> > > This should be !rcu_rdp_cpu_online(rdp)
> > >
> > > Sigh...
> >
> > I am pulling this in for testing with this change, thank you!
>
> And:
>
> Tested-by: Paul E. McKenney <paulmck@kernel.org>

With the correction,
Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org>

thanks,

 - Joel
diff mbox series

Patch

diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index a60616e69b66..36070b6bf4a1 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -3084,8 +3084,11 @@  __call_rcu_common(struct rcu_head *head, rcu_callback_t func, bool lazy_in)
 	head->func = func;
 	head->next = NULL;
 	kasan_record_aux_stack_noalloc(head);
+
 	local_irq_save(flags);
 	rdp = this_cpu_ptr(&rcu_data);
+	RCU_LOCKDEP_WARN(rcu_rdp_cpu_online(rdp), "Callback enqueued on offline CPU!");
+
 	lazy = lazy_in && !rcu_async_should_hurry();
 
 	/* Add the callback to our list. */