Messages in this thread | | | From | Ulf Hansson <> | Date | Thu, 4 Oct 2018 15:22:29 +0200 | Subject | Re: [PATCH] PM / core: Clear the direct_complete flag on errors |
| |
On 4 October 2018 at 11:08, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > If __device_suspend() returns early on an error or pending wakeup > and the power.direct_complete flag has been set for the device > already, the subsequent device_resume() will be confused by it > and it will call pm_runtime_enable() incorrectly, as runtime PM > has not been disabled for the device by __device_suspend().
I think it would be fair to mention that is related to the async suspend path, in dpm_suspend().
> > To avoid that, clear power.direct_complete if __device_suspend() > is not going to disable runtime PM for the device before returning.
Overall, by looking at the behavior in dpm_suspend() of async suspended devices, it does look a bit fragile to me.
My worries is that we put asynced suspended devices in the dpm_suspended_list, no matter if the device was successfully suspended or not. This differs from the no-async path.
In the long run, maybe we should change that instead?
> > Fixes: aae4518b3124 (PM / sleep: Mechanism to avoid resuming runtime-suspended devices unnecessarily) > Reported-by: Al Cooper <alcooperx@gmail.com> > Cc: 3.16+ <stable@vger.kernel.org> # 3.16+ > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
Kind regards Uffe
> --- > drivers/base/power/main.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > Index: linux-pm/drivers/base/power/main.c > =================================================================== > --- linux-pm.orig/drivers/base/power/main.c > +++ linux-pm/drivers/base/power/main.c > @@ -1713,8 +1713,10 @@ static int __device_suspend(struct devic > > dpm_wait_for_subordinate(dev, async); > > - if (async_error) > + if (async_error) { > + dev->power.direct_complete = false; > goto Complete; > + } > > /* > * If a device configured to wake up the system from sleep states > @@ -1726,6 +1728,7 @@ static int __device_suspend(struct devic > pm_wakeup_event(dev, 0); > > if (pm_wakeup_pending()) { > + dev->power.direct_complete = false; > async_error = -EBUSY; > goto Complete; > } >
| |