driver core: Fix handling of runtime PM flags in device_link_add()

After commit ead18c23c263 ("driver core: Introduce device links
reference counting"), if there is a link between the given supplier
and the given consumer already, device_link_add() will refcount it
and return it unconditionally without updating its flags.  It is
possible, however, that the second (or any subsequent) caller of
device_link_add() for the same consumer-supplier pair will pass
DL_FLAG_PM_RUNTIME, possibly along with DL_FLAG_RPM_ACTIVE, in flags
to it and the existing link may not behave as expected then.

First, if DL_FLAG_PM_RUNTIME is not set in the existing link's flags
at all, it needs to be set like during the original initialization of
the link.

Second, if DL_FLAG_RPM_ACTIVE is passed to device_link_add() in flags
(in addition to DL_FLAG_PM_RUNTIME), the existing link should to be
updated to reflect the "active" runtime PM configuration of the
consumer-supplier pair and extra care must be taken here to avoid
possible destructive races with runtime PM of the consumer.

To that end, redefine the rpm_active field in struct device_link
as a refcount, initialize it to 1 and make rpm_resume() (for the
consumer) and device_link_add() increment it whenever they acquire
a runtime PM reference on the supplier device.  Accordingly, make
rpm_suspend() (for the consumer) and pm_runtime_clean_up_links()
decrement it and drop runtime PM references to the supplier
device in a loop until rpm_active becones 1 again.

Fixes: ead18c23c263 ("driver core: Introduce device links reference counting")
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 50610cd87e7110ad1a0b7968275d9d24cf6e6d58..8611385e44b507ecf973be9ad7ff01a7f20f1537 100644 (file)
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -165,6 +165,19 @@ void device_pm_move_to_tail(struct device *dev)
        device_links_read_unlock(idx);
 }
 
+static void device_link_rpm_prepare(struct device *consumer,
+                                   struct device *supplier)
+{
+       pm_runtime_new_link(consumer);
+       /*
+        * If the link is being added by the consumer driver at probe time,
+        * balance the decrementation of the supplier's runtime PM usage counter
+        * after consumer probe in driver_probe_device().
+        */
+       if (consumer->links.status == DL_DEV_PROBING)
+               pm_runtime_get_noresume(supplier);
+}
+
 /**
  * device_link_add - Create a link between two devices.
  * @consumer: Consumer end of the link.
@@ -201,7 +214,6 @@ struct device_link *device_link_add(struct device *consumer,
                                    struct device *supplier, u32 flags)
 {
        struct device_link *link;
-       bool rpm_put_supplier = false;
 
        if (!consumer || !supplier ||
            (flags & DL_FLAG_STATELESS &&
@@ -213,7 +225,6 @@ struct device_link *device_link_add(struct device *consumer,
                        pm_runtime_put_noidle(supplier);
                        return NULL;
                }
-               rpm_put_supplier = true;
        }
 
        device_links_write_lock();
@@ -249,6 +260,15 @@ struct device_link *device_link_add(struct device *consumer,
                if (flags & DL_FLAG_AUTOREMOVE_SUPPLIER)
                        link->flags |= DL_FLAG_AUTOREMOVE_SUPPLIER;
 
+               if (flags & DL_FLAG_PM_RUNTIME) {
+                       if (!(link->flags & DL_FLAG_PM_RUNTIME)) {
+                               device_link_rpm_prepare(consumer, supplier);
+                               link->flags |= DL_FLAG_PM_RUNTIME;
+                       }
+                       if (flags & DL_FLAG_RPM_ACTIVE)
+                               refcount_inc(&link->rpm_active);
+               }
+
                kref_get(&link->kref);
                goto out;
        }
@@ -257,20 +277,15 @@ struct device_link *device_link_add(struct device *consumer,
        if (!link)
                goto out;
 
+       refcount_set(&link->rpm_active, 1);
+
        if (flags & DL_FLAG_PM_RUNTIME) {
-               if (flags & DL_FLAG_RPM_ACTIVE) {
-                       link->rpm_active = true;
-                       rpm_put_supplier = false;
-               }
-               pm_runtime_new_link(consumer);
-               /*
-                * If the link is being added by the consumer driver at probe
-                * time, balance the decrementation of the supplier's runtime PM
-                * usage counter after consumer probe in driver_probe_device().
-                */
-               if (consumer->links.status == DL_DEV_PROBING)
-                       pm_runtime_get_noresume(supplier);
+               if (flags & DL_FLAG_RPM_ACTIVE)
+                       refcount_inc(&link->rpm_active);
+
+               device_link_rpm_prepare(consumer, supplier);
        }
