[tip/core/rcu,15/15] rcu: RCU_SAVE_DYNTICK code no longer ever dead

From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>

Before RCU had unified idle, the RCU_SAVE_DYNTICK leg of the switch
statement in force_quiescent_state() was dead code for CONFIG_NO_HZ=n
kernel builds.  With unified idle, the code is never dead.  This commit
therefore removes the "if" statement designed to make gcc aware of when
the code was and was not dead.

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

On Fri, Jun 15, 2012 at 02:06:10PM -0700, Paul E. McKenney wrote:
> From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> 
> Before RCU had unified idle, the RCU_SAVE_DYNTICK leg of the switch
> statement in force_quiescent_state() was dead code for CONFIG_NO_HZ=n
> kernel builds.  With unified idle, the code is never dead.  This commit
> therefore removes the "if" statement designed to make gcc aware of when
> the code was and was not dead.
> 
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

One comment below; with that change:

Reviewed-by: Josh Triplett <josh@joshtriplett.org>

>  kernel/rcutree.c |    2 --
>  1 files changed, 0 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/rcutree.c b/kernel/rcutree.c
> index 75ad92a..0b0c9cc 100644
> --- a/kernel/rcutree.c
> +++ b/kernel/rcutree.c
> @@ -1744,8 +1744,6 @@ static void force_quiescent_state(struct rcu_state *rsp, int relaxed)
>  		break; /* grace period idle or initializing, ignore. */
>  
>  	case RCU_SAVE_DYNTICK:
> -		if (RCU_SIGNAL_INIT != RCU_SAVE_DYNTICK)
> -			break; /* So gcc recognizes the dead code. */
>  
>  		raw_spin_unlock(&rnp->lock);  /* irqs remain disabled */

Drop the blank line too?

On Fri, Jun 15, 2012 at 05:02:38PM -0700, Josh Triplett wrote:
> On Fri, Jun 15, 2012 at 02:06:10PM -0700, Paul E. McKenney wrote:
> > From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> > 
> > Before RCU had unified idle, the RCU_SAVE_DYNTICK leg of the switch
> > statement in force_quiescent_state() was dead code for CONFIG_NO_HZ=n
> > kernel builds.  With unified idle, the code is never dead.  This commit
> > therefore removes the "if" statement designed to make gcc aware of when
> > the code was and was not dead.
> > 
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> 
> One comment below; with that change:
> 
> Reviewed-by: Josh Triplett <josh@joshtriplett.org>
> 
> >  kernel/rcutree.c |    2 --
> >  1 files changed, 0 insertions(+), 2 deletions(-)
> > 
> > diff --git a/kernel/rcutree.c b/kernel/rcutree.c
> > index 75ad92a..0b0c9cc 100644
> > --- a/kernel/rcutree.c
> > +++ b/kernel/rcutree.c
> > @@ -1744,8 +1744,6 @@ static void force_quiescent_state(struct rcu_state *rsp, int relaxed)
> >  		break; /* grace period idle or initializing, ignore. */
> >  
> >  	case RCU_SAVE_DYNTICK:
> > -		if (RCU_SIGNAL_INIT != RCU_SAVE_DYNTICK)
> > -			break; /* So gcc recognizes the dead code. */
> >  
> >  		raw_spin_unlock(&rnp->lock);  /* irqs remain disabled */
> 
> Drop the blank line too?

Actually, I just realized a larger concern with what this change
implies: does this mean that whatever change made this code no longer
dead introduced a major locking bug here?  If so, has that change
already progressed past the point where you could update it to include
this fix?

- Josh Triplett

On Fri, Jun 15, 2012 at 05:04:49PM -0700, Josh Triplett wrote:
> On Fri, Jun 15, 2012 at 05:02:38PM -0700, Josh Triplett wrote:
> > On Fri, Jun 15, 2012 at 02:06:10PM -0700, Paul E. McKenney wrote:
> > > From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> > > 
> > > Before RCU had unified idle, the RCU_SAVE_DYNTICK leg of the switch
> > > statement in force_quiescent_state() was dead code for CONFIG_NO_HZ=n
> > > kernel builds.  With unified idle, the code is never dead.  This commit
> > > therefore removes the "if" statement designed to make gcc aware of when
> > > the code was and was not dead.
> > > 
> > > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > 
> > One comment below; with that change:
> > 
> > Reviewed-by: Josh Triplett <josh@joshtriplett.org>
> > 
> > >  kernel/rcutree.c |    2 --
> > >  1 files changed, 0 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/kernel/rcutree.c b/kernel/rcutree.c
> > > index 75ad92a..0b0c9cc 100644
> > > --- a/kernel/rcutree.c
> > > +++ b/kernel/rcutree.c
> > > @@ -1744,8 +1744,6 @@ static void force_quiescent_state(struct rcu_state *rsp, int relaxed)
> > >  		break; /* grace period idle or initializing, ignore. */
> > >  
> > >  	case RCU_SAVE_DYNTICK:
> > > -		if (RCU_SIGNAL_INIT != RCU_SAVE_DYNTICK)
> > > -			break; /* So gcc recognizes the dead code. */
> > >  
> > >  		raw_spin_unlock(&rnp->lock);  /* irqs remain disabled */
> > 
> > Drop the blank line too?
> 
> Actually, I just realized a larger concern with what this change
> implies: does this mean that whatever change made this code no longer
> dead introduced a major locking bug here?  If so, has that change
> already progressed past the point where you could update it to include
> this fix?

No, the lock is dropped and then reacquired, so the "break" is OK.
This change should have been made back when dyntick-idle mode became
unconditional from RCU's viewpoint.

And yes, I probably should change "rcu_dyntick" to "rcu_idle" and
make a bunch of similar changes.  But not particularly high priority.

							Thanx, Paul