Re: [PATCH v2] PM: sleep: core: Fix the handling of pending runtime resume requests

[Mika Westerberg]

Hi Rafael,

On Mon, Aug 24, 2020 at 07:35:31PM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> 
> It has been reported that system-wide suspend may be aborted in the
> absence of any wakeup events due to unforseen interactions of it with
> the runtume PM framework.
> 
> One failing scenario is when there are multiple devices sharing an
> ACPI power resource and runtime-resume needs to be carried out for
> one of them during system-wide suspend (for example, because it needs
> to be reconfigured before the whole system goes to sleep).  In that
> case, the runtime-resume of that device involves turning the ACPI
> power resource "on" which in turn causes runtime-resume requests
> to be queued up for all of the other devices sharing it.  Those
> requests go to the runtime PM workqueue which is frozen during
> system-wide suspend, so they are not actually taken care of until
> the resume of the whole system, but the pm_runtime_barrier()
> call in __device_suspend() sees them and triggers system wakeup
> events for them which then cause the system-wide suspend to be
> aborted if wakeup source objects are in active use.
> 
> Of course, the logic that leads to triggering those wakeup events is
> questionable in the first place, because clearly there are cases in
> which a pending runtime resume request for a device is not connected
> to any real wakeup events in any way (like the one above).  Moreover,
> it is racy, because the device may be resuming already by the time
> the pm_runtime_barrier() runs and so if the driver doesn't take care
> of signaling the wakeup event as appropriate, it will be lost.
> However, if the driver does take care of that, the extra
> pm_wakeup_event() call in the core is redundant.
> 
> Accordingly, drop the conditional pm_wakeup_event() call fron
> __device_suspend() and make the latter call pm_runtime_barrier()
> alone.  Also modify the comment next to that call to reflect the new
> code and extend it to mention the need to avoid unwanted interactions
> between runtime PM and system-wide device suspend callbacks.
> 
> Fixes: 1e2ef05bb8cf8 ("PM: Limit race conditions between runtime PM and system sleep (v2)")
> Reported-by: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>

I guess the more correct here is

Reported-by: Utkarsh H Patel <utkarsh.h.patel@xxxxxxxxx>

> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>

I also got confirmation that this fixes the reported issue and did not
seem to cause regressions either :)

Please add the following tags:

Tested-by: Utkarsh H Patel <utkarsh.h.patel@xxxxxxxxx>
Tested-by: Pengfei Xu <pengfei.xu@xxxxxxxxx>

Thanks for fixing this!