[PATCH v1 0/3] PM: Tweaks on top of "smart suspend" handling changes

[PATCH v1 0/3] PM: Tweaks on top of "smart suspend" handling changes

[Rafael J. Wysocki]

Hi Everyone,

These are some tweaks based on

https://lore.kernel.org/linux-pm/12612706.O9o76ZdvQC@rjwysocki.net/

which in the meantime has been applied to linux-pm.git/linux-next (as 6.15
material).

None of them is essential, so if you have objections, please let me know,
but IMV they generally will make the code work better.

Please refer to the individual patch changelogs for details.

Thanks!

[PATCH v1 1/3] PM: sleep: Update power.smart_suspend under PM spinlock

[Rafael J. Wysocki]

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

Put the update of the power.smart_suspend device flag under the PM
spinlock of the device in case multiple bit fields in struct dev_pm_info
occupy one memory location which needs to be updated via RMW every time
any of these bit fields is updated.

The lock in question is already held around the power.direct_complete
flag update in device_prepare() for the same reason, so this change does
not add locking-related overhead to the code.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/base/power/main.c |   35 +++++++++++++++++++----------------
 1 file changed, 19 insertions(+), 16 deletions(-)

--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -1789,9 +1789,10 @@
 	return error;
 }
 
-static void device_prepare_smart_suspend(struct device *dev)
+static bool device_prepare_smart_suspend(struct device *dev)
 {
 	struct device_link *link;
+	bool ret = true;
 	int idx;
 
 	/*
@@ -1802,17 +1803,13 @@
 	 * or any of its suppliers that take runtime PM into account, it cannot
 	 * be enabled for the device either.
 	 */
-	dev->power.smart_suspend = dev->power.no_pm_callbacks ||
-		dev_pm_test_driver_flags(dev, DPM_FLAG_SMART_SUSPEND);
-
-	if (!dev_pm_smart_suspend(dev))
-		return;
+	if (!dev->power.no_pm_callbacks &&
+	    !dev_pm_test_driver_flags(dev, DPM_FLAG_SMART_SUSPEND))
+		return false;
 
 	if (dev->parent && !dev_pm_smart_suspend(dev->parent) &&
-	    !dev->parent->power.ignore_children && !pm_runtime_blocked(dev->parent)) {
-		dev->power.smart_suspend = false;
-		return;
-	}
+	    !dev->parent->power.ignore_children && !pm_runtime_blocked(dev->parent))
+		return false;
 
 	idx = device_links_read_lock();
 
@@ -1822,12 +1819,14 @@
 
 		if (!dev_pm_smart_suspend(link->supplier) &&
 		    !pm_runtime_blocked(link->supplier)) {
-			dev->power.smart_suspend = false;
+			ret = false;
 			break;
 		}
 	}
 
 	device_links_read_unlock(idx);
+
+	return ret;
 }
 
 /**
@@ -1841,7 +1840,7 @@
 static int device_prepare(struct device *dev, pm_message_t state)
 {
 	int (*callback)(struct device *) = NULL;
-	bool no_runtime_pm;
+	bool smart_suspend;
 	int ret = 0;
 
 	/*
@@ -1857,7 +1856,7 @@
 	 * suspend-resume cycle is complete, so prepare to trigger a warning on
 	 * subsequent attempts to enable it.
 	 */
-	no_runtime_pm = pm_runtime_block_if_disabled(dev);
+	smart_suspend = !pm_runtime_block_if_disabled(dev);
 
 	if (dev->power.syscore)
 		return 0;
@@ -1893,9 +1892,12 @@
 		return ret;
 	}
 	/* Do not enable "smart suspend" for devices without runtime PM. */
-	if (!no_runtime_pm)
-		device_prepare_smart_suspend(dev);
+	if (smart_suspend)
+		smart_suspend = device_prepare_smart_suspend(dev);
+
+	spin_lock_irq(&dev->power.lock);
 
+	dev->power.smart_suspend = smart_suspend;
 	/*
 	 * A positive return value from ->prepare() means "this device appears
 	 * to be runtime-suspended and its state is fine, so if it really is
@@ -1903,11 +1905,12 @@
 	 * will do the same thing with all of its descendants".  This only
 	 * applies to suspend transitions, however.
 	 */
-	spin_lock_irq(&dev->power.lock);
 	dev->power.direct_complete = state.event == PM_EVENT_SUSPEND &&
 		(ret > 0 || dev->power.no_pm_callbacks) &&
 		!dev_pm_test_driver_flags(dev, DPM_FLAG_NO_DIRECT_COMPLETE);
+
 	spin_unlock_irq(&dev->power.lock);
+
 	return 0;
 }
 

