PM: runtime: Fixes related to device links management

drivers/base/dd.c +0 -2

  				return;
  		}
 
-		pm_runtime_get_sync(dev);
-
  		driver_sysfs_remove(dev);
 
  		if (dev->bus)

.

Hi Rafael,

Quoted 35 lines · Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

Since the device is resumed from runtime-suspend in __device_release_driver() anyway, it is better to do that before looking for busy managed device links from it to consumers, because if there are any, device_links_unbind_consumers() will be called and it will cause the consumer devices' drivers to unbind, so the consumer devices will be runtime-resumed. In turn, resuming each consumer device will cause the supplier to be resumed and when the runtime PM references from the given consumer to it are dropped, it may be suspended. Then, the runtime-resume of the next consumer will cause the supplier to resume again and so on.

Update the code accordingly.

Signed-off-by Rafael J. Wysocki <[email protected]>

Fixes 9ed9895370ae ("driver core: Functional dependencies tracking support")

Cc All applicable <[email protected]> # All applicable

---
  drivers/base/dd.c |    4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

Index: linux-pm/drivers/base/dd.c
===================================================================

drivers/base/dd.c +2 -0

@@ -1117,6 +1117,8 @@ static void __device_release_driver(stru
 
      drv = dev->driver;
      if (drv) {
+             pm_runtime_get_sync(dev);
+
              while (device_links_busy(dev)) {
                      __device_driver_unlock(dev, parent);
 

drivers/base/dd.c static void __device_release_driver(stru

Quoted 1 line

drivers/base/dd.c +0 -0

@@ -1132,8 +1134,6 @@ static void __device_release_driver(stru

pm_runtime_put_sync() is required to be called if existed from here.

Quoted 1 line

drivers/base/dd.c +0 -0

                              return;

Quoted 7 lines

drivers/base/dd.c +0 -2

              }
 
-             pm_runtime_get_sync(dev);
-
              driver_sysfs_remove(dev);
 
              if (dev->bus)

Hi Rafael,

Quoted 30 lines · Rafael J. Wysocki

Hi Greg & all,

Commit d12544fb2aa9 ("PM: runtime: Remove link state checks in rpm_get/put_supplier()") merged recently introduced a weakness in the handling of device links in the runtime PM framework that may be confusing and even harmful.

Namely, the checks removed by that commit prevented PM-runtime from getting or dropping references to the supplier device whose driver was going away via its links to consumers, which specifically allowed the pm_runtime_clean_up_links() called from __device_release_driver() to run without interfering with runtime suspend/resume of consumer devices (which still might happen even though the drivers had been unbound from them by that time).

After the above commit, calling pm_runtime_clean_up_links() from __device_release_driver() makes a little sense and it may be interfering destructively with regular PM-runtime suspend/resume control flows, so it needs to be either fixed or dropped altogether. I prefer the latter, because among other things this removes an arbitrary difference in the handling of managed device links with respect to the stateless ones, so patch [2/3] is doing just that.

However, in some rare cases pm_runtime_clean_up_links() may help to clean up leftover PM-runtime references, so if that function goes away, they need to be cleaned up elsewhere. That's why patch [1/3] modifies __device_link_del() to drop them upon device link removal (which also needs to be done for stateless device links and that's why I'm regarding this patch as a fix).

Quoted 4 lines

Finally, to avoid pointless overhead related to suspending and resuming the target device for multiple times in a row in __device_release_driver(), it is better to resume it upfront before checking its links to consumers, which is done by patch [3/3].

I have tested the patchset, and it solves my reported issue, so please feel free to add :

Tested-by Xiang Chen <[email protected]>

Hi Greg & all,

Commit d12544fb2aa9 ("PM: runtime: Remove link state checks in rpm_get/put_supplier()") merged recently introduced a weakness in the handling of device links in the runtime PM framework that may be confusing and even harmful.

Namely, the checks removed by that commit prevented PM-runtime from getting or dropping references to the supplier device whose driver was going away via its links to consumers, which specifically allowed the pm_runtime_clean_up_links() called from __device_release_driver() to run without interfering with runtime suspend/resume of consumer devices (which still might happen even though the drivers had been unbound from them by that time).

After the above commit, calling pm_runtime_clean_up_links() from __device_release_driver() makes a little sense and it may be interfering destructively with regular PM-runtime suspend/resume control flows, so it needs to be either fixed or dropped altogether. I prefer the latter, because among other things this removes an arbitrary difference in the handling of managed device links with respect to the stateless ones, so patch [2/3] is doing just that.

However, in some rare cases pm_runtime_clean_up_links() may help to clean up leftover PM-runtime references, so if that function goes away, they need to be cleaned up elsewhere. That's why patch [1/3] modifies __device_link_del() to drop them upon device link removal (which also needs to be done for stateless device links and that's why I'm regarding this patch as a fix).

Finally, to avoid pointless overhead related to suspending and resuming the target device for multiple times in a row in __device_release_driver(), it is better to resume it upfront before checking its links to consumers, which is done by patch [3/3].

While this series touches the driver core, it really is mostly related to runtime PM, so I can apply it if that's OK.

Hi Greg & all,

Commit d12544fb2aa9 ("PM: runtime: Remove link state checks in rpm_get/put_supplier()") merged recently introduced a weakness in the handling of device links in the runtime PM framework that may be confusing and even harmful.

Namely, the checks removed by that commit prevented PM-runtime from getting or dropping references to the supplier device whose driver was going away via its links to consumers, which specifically allowed the pm_runtime_clean_up_links() called from __device_release_driver() to run without interfering with runtime suspend/resume of consumer devices (which still might happen even though the drivers had been unbound from them by that time).

After the above commit, calling pm_runtime_clean_up_links() from __device_release_driver() makes a little sense and it may be interfering destructively with regular PM-runtime suspend/resume control flows, so it needs to be either fixed or dropped altogether. I prefer the latter, because among other things this removes an arbitrary difference in the handling of managed device links with respect to the stateless ones, so patch [2/3] is doing just that.

However, in some rare cases pm_runtime_clean_up_links() may help to clean up leftover PM-runtime references, so if that function goes away, they need to be cleaned up elsewhere. That's why patch [1/3] modifies __device_link_del() to drop them upon device link removal (which also needs to be done for stateless device links and that's why I'm regarding this patch as a fix).

Finally, to avoid pointless overhead related to suspending and resuming the target device for multiple times in a row in __device_release_driver(), it is better to resume it upfront before checking its links to consumers, which is done by patch [3/3].

While this series touches the driver core, it really is mostly related to runtime PM, so I can apply it if that's OK.