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

After commit ead18c23c2 ("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: ead18c23c2 ("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>
This commit is contained in:
Rafael J. Wysocki 2019-02-01 01:49:14 +01:00 committed by Greg Kroah-Hartman
parent 5db25c9eb8
commit e2f3cd831a
3 changed files with 42 additions and 31 deletions

View File

@ -165,6 +165,19 @@ void device_pm_move_to_tail(struct device *dev)
device_links_read_unlock(idx); 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. * device_link_add - Create a link between two devices.
* @consumer: Consumer end of the link. * @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 *supplier, u32 flags)
{ {
struct device_link *link; struct device_link *link;
bool rpm_put_supplier = false;
if (!consumer || !supplier || if (!consumer || !supplier ||
(flags & DL_FLAG_STATELESS && (flags & DL_FLAG_STATELESS &&
@ -213,7 +225,6 @@ struct device_link *device_link_add(struct device *consumer,
pm_runtime_put_noidle(supplier); pm_runtime_put_noidle(supplier);
return NULL; return NULL;
} }
rpm_put_supplier = true;
} }
device_links_write_lock(); device_links_write_lock();
@ -249,6 +260,15 @@ struct device_link *device_link_add(struct device *consumer,
if (flags & DL_FLAG_AUTOREMOVE_SUPPLIER) if (flags & DL_FLAG_AUTOREMOVE_SUPPLIER)
link->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); kref_get(&link->kref);
goto out; goto out;
} }
@ -257,20 +277,15 @@ struct device_link *device_link_add(struct device *consumer,
if (!link) if (!link)
goto out; goto out;
refcount_set(&link->rpm_active, 1);
if (flags & DL_FLAG_PM_RUNTIME) { if (flags & DL_FLAG_PM_RUNTIME) {
if (flags & DL_FLAG_RPM_ACTIVE) { if (flags & DL_FLAG_RPM_ACTIVE)
link->rpm_active = true; refcount_inc(&link->rpm_active);
rpm_put_supplier = false;
} device_link_rpm_prepare(consumer, 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);
} }
get_device(supplier); get_device(supplier);
link->supplier = supplier; link->supplier = supplier;
INIT_LIST_HEAD(&link->s_node); INIT_LIST_HEAD(&link->s_node);
@ -333,7 +348,7 @@ struct device_link *device_link_add(struct device *consumer,
device_pm_unlock(); device_pm_unlock();
device_links_write_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); pm_runtime_put(supplier);
return link; return link;

View File

@ -259,11 +259,8 @@ static int rpm_get_suppliers(struct device *dev)
list_for_each_entry_rcu(link, &dev->links.suppliers, c_node) { list_for_each_entry_rcu(link, &dev->links.suppliers, c_node) {
int retval; int retval;
if (!(link->flags & DL_FLAG_PM_RUNTIME)) if (!(link->flags & DL_FLAG_PM_RUNTIME) ||
continue; READ_ONCE(link->status) == DL_STATE_SUPPLIER_UNBIND)
if (READ_ONCE(link->status) == DL_STATE_SUPPLIER_UNBIND ||
link->rpm_active)
continue; continue;
retval = pm_runtime_get_sync(link->supplier); 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); pm_runtime_put_noidle(link->supplier);
return retval; return retval;
} }
link->rpm_active = true; refcount_inc(&link->rpm_active);
} }
return 0; return 0;
} }
@ -281,12 +278,13 @@ static void rpm_put_suppliers(struct device *dev)
{ {
struct device_link *link; struct device_link *link;
list_for_each_entry_rcu(link, &dev->links.suppliers, c_node) list_for_each_entry_rcu(link, &dev->links.suppliers, c_node) {
if (link->rpm_active && if (READ_ONCE(link->status) == DL_STATE_SUPPLIER_UNBIND)
READ_ONCE(link->status) != DL_STATE_SUPPLIER_UNBIND) { continue;
while (refcount_dec_not_one(&link->rpm_active))
pm_runtime_put(link->supplier); 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 * 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 * 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. * 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) if (link->flags & DL_FLAG_STATELESS)
continue; continue;
if (link->rpm_active) { while (refcount_dec_not_one(&link->rpm_active))
pm_runtime_put_noidle(dev); pm_runtime_put_noidle(dev);
link->rpm_active = false;
}
} }
device_links_read_unlock(idx); device_links_read_unlock(idx);

View File

@ -853,7 +853,7 @@ struct device_link {
struct list_head c_node; struct list_head c_node;
enum device_link_state status; enum device_link_state status;
u32 flags; u32 flags;
bool rpm_active; refcount_t rpm_active;
struct kref kref; struct kref kref;
#ifdef CONFIG_SRCU #ifdef CONFIG_SRCU
struct rcu_head rcu_head; struct rcu_head rcu_head;