linux-kernel - Re: [PATCH 1/2] cpuidle: Clean up fallback handling in cpuidle_idle

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <1646614.U29HtKCHyB@vostro.rjw.lan>
Date:	Mon, 02 Mar 2015 23:30:49 +0100
From:	"Rafael J. Wysocki" <rjw@...ysocki.net>
To:	Lorenzo Pieralisi <lorenzo.pieralisi@....com>
Cc:	Daniel Lezcano <daniel.lezcano@...aro.org>,
	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Peter Zijlstra <peterz@...radead.org>,
	"preeti@...ux.vnet.ibm.com" <preeti@...ux.vnet.ibm.com>
Subject: Re: [PATCH 1/2] cpuidle: Clean up fallback handling in cpuidle_idle_call()

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@...el.com>
> > 
> > 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@...el.com>
> > ---
> >  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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/