Commit e2f3cd83 authored by Rafael J. Wysocki's avatar Rafael J. Wysocki Committed by Greg Kroah-Hartman
Browse files

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



After commit ead18c23 ("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: ead18c23 ("driver core: Introduce device links reference counting")
Signed-off-by: default avatarRafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 5db25c9e

drivers/base/core.c

+30 −15
Original line number Diff line number Diff line
@@ -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;

drivers/base/power/runtime.c

+11 −15
Original line number Diff line number Diff line
@@ -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,11 +278,12 @@ 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);

include/linux/device.h

+1 −1
Original line number Diff line number Diff line
@@ -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;

CRA Git | Maintained and supported by SUSTech CRA and CCSE