Re: [PATCH] cpuidle: fix fallback mechanism for suspend to idle in absence of enter_freeze

[Sudeep Holla]

On 22/01/16 01:15, Rafael J. Wysocki wrote:
> On Thursday, January 21, 2016 11:19:29 AM Sudeep Holla wrote:
> > Commit 51164251f5c3 ("sched / idle: Drop default_idle_call() fallback
> > from call_cpuidle()") made find_deepest_state() return non-negative
> > value and check all the states with index > 0. Also a result,
> > find_deepest_state() returns 0 even when enter_freeze callbacks are not
> > implemented and enter_freeze_proper is called which ends up crashing
> > the kernel.
> >
> > This patch updates the check for index > 0 in cpuidle_enter_freeze and
> > cpuidle_idle_call(when idle_should_freeze is true) to restore the
> > suspend-to-idle functionality in absence of enter_freeze callback.
> >
> > Fixes: 51164251f5c3 ("sched / idle: Drop default_idle_call() fallback from call_cpuidle()")
> > Cc: "Rafael J. Wysocki" <rjw@xxxxxxxxxxxxx>
> > Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> > Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> > Signed-off-by: Sudeep Holla <sudeep.holla@xxxxxxx>
> > ---
> >   drivers/cpuidle/cpuidle.c | 2 +-
> >   kernel/sched/idle.c       | 2 +-
> >   2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > Hi Rafael,
>
> Hi,
>
> Sorry for the breakage.

Sorry that I missed to test suspend-to-idle before it got merged.

> > I assume you prefer to retain find_deepest_state return non-negative
> > values, so I took this approach for fixing the bug. Do you think we
> > need to support enter_freeze_proper for index 0 ?
>
> Zero is a special case on x86, so supporting enter_freeze_proper() for it
> is not necessary.

Even on ARM, 0 is used for WFI only, which will not be used for freeze.

> If you think we can also make 0 a special case on ARM, the others should
> not object to that either.

Makes sense and since it's already reserved for WFI on ARM, it should be
fine. If there are no objections, can you pick up this fix ?

--
Regards,
Sudeep