[PATCH v1] PM: sleep: Update power.completion for all devices on errors

[PATCH v1] PM: sleep: Update power.completion for all devices on errors

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

After commit aa7a9275ab81 ("PM: sleep: Suspend async parents after
suspending children"), the following scenario is possible:

 1. Device A is async and it depends on device B that is sync.
 2. Async suspend is scheduled for A before the processing of B is
    started.
 3. A is waiting for B.
 4. In the meantime, an unrelated device fails to suspend and returns
    an error.
 5. The processing of B doesn't start at all and its power.completion is
    not updated.
 6. A is still waiting for B when async_synchronize_full() is called.
 7. Deadlock ensues.

To prevent this from happening, update power.completion for all devices
on errors in all suspend phases, but do not do it directly for devices
that are already being processed or are waiting for the processing to
start because in those cases it may be necessary to wait for the
processing to actually complete before updating power.completion for
the device.

Fixes: aa7a9275ab81 ("PM: sleep: Suspend async parents after suspending children")
Fixes: 443046d1ad66 ("PM: sleep: Make suspend of devices more asynchronous")
Closes: https://lore.kernel.org/linux-pm/e13740a0-88f3-4a6f-920f-15805071a7d6@linaro.org/
Reported-and-tested-by: Tudor Ambarus <tudor.ambarus@linaro.org>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/base/power/main.c |   19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -1323,6 +1323,22 @@
 	device_links_read_unlock(idx);
 }
 
+static void dpm_async_suspend_complete_all(struct list_head *device_list)
+{
+	struct device *dev;
+
+	guard(mutex)(&async_wip_mtx);
+
+	list_for_each_entry_reverse(dev, device_list, power.entry) {
+		/*
+		 * In case the device is being waited for and async processing
+		 * has not started for it yet, let the waiters make progress.
+		 */
+		if (!dev->power.work_in_progress)
+			complete_all(&dev->power.completion);
+	}
+}
+
 /**
  * resume_event - Return a "resume" message for given "suspend" sleep state.
  * @sleep_state: PM message representing a sleep state.
@@ -1499,6 +1515,7 @@
 		mutex_lock(&dpm_list_mtx);
 
 		if (error || async_error) {
+			dpm_async_suspend_complete_all(&dpm_late_early_list);
 			/*
 			 * Move all devices to the target list to resume them
 			 * properly.
@@ -1701,6 +1718,7 @@
 		mutex_lock(&dpm_list_mtx);
 
 		if (error || async_error) {
+			dpm_async_suspend_complete_all(&dpm_suspended_list);
 			/*
 			 * Move all devices to the target list to resume them
 			 * properly.
@@ -1994,6 +2012,7 @@
 		mutex_lock(&dpm_list_mtx);
 
 		if (error || async_error) {
+			dpm_async_suspend_complete_all(&dpm_prepared_list);
 			/*
 			 * Move all devices to the target list to resume them
 			 * properly.

Re: [PATCH v1] PM: sleep: Update power.completion for all devices on errors

[Ulf Hansson]

On Mon, 14 Jul 2025 at 19:45, Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> After commit aa7a9275ab81 ("PM: sleep: Suspend async parents after
> suspending children"), the following scenario is possible:
>
>  1. Device A is async and it depends on device B that is sync.
>  2. Async suspend is scheduled for A before the processing of B is
>     started.
>  3. A is waiting for B.
>  4. In the meantime, an unrelated device fails to suspend and returns
>     an error.
>  5. The processing of B doesn't start at all and its power.completion is
>     not updated.
>  6. A is still waiting for B when async_synchronize_full() is called.
>  7. Deadlock ensues.
>
> To prevent this from happening, update power.completion for all devices
> on errors in all suspend phases, but do not do it directly for devices
> that are already being processed or are waiting for the processing to
> start because in those cases it may be necessary to wait for the
> processing to actually complete before updating power.completion for
> the device.
>
> Fixes: aa7a9275ab81 ("PM: sleep: Suspend async parents after suspending children")
> Fixes: 443046d1ad66 ("PM: sleep: Make suspend of devices more asynchronous")
> Closes: https://lore.kernel.org/linux-pm/e13740a0-88f3-4a6f-920f-15805071a7d6@linaro.org/
> Reported-and-tested-by: Tudor Ambarus <tudor.ambarus@linaro.org>
> 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 |   19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
>
> --- a/drivers/base/power/main.c
> +++ b/drivers/base/power/main.c
> @@ -1323,6 +1323,22 @@
>         device_links_read_unlock(idx);
>  }
>
> +static void dpm_async_suspend_complete_all(struct list_head *device_list)
> +{
> +       struct device *dev;
> +
> +       guard(mutex)(&async_wip_mtx);
> +
> +       list_for_each_entry_reverse(dev, device_list, power.entry) {
> +               /*
> +                * In case the device is being waited for and async processing
> +                * has not started for it yet, let the waiters make progress.
> +                */
> +               if (!dev->power.work_in_progress)
> +                       complete_all(&dev->power.completion);
> +       }
> +}
> +
>  /**
>   * resume_event - Return a "resume" message for given "suspend" sleep state.
>   * @sleep_state: PM message representing a sleep state.
> @@ -1499,6 +1515,7 @@
>                 mutex_lock(&dpm_list_mtx);
>
>                 if (error || async_error) {
> +                       dpm_async_suspend_complete_all(&dpm_late_early_list);
>                         /*
>                          * Move all devices to the target list to resume them
>                          * properly.
> @@ -1701,6 +1718,7 @@
>                 mutex_lock(&dpm_list_mtx);
>
>                 if (error || async_error) {
> +                       dpm_async_suspend_complete_all(&dpm_suspended_list);
>                         /*
>                          * Move all devices to the target list to resume them
>                          * properly.
> @@ -1994,6 +2012,7 @@
>                 mutex_lock(&dpm_list_mtx);
>
>                 if (error || async_error) {
> +                       dpm_async_suspend_complete_all(&dpm_prepared_list);
>                         /*
>                          * Move all devices to the target list to resume them
>                          * properly.
>
>
>