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

driver core: Fix PM-runtime for links added during consumer probe 



Commit 4c06c4e6 ("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 commit 4c06c4e6 as 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: 4c06c4e6 ("driver core: Fix possible supplier PM-usage counter imbalance")
Reported-by: default avatarJon Hunter <jonathanh@nvidia.com>
Tested-by: default avatarJon Hunter <jonathanh@nvidia.com>
Tested-by: default avatarUlf Hansson <ulf.hansson@linaro.org>
Reviewed-by: default avatarUlf Hansson <ulf.hansson@linaro.org>
Signed-off-by: default avatarRafael J. Wysocki <rafael.j.wysocki@intel.com>
Tested-by: default avatarThierry Reding <treding@nvidia.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent e4246b05

drivers/base/core.c

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

	}

drivers/base/power/runtime.c

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

include/linux/device.h

+1 −0
Original line number Diff line number Diff line
@@ -861,6 +861,7 @@ struct device_link { 
#ifdef CONFIG_SRCU

	struct rcu_head rcu_head;

#endif

	bool supplier_preactivated; /* Owned by consumer probe. */

};



/**

include/linux/pm_runtime.h

+0 −4
Original line number Diff line number Diff line
@@ -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 */

CRA Git | Maintained and supported by SUSTech CRA and CCSE