linux-kernel - Re: [PATCH v2 2/3] cpuidle: allow setting deepest 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: <20161115085217.72bec126@jacob-builder>
Date:   Tue, 15 Nov 2016 08:52:17 -0800
From:   Jacob Pan <jacob.jun.pan@...ux.intel.com>
To:     "Rafael J. Wysocki" <rjw@...ysocki.net>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        LKML <linux-kernel@...r.kernel.org>,
        Linux PM <linux-pm@...r.kernel.org>,
        Arjan van de Ven <arjan@...ux.intel.com>,
        Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
        Len Brown <lenb@...nel.org>,
        Rafael Wysocki <rafael.j.wysocki@...el.com>,
        Eduardo Valentin <edubezval@...il.com>,
        Zhang Rui <rui.zhang@...el.com>,
        Petr Mladek <pmladek@...e.com>,
        Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
        jacob.jun.pan@...ux.intel.com
Subject: Re: [PATCH v2 2/3] cpuidle: allow setting deepest idle

On Mon, 14 Nov 2016 22:42:03 +0100
"Rafael J. Wysocki" <rjw@...ysocki.net> wrote:

> > diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
> > index cb6442f..9e80f32 100644
> > --- a/kernel/sched/idle.c
> > +++ b/kernel/sched/idle.c
> > @@ -173,6 +173,9 @@ static void cpuidle_idle_call(void)
> >  
> >  		next_state = cpuidle_find_deepest_state(drv, dev);
> >  		call_cpuidle(drv, dev, next_state);
> > +	} else if (dev->use_deepest_state) {
> > +		next_state = cpuidle_find_deepest_state(drv, dev);
> > +		call_cpuidle(drv, dev, next_state);
> >  	} else {
> >  		/*
> >  		 * Ask the cpuidle framework to choose a
> > convenient idle state.  
> 
> I would arrange the code slightly differently here:
> 
> 	if (idle_should_freeze() || dev->use_deepest_state) {
> 		if (idle_should_freeze()) {
> 			entered_state = cpuidle_enter_freeze(drv,
> dev); if (entered_state > 0) {
> 				local_irq_enable();
> 				goto exit_idle;
> 			}
> 		}
> 
> 		next_state = cpuidle_find_deepest_state(drv, dev);
> 		call_cpuidle(drv, dev, next_state);
> 	} else {
> 
> 
> This way you'd avoid the ugly code duplication and the extra
> dev->use_deepest_state branch in the most frequent case.  I guess you
> could take the unlikely() thing away from idle_should_freeze() and
> use it directly here too.
Sounds good. Will change in the next version.