[PATCH v1 2/3] PM: runtime: Convert pm_runtime_blocked() to static inline

[Rafael J. Wysocki]

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

The comment in pm_runtime_blocked() is acutally wrong: power.last_status
is not a bit field.  Its data type is an enum and so one can reasonably
assume that partial updates of it will not be observed.

Accordingly, pm_runtime_blocked() can be converted to a static inline
function and the related locking overhead can be eliminated, so long
as it is only used in system suspend/resume code paths because
power.last_status is not expected to be updated concurrently while
that code is running.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/base/power/runtime.c |   17 -----------------
 include/linux/pm_runtime.h   |   12 +++++++++++-
 2 files changed, 11 insertions(+), 18 deletions(-)

--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -1560,23 +1560,6 @@
 }
 EXPORT_SYMBOL_GPL(pm_runtime_enable);
 
-bool pm_runtime_blocked(struct device *dev)
-{
-	bool ret;
-
-	/*
-	 * dev->power.last_status is a bit field, so in case it is updated via
-	 * RMW, read it under the spin lock.
-	 */
-	spin_lock_irq(&dev->power.lock);
-
-	ret = dev->power.last_status == RPM_BLOCKED;
-
-	spin_unlock_irq(&dev->power.lock);
-
-	return ret;
-}
-
 static void pm_runtime_disable_action(void *data)
 {
 	pm_runtime_dont_use_autosuspend(data);
--- a/include/linux/pm_runtime.h
+++ b/include/linux/pm_runtime.h
@@ -81,7 +81,6 @@
 extern void pm_runtime_unblock(struct device *dev);
 extern void pm_runtime_enable(struct device *dev);
 extern void __pm_runtime_disable(struct device *dev, bool check_resume);
-extern bool pm_runtime_blocked(struct device *dev);
 extern void pm_runtime_allow(struct device *dev);
 extern void pm_runtime_forbid(struct device *dev);
 extern void pm_runtime_no_callbacks(struct device *dev);
@@ -200,6 +199,17 @@
 }
 
 /**
+ * pm_runtime_blocked - Check if runtime PM enabling is blocked.
+ * @dev: Target device.
+ *
+ * Do not call this function outside system suspend/resume code paths.
+ */
+static inline bool pm_runtime_blocked(struct device *dev)
+{
+	return dev->power.last_status == RPM_BLOCKED;
+}
+
+/**
  * pm_runtime_has_no_callbacks - Check if runtime PM callbacks may be present.
  * @dev: Target device.
  *

[PATCH v1 3/3] PM: core: Tweak pm_runtime_block_if_disabled() return value

[Rafael J. Wysocki]

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

Modify pm_runtime_block_if_disabled() to return true when runtime PM
is disabled for the device, regardless of the power.last_status value.

This effectively prevents "smart suspend" from being enabled for
devices with runtime PM disabled in device_prepare(), even transiently,
so update the related comment in that function accordingly.

If a device has runtime PM disabled in device_prepare(), it is not
actually known whether or not runtime PM will be enabled for that
device going forward, so it is more appropriate to postpone the
"smart suspend" optimization for the device in the given system
suspend-resume cycle than to enable it and get confused going
forward.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/base/power/main.c    |    2 +-
 drivers/base/power/runtime.c |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -1891,7 +1891,7 @@
 		pm_runtime_put(dev);
 		return ret;
 	}
-	/* Do not enable "smart suspend" for devices without runtime PM. */
+	/* Do not enable "smart suspend" for devices with disabled runtime PM. */
 	if (smart_suspend)
 		smart_suspend = device_prepare_smart_suspend(dev);
 
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -1466,8 +1466,8 @@
 
 	spin_lock_irq(&dev->power.lock);
 
-	ret = dev->power.disable_depth && dev->power.last_status == RPM_INVALID;
-	if (ret)
+	ret = !pm_runtime_enabled(dev);
+	if (ret && dev->power.last_status == RPM_INVALID)
 		dev->power.last_status = RPM_BLOCKED;
 
 	spin_unlock_irq(&dev->power.lock);

Re: [PATCH v1 0/3] PM: Tweaks on top of "smart suspend" handling changes

[Ulf Hansson]

On Thu, 27 Feb 2025 at 11:49, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>
> Hi Everyone,
>
> These are some tweaks based on
>
> https://lore.kernel.org/linux-pm/12612706.O9o76ZdvQC@rjwysocki.net/
>
> which in the meantime has been applied to linux-pm.git/linux-next (as 6.15
> material).
>
> None of them is essential, so if you have objections, please let me know,
> but IMV they generally will make the code work better.
>
> Please refer to the individual patch changelogs for details.

For the series:

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

Kind regards
Uffe