[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <12627587.O9o76ZdvQC@rjwysocki.net>
Date: Thu, 13 Mar 2025 17:00:00 +0100
From: "Rafael J. Wysocki" <rjw@...ysocki.net>
To: Linux PM <linux-pm@...r.kernel.org>
Cc: LKML <linux-kernel@...r.kernel.org>,
Alan Stern <stern@...land.harvard.edu>, Ulf Hansson <ulf.hansson@...aro.org>,
Johan Hovold <johan@...nel.org>,
Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
Jon Hunter <jonathanh@...dia.com>, Saravana Kannan <saravanak@...gle.com>
Subject:
[PATCH v1] PM: sleep: Fix handling devices with direct_complete set on errors
From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
When dpm_suspend() fails, some devices with power.direct_complete set
may not have been handled by device_suspend() yet, so runtime PM has
not been disabled for them yet even though power.direct_complete is set.
Since device_resume() expects that runtime PM has been disabled for all
devices with power.direct_complete set, it will attempt to reenable
runtime PM for the devices that have not been processed by device_suspend()
which does not make sense. Had those devices had runtime PM disabled
before device_suspend() had run, device_resume() would have inadvertently
enable runtime PM for them, but this is not expected to happen because
it would require ->prepare() callbacks to return positive values for
devices with runtime PM disabled, which would be invalid.
In practice, this issue is most likely benign because pm_runtime_enable()
will not allow the "disable depth" counter to underflow, but it causes a
warning message to be printed for each affected device.
To allow device_resume() to distinguish the "direct complete" devices
that have been processed by device_suspend() from those which have not
been handled by it, make device_suspend() set power.is_suspended for
"direct complete" devices.
Next, move the power.is_suspended check in device_resume() before the
power.direct_complete check in it to make it skip the "direct complete"
devices that have not been handled by device_suspend().
This change is based on a preliminary patch from Saravana Kannan.
Fixes: aae4518b3124 ("PM / sleep: Mechanism to avoid resuming runtime-suspended devices unnecessarily")
Link: https://lore.kernel.org/linux-pm/20241114220921.2529905-2-saravanak@google.com/
Reported-by: Saravana Kannan <saravanak@...gle.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
---
drivers/base/power/main.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -938,6 +938,9 @@
if (dev->power.syscore)
goto Complete;
+ if (!dev->power.is_suspended)
+ goto Complete;
+
if (dev->power.direct_complete) {
/*
* Allow new children to be added under the device after this
@@ -963,9 +966,6 @@
*/
dev->power.is_prepared = false;
- if (!dev->power.is_suspended)
- goto Unlock;
-
if (dev->pm_domain) {
info = "power domain ";
callback = pm_op(&dev->pm_domain->ops, state);
@@ -1005,7 +1005,6 @@
error = dpm_run_callback(callback, dev, state, info);
dev->power.is_suspended = false;
- Unlock:
device_unlock(dev);
dpm_watchdog_clear(&wd);
@@ -1669,6 +1668,7 @@
pm_runtime_disable(dev);
if (pm_runtime_status_suspended(dev)) {
pm_dev_dbg(dev, state, "direct-complete ");
+ dev->power.is_suspended = true;
goto Complete;
}
Powered by blists - more mailing lists