[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1665211.FOmLsISsVf@aspire.rjw.lan>
Date: Thu, 24 Jan 2019 12:21:22 +0100
From: "Rafael J. Wysocki" <rjw@...ysocki.net>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: LKML <linux-kernel@...r.kernel.org>,
Linux PM <linux-pm@...r.kernel.org>,
Ulf Hansson <ulf.hansson@...aro.org>,
Daniel Vetter <daniel@...ll.ch>,
Lukas Wunner <lukas@...ner.de>,
Andrzej Hajda <a.hajda@...sung.com>,
Russell King - ARM Linux <linux@...linux.org.uk>,
Lucas Stach <l.stach@...gutronix.de>,
Linus Walleij <linus.walleij@...aro.org>,
Thierry Reding <thierry.reding@...il.com>,
Laurent Pinchart <laurent.pinchart@...asonboard.com>
Subject: [PATCH 5/6] driver core: Fix handling of runtime PM flags in device_link_add()
From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
After commit ead18c23c263 ("driver core: Introduce device links
reference counting"), if 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. In that case DL_FLAG_RPM_ACTIVE should take effect like
during the original initialization of the link too.
Second, however, if DL_FLAG_PM_RUNTIME is set in the existing link's
flags, attempting to cause DL_FLAG_RPM_ACTIVE to take its normal
effect is generally unsafe and it is better to return NULL from
device_link_add() in that case (to indicate to the caller that the
expected behavior of the new device link could not be guaranteed).
Modify device_link_add() to behave as per the above.
[Note that the change in behavior regarding DL_FLAG_RPM_ACTIVE should
not affect the existing users of that flag.]
Fixes: ead18c23c263 ("driver core: Introduce device links reference counting")
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
---
drivers/base/core.c | 56 +++++++++++++++++++++++++++++++++++-----------------
1 file changed, 38 insertions(+), 18 deletions(-)
Index: linux-pm/drivers/base/core.c
===================================================================
--- linux-pm.orig/drivers/base/core.c
+++ linux-pm/drivers/base/core.c
@@ -165,6 +165,23 @@ void device_pm_move_to_tail(struct devic
device_links_read_unlock(idx);
}
+static void device_link_rpm_prepare(struct device *consumer,
+ struct device *supplier,
+ struct device_link *link, u32 flags)
+{
+ if (flags & DL_FLAG_RPM_ACTIVE)
+ link->rpm_active = true;
+
+ 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.
@@ -177,7 +194,9 @@ void device_pm_move_to_tail(struct devic
* DL_FLAG_RPM_ACTIVE flag is set in addition to it, the supplier devices will
* be forced into the active metastate and reference-counted upon the creation
* of the link. If DL_FLAG_PM_RUNTIME is not set, DL_FLAG_RPM_ACTIVE will be
- * ignored.
+ * ignored. However, passing both DL_FLAG_PM_RUNTIME and DL_FLAG_RPM_ACTIVE in
+ * @flags will cause NULL to be returned if an existing link between @consumer
+ * and @supplier with the DL_FLAG_PM_RUNTIME flag set is found.
*
* If the DL_FLAG_AUTOREMOVE_CONSUMER flag is set, the reference to the link
* acquired by this function will be dropped automatically when the consumer
@@ -202,7 +221,6 @@ struct device_link *device_link_add(stru
struct device *supplier, u32 flags)
{
struct device_link *link;
- bool rpm_put_supplier = false;
if (!consumer || !supplier ||
(flags & DL_FLAG_STATELESS &&
@@ -214,7 +232,6 @@ struct device_link *device_link_add(stru
pm_runtime_put_noidle(supplier);
return NULL;
}
- rpm_put_supplier = true;
}
device_links_write_lock();
@@ -250,6 +267,20 @@ struct device_link *device_link_add(stru
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) {
+ if (WARN_ON(flags & DL_FLAG_RPM_ACTIVE)) {
+ link = NULL;
+ goto out;
+ }
+ } else {
+ link->flags |= DL_FLAG_PM_RUNTIME;
+
+ device_link_rpm_prepare(consumer, supplier,
+ link, flags);
+ }
+ }
+
kref_get(&link->kref);
goto out;
}
@@ -258,20 +289,9 @@ struct device_link *device_link_add(stru
if (!link)
goto out;
- 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_PM_RUNTIME)
+ device_link_rpm_prepare(consumer, supplier, link, flags);
+
get_device(supplier);
link->supplier = supplier;
INIT_LIST_HEAD(&link->s_node);
@@ -334,7 +354,7 @@ struct device_link *device_link_add(stru
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;
Powered by blists - more mailing lists