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

Reply via email to