diff mbox

[tip/core/rcu,3/4] rcu: stop spurious warnings from synchronize_sched_expedited

Message ID 1329847110-10897-3-git-send-email-paulmck@linux.vnet.ibm.com
State Superseded
Headers show

Commit Message

Paul E. McKenney Feb. 21, 2012, 5:58 p.m. UTC
From: Hugh Dickins <hughd@google.com>

synchronize_sched_expedited() is spamming CONFIG_DEBUG_PREEMPT=y
users with an unintended warning from the cpu_is_offline() check:
use raw_smp_processor_id() instead of smp_processor_id() there.

Signed-off-by: Hugh Dickins <hughd@google.com>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 kernel/rcutree.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Comments

Peter Zijlstra Feb. 21, 2012, 6 p.m. UTC | #1
On Tue, 2012-02-21 at 09:58 -0800, Paul E. McKenney wrote:
> From: Hugh Dickins <hughd@google.com>
> 
> synchronize_sched_expedited() is spamming CONFIG_DEBUG_PREEMPT=y
> users with an unintended warning from the cpu_is_offline() check:
> use raw_smp_processor_id() instead of smp_processor_id() there.

This fails to mention why it makes sense to test a random cpu for
offline-ness..

> Signed-off-by: Hugh Dickins <hughd@google.com>
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> ---
>  kernel/rcutree.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/kernel/rcutree.c b/kernel/rcutree.c
> index eacc10b..1050d6d 100644
> --- a/kernel/rcutree.c
> +++ b/kernel/rcutree.c
> @@ -2014,7 +2014,7 @@ void synchronize_sched_expedited(void)
>  	/* Note that atomic_inc_return() implies full memory barrier. */
>  	firstsnap = snap = atomic_inc_return(&sync_sched_expedited_started);
>  	get_online_cpus();
> -	WARN_ON_ONCE(cpu_is_offline(smp_processor_id()));
> +	WARN_ON_ONCE(cpu_is_offline(raw_smp_processor_id()));
>  
>  	/*
>  	 * Each pass through the following loop attempts to force a
Paul E. McKenney Feb. 21, 2012, 6:28 p.m. UTC | #2
On Tue, Feb 21, 2012 at 07:00:17PM +0100, Peter Zijlstra wrote:
> On Tue, 2012-02-21 at 09:58 -0800, Paul E. McKenney wrote:
> > From: Hugh Dickins <hughd@google.com>
> > 
> > synchronize_sched_expedited() is spamming CONFIG_DEBUG_PREEMPT=y
> > users with an unintended warning from the cpu_is_offline() check:
> > use raw_smp_processor_id() instead of smp_processor_id() there.
> 
> This fails to mention why it makes sense to test a random cpu for
> offline-ness..

The check was already there, Hugh simply fixed it to use raw_.  The check
itself was added in c0d6d01bf (Check for illegal use of RCU from offlined
CPUs).  The purpose is to catch improper use of RCU from CPU_DYING
notifiers and on the path from the CPU_DYING notifiers to the idle loop.

							Thanx, Paul

> > Signed-off-by: Hugh Dickins <hughd@google.com>
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > ---
> >  kernel/rcutree.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/kernel/rcutree.c b/kernel/rcutree.c
> > index eacc10b..1050d6d 100644
> > --- a/kernel/rcutree.c
> > +++ b/kernel/rcutree.c
> > @@ -2014,7 +2014,7 @@ void synchronize_sched_expedited(void)
> >  	/* Note that atomic_inc_return() implies full memory barrier. */
> >  	firstsnap = snap = atomic_inc_return(&sync_sched_expedited_started);
> >  	get_online_cpus();
> > -	WARN_ON_ONCE(cpu_is_offline(smp_processor_id()));
> > +	WARN_ON_ONCE(cpu_is_offline(raw_smp_processor_id()));
> >  
> >  	/*
> >  	 * Each pass through the following loop attempts to force a
>
Peter Zijlstra Feb. 21, 2012, 6:57 p.m. UTC | #3
On Tue, 2012-02-21 at 10:28 -0800, Paul E. McKenney wrote:
> On Tue, Feb 21, 2012 at 07:00:17PM +0100, Peter Zijlstra wrote:
> > On Tue, 2012-02-21 at 09:58 -0800, Paul E. McKenney wrote:
> > > From: Hugh Dickins <hughd@google.com>
> > > 
> > > synchronize_sched_expedited() is spamming CONFIG_DEBUG_PREEMPT=y
> > > users with an unintended warning from the cpu_is_offline() check:
> > > use raw_smp_processor_id() instead of smp_processor_id() there.
> > 
> > This fails to mention why it makes sense to test a random cpu for
> > offline-ness..
> 
> The check was already there, Hugh simply fixed it to use raw_.  The check
> itself was added in c0d6d01bf (Check for illegal use of RCU from offlined
> CPUs).  The purpose is to catch improper use of RCU from CPU_DYING
> notifiers and on the path from the CPU_DYING notifiers to the idle loop.

Then I think this patch wants to add a comment explaining this. Because
cpu_offline(raw_smp_processor_id()) looks really rather suspicious.
Paul E. McKenney Feb. 21, 2012, 11:30 p.m. UTC | #4
On Tue, Feb 21, 2012 at 07:57:03PM +0100, Peter Zijlstra wrote:
> On Tue, 2012-02-21 at 10:28 -0800, Paul E. McKenney wrote:
> > On Tue, Feb 21, 2012 at 07:00:17PM +0100, Peter Zijlstra wrote:
> > > On Tue, 2012-02-21 at 09:58 -0800, Paul E. McKenney wrote:
> > > > From: Hugh Dickins <hughd@google.com>
> > > > 
> > > > synchronize_sched_expedited() is spamming CONFIG_DEBUG_PREEMPT=y
> > > > users with an unintended warning from the cpu_is_offline() check:
> > > > use raw_smp_processor_id() instead of smp_processor_id() there.
> > > 
> > > This fails to mention why it makes sense to test a random cpu for
> > > offline-ness..
> > 
> > The check was already there, Hugh simply fixed it to use raw_.  The check
> > itself was added in c0d6d01bf (Check for illegal use of RCU from offlined
> > CPUs).  The purpose is to catch improper use of RCU from CPU_DYING
> > notifiers and on the path from the CPU_DYING notifiers to the idle loop.
> 
> Then I think this patch wants to add a comment explaining this. Because
> cpu_offline(raw_smp_processor_id()) looks really rather suspicious.

Good point, will fix!

							Thanx, Paul
diff mbox

Patch

diff --git a/kernel/rcutree.c b/kernel/rcutree.c
index eacc10b..1050d6d 100644
--- a/kernel/rcutree.c
+++ b/kernel/rcutree.c
@@ -2014,7 +2014,7 @@  void synchronize_sched_expedited(void)
 	/* Note that atomic_inc_return() implies full memory barrier. */
 	firstsnap = snap = atomic_inc_return(&sync_sched_expedited_started);
 	get_online_cpus();
-	WARN_ON_ONCE(cpu_is_offline(smp_processor_id()));
+	WARN_ON_ONCE(cpu_is_offline(raw_smp_processor_id()));
 
 	/*
 	 * Each pass through the following loop attempts to force a