| Message ID | 3318440.aeNJFYEL58@rjwysocki.net |
|---|---|
| State | New |
| Headers | show |
| Series | cpuidle: Do not return from cpuidle_play_dead() on callback failures | expand |
Hello Rafael, On Fri, Nov 15, 2024 at 09:58:31PM +0100, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > If the :enter_dead() idle state callback fails for a certain state, > there may be still a shallower state for which it will work. > > Because the only caller of cpuidle_play_dead(), native_play_dead(), > falls back to hlt_play_dead() if it returns an error, it should > better try all of the idle states for which :enter_dead() is present > before failing, so change it accordingly. > > Also notice that the :enter_dead() state callback is not expected > to return on success (the CPU should be "dead" then), so make > cpuidle_play_dead() ignore its return value. > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > Reviewed-by: Mario Limonciello <mario.limonciello@amd.com> > Tested-by: Mario Limonciello <mario.limonciello@amd.com> # 6.12-rc7 Thanks for fixing this. Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com> -- Thanks and Regards gautham. > --- > > v1 -> v2: > * Make cpuidle_play_dead() never return 0. > * Add tags from Mario (I have added them because the change of the patch > should not make a practical difference, but if you want them to be > dropped, please let me know). > > --- > drivers/cpuidle/cpuidle.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > Index: linux-pm/drivers/cpuidle/cpuidle.c > =================================================================== > --- linux-pm.orig/drivers/cpuidle/cpuidle.c > +++ linux-pm/drivers/cpuidle/cpuidle.c > @@ -69,11 +69,15 @@ int cpuidle_play_dead(void) > if (!drv) > return -ENODEV; > > - /* Find lowest-power state that supports long-term idle */ > - for (i = drv->state_count - 1; i >= 0; i--) > + for (i = drv->state_count - 1; i >= 0; i--) { > if (drv->states[i].enter_dead) > - return drv->states[i].enter_dead(dev, i); > + drv->states[i].enter_dead(dev, i); > + } > > + /* > + * If :enter_dead() is successful, it will never return, so reaching > + * here means that all of them failed above or were not present. > + */ > return -ENODEV; > } > > > >
Index: linux-pm/drivers/cpuidle/cpuidle.c =================================================================== --- linux-pm.orig/drivers/cpuidle/cpuidle.c +++ linux-pm/drivers/cpuidle/cpuidle.c @@ -69,11 +69,15 @@ int cpuidle_play_dead(void) if (!drv) return -ENODEV; - /* Find lowest-power state that supports long-term idle */ - for (i = drv->state_count - 1; i >= 0; i--) + for (i = drv->state_count - 1; i >= 0; i--) { if (drv->states[i].enter_dead) - return drv->states[i].enter_dead(dev, i); + drv->states[i].enter_dead(dev, i); + } + /* + * If :enter_dead() is successful, it will never return, so reaching + * here means that all of them failed above or were not present. + */ return -ENODEV; }