driver core: Fix handling of runtime PM flags in device_link_add()
After commitead18c23c2("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:
		
							parent
							
								
									5db25c9eb8
								
							
						
					
					
						commit
						e2f3cd831a
					
				| @ -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; | ||||
|  | ||||
| @ -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); | ||||
|  | ||||
| @ -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; | ||||
|  | ||||
		Loading…
	
		Reference in New Issue
	
	Block a user