Re: [PATCH v2 2/3] cpuidle: allow setting deepest idle

From	Jacob Pan <jacob.jun.pan@linux.intel.com>
Newsgroups	linux.kernel
Subject	Re: [PATCH v2 2/3] cpuidle: allow setting deepest idle
Date	2016-11-15 18:00 +0100
Message-ID	<sDPfr-7tr-19@gated-at.bofh.it> (permalink)
References	<sDuum-2hM-7@gated-at.bofh.it> <sDuE1-2m1-3@gated-at.bofh.it> <sDx8S-3Zy-11@gated-at.bofh.it>
Organization	OTC

Show all headers | View raw

On Mon, 14 Nov 2016 22:42:03 +0100
"Rafael J. Wysocki" <rjw@rjwysocki.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.

Thread

[PATCH v2 0/3] Stop sched tick in idle injection task Jacob Pan <jacob.jun.pan@linux.intel.com> - 2016-11-14 19:50 +0100
  [PATCH v2 2/3] cpuidle: allow setting deepest idle Jacob Pan <jacob.jun.pan@linux.intel.com> - 2016-11-14 20:00 +0100
    Re: [PATCH v2 2/3] cpuidle: allow setting deepest idle "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-11-14 22:40 +0100
      Re: [PATCH v2 2/3] cpuidle: allow setting deepest idle Jacob Pan <jacob.jun.pan@linux.intel.com> - 2016-11-15 18:00 +0100
  [PATCH v2 1/3] idle: add support for tasks that inject idle Jacob Pan <jacob.jun.pan@linux.intel.com> - 2016-11-14 20:00 +0100
  [PATCH v2 3/3] thermal/powerclamp: stop sched tick in forced idle Jacob Pan <jacob.jun.pan@linux.intel.com> - 2016-11-14 20:00 +0100

csiph-web