forked from mirrors/linux
		
	driver core: Fix PM-runtime for links added during consumer probe
Commit4c06c4e6cf("driver core: Fix possible supplier PM-usage counter imbalance") introduced a regression that causes suppliers to be suspended prematurely for device links added during consumer driver probe if the initial PM-runtime status of the consumer is "suspended" and the consumer is resumed after adding the link and before pm_runtime_put_suppliers() is called. In that case, pm_runtime_put_suppliers() will drop the rpm_active refcount for the link by one and (since rpm_active is equal to two after the preceding consumer resume) the supplier's PM-runtime usage counter will be decremented, which may cause the supplier to suspend even though the consumer's PM-runtime status is "active". For this reason, partially revert commit4c06c4e6cfas the problem it tried to fix needs to be addressed somewhat differently, and change pm_runtime_get_suppliers() and pm_runtime_put_suppliers() so that the latter only drops rpm_active references acquired by the former. [This requires adding a new field to struct device_link, but I coulnd't find a cleaner way to address the issue that would work in all cases.] This causes pm_runtime_put_suppliers() to effectively ignore device links added during consumer probe, so device_link_add() doesn't need to worry about ensuring that suppliers will remain active after pm_runtime_put_suppliers() for links created with DL_FLAG_RPM_ACTIVE set and it only needs to bump up rpm_active by one for those links, so pm_runtime_active_link() is not necessary any more. Fixes:4c06c4e6cf("driver core: Fix possible supplier PM-usage counter imbalance") Reported-by: Jon Hunter <jonathanh@nvidia.com> Tested-by: Jon Hunter <jonathanh@nvidia.com> Tested-by: Ulf Hansson <ulf.hansson@linaro.org> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Tested-by: Thierry Reding <treding@nvidia.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
		
							parent
							
								
									e4246b0550
								
							
						
					
					
						commit
						36003d4cf5
					
				
					 4 changed files with 9 additions and 29 deletions
				
			
		| 
						 | 
				
			
			@ -277,7 +277,7 @@ struct device_link *device_link_add(struct device *consumer,
 | 
			
		|||
				link->flags |= DL_FLAG_PM_RUNTIME;
 | 
			
		||||
			}
 | 
			
		||||
			if (flags & DL_FLAG_RPM_ACTIVE)
 | 
			
		||||
				pm_runtime_active_link(link, supplier);
 | 
			
		||||
				refcount_inc(&link->rpm_active);
 | 
			
		||||
		}
 | 
			
		||||
 | 
			
		||||
		if (flags & DL_FLAG_STATELESS) {
 | 
			
		||||
| 
						 | 
				
			
			@ -310,7 +310,7 @@ struct device_link *device_link_add(struct device *consumer,
 | 
			
		|||
 | 
			
		||||
	if (flags & DL_FLAG_PM_RUNTIME) {
 | 
			
		||||
		if (flags & DL_FLAG_RPM_ACTIVE)
 | 
			
		||||
			pm_runtime_active_link(link, supplier);
 | 
			
		||||
			refcount_inc(&link->rpm_active);
 | 
			
		||||
 | 
			
		||||
		pm_runtime_new_link(consumer);
 | 
			
		||||
	}
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -1626,6 +1626,7 @@ void pm_runtime_get_suppliers(struct device *dev)
 | 
			
		|||
 | 
			
		||||
	list_for_each_entry_rcu(link, &dev->links.suppliers, c_node)
 | 
			
		||||
		if (link->flags & DL_FLAG_PM_RUNTIME) {
 | 
			
		||||
			link->supplier_preactivated = true;
 | 
			
		||||
			refcount_inc(&link->rpm_active);
 | 
			
		||||
			pm_runtime_get_sync(link->supplier);
 | 
			
		||||
		}
 | 
			
		||||
| 
						 | 
				
			
			@ -1645,9 +1646,11 @@ void pm_runtime_put_suppliers(struct device *dev)
 | 
			
		|||
	idx = device_links_read_lock();
 | 
			
		||||
 | 
			
		||||
	list_for_each_entry_rcu(link, &dev->links.suppliers, c_node)
 | 
			
		||||
		if (link->flags & DL_FLAG_PM_RUNTIME &&
 | 
			
		||||
		    refcount_dec_not_one(&link->rpm_active))
 | 
			
		||||
		if (link->supplier_preactivated) {
 | 
			
		||||
			link->supplier_preactivated = false;
 | 
			
		||||
			if (refcount_dec_not_one(&link->rpm_active))
 | 
			
		||||
				pm_runtime_put(link->supplier);
 | 
			
		||||
		}
 | 
			
		||||
 | 
			
		||||
	device_links_read_unlock(idx);
 | 
			
		||||
}
 | 
			
		||||
| 
						 | 
				
			
			@ -1659,26 +1662,6 @@ void pm_runtime_new_link(struct device *dev)
 | 
			
		|||
	spin_unlock_irq(&dev->power.lock);
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
/**
 | 
			
		||||
 * pm_runtime_active_link - Set up new device link as active for PM-runtime.
 | 
			
		||||
 * @link: Device link to be set up as active.
 | 
			
		||||
 * @supplier: Supplier end of the link.
 | 
			
		||||
 *
 | 
			
		||||
 * Add 2 to the rpm_active refcount of @link and increment the PM-runtime
 | 
			
		||||
 * usage counter of @supplier once more in case the link is being added while
 | 
			
		||||
 * the consumer driver is probing and pm_runtime_put_suppliers() will be called
 | 
			
		||||
 * subsequently.
 | 
			
		||||
 *
 | 
			
		||||
 * Note that this doesn't prevent rpm_put_suppliers() from decreasing the link's
 | 
			
		||||
 * rpm_active refcount down to one, so runtime suspend of the consumer end of
 | 
			
		||||
 * @link is not affected.
 | 
			
		||||
 */
 | 
			
		||||
void pm_runtime_active_link(struct device_link *link, struct device *supplier)
 | 
			
		||||
{
 | 
			
		||||
	refcount_add(2, &link->rpm_active);
 | 
			
		||||
	pm_runtime_get_noresume(supplier);
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
void pm_runtime_drop_link(struct device *dev)
 | 
			
		||||
{
 | 
			
		||||
	spin_lock_irq(&dev->power.lock);
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -861,6 +861,7 @@ struct device_link {
 | 
			
		|||
#ifdef CONFIG_SRCU
 | 
			
		||||
	struct rcu_head rcu_head;
 | 
			
		||||
#endif
 | 
			
		||||
	bool supplier_preactivated; /* Owned by consumer probe. */
 | 
			
		||||
};
 | 
			
		||||
 | 
			
		||||
/**
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -59,8 +59,6 @@ extern void pm_runtime_clean_up_links(struct device *dev);
 | 
			
		|||
extern void pm_runtime_get_suppliers(struct device *dev);
 | 
			
		||||
extern void pm_runtime_put_suppliers(struct device *dev);
 | 
			
		||||
extern void pm_runtime_new_link(struct device *dev);
 | 
			
		||||
extern void pm_runtime_active_link(struct device_link *link,
 | 
			
		||||
				   struct device *supplier);
 | 
			
		||||
extern void pm_runtime_drop_link(struct device *dev);
 | 
			
		||||
 | 
			
		||||
static inline void pm_suspend_ignore_children(struct device *dev, bool enable)
 | 
			
		||||
| 
						 | 
				
			
			@ -178,8 +176,6 @@ static inline void pm_runtime_clean_up_links(struct device *dev) {}
 | 
			
		|||
static inline void pm_runtime_get_suppliers(struct device *dev) {}
 | 
			
		||||
static inline void pm_runtime_put_suppliers(struct device *dev) {}
 | 
			
		||||
static inline void pm_runtime_new_link(struct device *dev) {}
 | 
			
		||||
static inline void pm_runtime_active_link(struct device_link *link,
 | 
			
		||||
					  struct device *supplier) {}
 | 
			
		||||
static inline void pm_runtime_drop_link(struct device *dev) {}
 | 
			
		||||
 | 
			
		||||
#endif /* !CONFIG_PM */
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
		Loading…
	
		Reference in a new issue