Re: [RFC][PATCH 2/3] PM / sleep: Mechanism to avoid resuming runtime-suspended devices unnecessarily

Rafael J. Wysocki
2014-05-13T16:00:08Z

On Tuesday, May 13, 2014 11:46:55 AM Alan Stern wrote:
> On Tue, 13 May 2014, Rafael J. Wysocki wrote:
>
> > > A wakeup request from the hardware could cause a runtime resume to
> > > occur at this time. The barrier wouldn't prevent that.
> > >
> > > It's unlikely, I agree, but not impossible.
> >
> > Yeah, I didn't think about that.
>
> Come to think of it, if the hardware sends a wakeup request then it
> must have been enabled for remote wakeup. And if the hardware settings
> are appropriate for system suspend then it must be enabled for system
> wakeup. Consequently a wakeup from the hardware ought to abort the
> system suspend in any case. So maybe we don't care about this
> scenario.
>
> On the other hand, there may be other mechanisms that could cause a
> runtime resume at this inconvenient time. A timer routine, for
> instance.
>
> > But that also can occur in __device_suspend(), after we've checked the flag
> > and decided not to invoke the ->suspend() callback, right? So moving the
> > check in there doesn't help much I'd say. It closes the race window, but
> > that's it.
> >
> > That means that the whole approach based on ->prepare() is problematic
> > unless we somehow mix it with disabling runtime PM.
>
> Maybe the call to __pm_runtime_disable() should be moved from
> __device_suspend_late() to __device_suspend(), after the callback has
> been invoked (or skipped, as the case may be). Then after runtime PM
> has been disabled, you can check the device's status has changed and go
> back to invoke the callback if necessary.

We moved __pm_runtime_disable() to __device_suspend_late() to be able to
use pm_runtime_resume() in __device_suspend() (and we actually do that in
some places now).

But, in principle, we can do __pm_runtime_disable() temporarily in some place
between ->prepare() and ->suspend(), it doesn't matter if that's in
device_prepare() in __device_suspend() really. Then, we can check the device's
runtime PM status (that'd need to be done carefully to take the disabling into
account) and
(1) if the device is runtime-suspended, set direct_complete for it without
enabling runtime PM, or
(2) if the device is not runtime-suspended, clear direct_complete for it
and re-enable runtime PM.
and in case of (1) we would re-enable runtime PM in device_complete().

That should work I suppose?

Of course, question is what ->prepare() is supposed to do then if it needs
to check the state of the device before deciding whether or not to return 1.
I guess it would need to disable runtime PM around that check too.

Rafael