Re: [PATCH 1/2] cpuidle: Clean up fallback handling in cpuidle_idle_call()

[Rafael J. Wysocki]

On Monday, March 02, 2015 04:05:36 PM Lorenzo Pieralisi wrote:
> On Mon, Mar 02, 2015 at 02:51:35PM +0000, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> > 
> > Move the fallback code path in cpuidle_idle_call() to the end of the
> > function to avoid jumping to a label in a an if () branch.
> 
> Nit: "in an if () branch"
> 
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> > ---
> >  kernel/sched/idle.c |   29 +++++++++++++++--------------
> >  1 file changed, 15 insertions(+), 14 deletions(-)
> > 
> > Index: linux-pm/kernel/sched/idle.c
> > ===================================================================
> > --- linux-pm.orig/kernel/sched/idle.c
> > +++ linux-pm/kernel/sched/idle.c
> > @@ -124,20 +124,8 @@ static void cpuidle_idle_call(void)
> >  	 * Fall back to the default arch idle method on errors.
> >  	 */
> >  	next_state = cpuidle_select(drv, dev);
> > -	if (next_state < 0) {
> > -use_default:
> > -		/*
> > -		 * We can't use the cpuidle framework, let's use the default
> > -		 * idle routine.
> > -		 */
> > -		if (current_clr_polling_and_test())
> > -			local_irq_enable();
> > -		else
> > -			arch_cpu_idle();
> > -
> > -		goto exit_idle;
> > -	}
> > -
> > +	if (next_state < 0)
> > +		goto use_default;
> >  
> >  	/*
> >  	 * The idle task must be scheduled, it is pointless to
> > @@ -195,6 +183,19 @@ exit_idle:
> >  
> >  	rcu_idle_exit();
> >  	start_critical_timings();
> > +	return;
> > +
> > +use_default:
> > +	/*
> > +	 * We can't use the cpuidle framework, let's use the default
> > +	 * idle routine.
> > +	 */
> > +	if (current_clr_polling_and_test())
> > +		local_irq_enable();
> > +	else
> > +		arch_cpu_idle();
> > +
> > +	goto exit_idle;
> 
> I wonder whether making the code at label use_default a function saves us
> some jumping around, not sure it is worth the churn, your call, I am ok as
> it is.

We'd need to jump anyway, we might only need to jump one time less and we'd
need to call the function for multiple times instead if I'm not mistaken.

I'm going to apply the current version.

Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/