Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

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

driver core: Fix possible supplier PM-usage counter imbalance 



If a stateless device link to a certain supplier with
DL_FLAG_PM_RUNTIME set in the flags is added and then removed by the
consumer driver's probe callback, the supplier's PM-runtime usage
counter will be nonzero after that which effectively causes the
supplier to remain "always on" going forward.

Namely, device_link_add() called to add the link invokes
device_link_rpm_prepare() which notices that the consumer driver is
probing, so it increments the supplier's PM-runtime usage counter
with the assumption that the link will stay around until
pm_runtime_put_suppliers() is called by driver_probe_device(),
but if the link goes away before that point, the supplier's
PM-runtime usage counter will remain nonzero.

To prevent that from happening, first rework pm_runtime_get_suppliers()
and pm_runtime_put_suppliers() to use the rpm_active refounts of device
links and make the latter only drop rpm_active and the supplier's
PM-runtime usage counter for each link by one, unless rpm_active is
one already for it.  Next, modify device_link_add() to bump up the
new link's rpm_active refcount and the suppliers PM-runtime usage
counter by two, to prevent pm_runtime_put_suppliers(), if it is
called subsequently, from suspending the supplier prematurely (in
case its PM-runtime usage counter goes down to 0 in there).

Due to the way rpm_put_suppliers() works, this change does not
affect runtime suspend of the consumer ends of new device links (or,
generally, device links for which DL_FLAG_PM_RUNTIME has just been
set).

Fixes: e2f3cd83 ("driver core: Fix handling of runtime PM flags in device_link_add()")
Reported-by: default avatarUlf Hansson <ulf.hansson@linaro.org>
Signed-off-by: default avatarRafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: default avatarUlf Hansson <ulf.hansson@linaro.org>
Tested-by: default avatarUlf Hansson <ulf.hansson@linaro.org>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent c1567f81

drivers/base/core.c

+4 −17
Original line number Diff line number Diff line
@@ -165,19 +165,6 @@ 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.
@@ -286,11 +273,11 @@ struct device_link *device_link_add(struct device *consumer, 


		if (flags & DL_FLAG_PM_RUNTIME) {

			if (!(link->flags & DL_FLAG_PM_RUNTIME)) {

				device_link_rpm_prepare(consumer, supplier);

				pm_runtime_new_link(consumer);

				link->flags |= DL_FLAG_PM_RUNTIME;

			}

			if (flags & DL_FLAG_RPM_ACTIVE)

				refcount_inc(&link->rpm_active);

				pm_runtime_active_link(link, supplier);

		}



		if (flags & DL_FLAG_STATELESS) {
@@ -323,9 +310,9 @@ struct device_link *device_link_add(struct device *consumer, 


	if (flags & DL_FLAG_PM_RUNTIME) {

		if (flags & DL_FLAG_RPM_ACTIVE)

			refcount_inc(&link->rpm_active);

			pm_runtime_active_link(link, supplier);



		device_link_rpm_prepare(consumer, supplier);

		pm_runtime_new_link(consumer);

	}



	get_device(supplier);

drivers/base/power/runtime.c

+25 −2
Original line number Diff line number Diff line
@@ -1625,8 +1625,10 @@ void pm_runtime_get_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)

		if (link->flags & DL_FLAG_PM_RUNTIME) {

			refcount_inc(&link->rpm_active);

			pm_runtime_get_sync(link->supplier);

		}



	device_links_read_unlock(idx);

}
@@ -1643,7 +1645,8 @@ 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)

		if (link->flags & DL_FLAG_PM_RUNTIME &&

		    refcount_dec_not_one(&link->rpm_active))

			pm_runtime_put(link->supplier);



	device_links_read_unlock(idx);
@@ -1656,6 +1659,26 @@ 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/pm_runtime.h

+4 −0
Original line number Diff line number Diff line
@@ -59,6 +59,8 @@ 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)
@@ -176,6 +178,8 @@ 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 */