+
        get_device(supplier);
        link->supplier = supplier;
        INIT_LIST_HEAD(&link->s_node);
@@ -333,7 +348,7 @@ struct device_link *device_link_add(struct device *consumer,
        device_pm_unlock();
        device_links_write_unlock();
 
-       if (rpm_put_supplier)
+       if ((flags & DL_FLAG_PM_RUNTIME && flags & DL_FLAG_RPM_ACTIVE) && !link)
                pm_runtime_put(supplier);
 
        return link;

diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index 457be03b744d276c9279b2a41686afdc49a05e20..8bc9a432de7086db1746c63db2f75d7118254332 100644 (file)
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -259,11 +259,8 @@ static int rpm_get_suppliers(struct device *dev)
        list_for_each_entry_rcu(link, &dev->links.suppliers, c_node) {
                int retval;
 
-               if (!(link->flags & DL_FLAG_PM_RUNTIME))
-                       continue;
-
-               if (READ_ONCE(link->status) == DL_STATE_SUPPLIER_UNBIND ||
-                   link->rpm_active)
+               if (!(link->flags & DL_FLAG_PM_RUNTIME) ||
+                   READ_ONCE(link->status) == DL_STATE_SUPPLIER_UNBIND)
                        continue;
 
                retval = pm_runtime_get_sync(link->supplier);
@@ -272,7 +269,7 @@ static int rpm_get_suppliers(struct device *dev)
                        pm_runtime_put_noidle(link->supplier);
                        return retval;
                }
-               link->rpm_active = true;
+               refcount_inc(&link->rpm_active);
        }
        return 0;
 }
@@ -281,12 +278,13 @@ static void rpm_put_suppliers(struct device *dev)
 {
        struct device_link *link;
 
-       list_for_each_entry_rcu(link, &dev->links.suppliers, c_node)
-               if (link->rpm_active &&
-                   READ_ONCE(link->status) != DL_STATE_SUPPLIER_UNBIND) {
+       list_for_each_entry_rcu(link, &dev->links.suppliers, c_node) {
+               if (READ_ONCE(link->status) == DL_STATE_SUPPLIER_UNBIND)
+                       continue;
+
+               while (refcount_dec_not_one(&link->rpm_active))
                        pm_runtime_put(link->supplier);
-                       link->rpm_active = false;
-               }
+       }
 }
 
 /**
@@ -1539,7 +1537,7 @@ void pm_runtime_remove(struct device *dev)
  *
  * Check links from this device to any consumers and if any of them have active
  * runtime PM references to the device, drop the usage counter of the device
- * (once per link).
+ * (as many times as needed).
  *
  * Links with the DL_FLAG_STATELESS flag set are ignored.
  *
@@ -1561,10 +1559,8 @@ void pm_runtime_clean_up_links(struct device *dev)
                if (link->flags & DL_FLAG_STATELESS)
                        continue;
 
-               if (link->rpm_active) {
+               while (refcount_dec_not_one(&link->rpm_active))
                        pm_runtime_put_noidle(dev);
-                       link->rpm_active = false;
-               }
        }
 
        device_links_read_unlock(idx);

diff --git a/include/linux/device.h b/include/linux/device.h
index d0e452fd0bff2e4a95d13be105a21d808d7dd9ba..5f49d2eff6ed93af3261457de4e6720e7686ca80 100644 (file)
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -853,7 +853,7 @@ struct device_link {
        struct list_head c_node;
        enum device_link_state status;
        u32 flags;
-       bool rpm_active;
+       refcount_t rpm_active;
        struct kref kref;
 #ifdef CONFIG_SRCU
        struct rcu_head rcu_head;