aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorLukas Wunner <lukas@wunner.de>2018-02-10 13:27:12 -0500
committerRafael J. Wysocki <rafael.j.wysocki@intel.com>2018-02-27 12:10:42 -0500
commitead18c23c263374ed098a7d955b29b4a466d4573 (patch)
tree17e0951e486bd5ba4b757028c0ffef88c4d34715
parentda997b22c40473b7db60bde6ea188d35565d10c8 (diff)
driver core: Introduce device links reference counting
If device_link_add() is invoked multiple times with the same supplier and consumer combo, it will create the link on first addition and return a pointer to the already existing link on all subsequent additions. The semantics for device_link_del() are quite different, it deletes the link unconditionally, so multiple invocations are not allowed. In other words, this snippet ... struct device *dev1, *dev2; struct device_link *link1, *link2; link1 = device_link_add(dev1, dev2, 0); link2 = device_link_add(dev1, dev2, 0); device_link_del(link1); device_link_del(link2); ... causes the following crash: WARNING: CPU: 4 PID: 2686 at drivers/base/power/runtime.c:1611 pm_runtime_drop_link+0x40/0x50 [...] list_del corruption, 0000000039b800a4->prev is LIST_POISON2 (00000000ecf79852) kernel BUG at lib/list_debug.c:50! The issue isn't as arbitrary as it may seem: Imagine a device link which is added in both the supplier's and the consumer's ->probe hook. The two drivers can't just call device_link_del() in their ->remove hook without coordination. Fix by counting multiple additions and dropping the device link only when the last addition is unwound. Signed-off-by: Lukas Wunner <lukas@wunner.de> [ rjw: Subject ] Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
-rw-r--r--drivers/base/core.c25
-rw-r--r--include/linux/device.h2
2 files changed, 19 insertions, 8 deletions
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 5847364f25d9..b610816eb887 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -196,8 +196,10 @@ struct device_link *device_link_add(struct device *consumer,
196 } 196 }
197 197
198 list_for_each_entry(link, &supplier->links.consumers, s_node) 198 list_for_each_entry(link, &supplier->links.consumers, s_node)
199 if (link->consumer == consumer) 199 if (link->consumer == consumer) {
200 kref_get(&link->kref);
200 goto out; 201 goto out;
202 }
201 203
202 link = kzalloc(sizeof(*link), GFP_KERNEL); 204 link = kzalloc(sizeof(*link), GFP_KERNEL);
203 if (!link) 205 if (!link)
@@ -222,6 +224,7 @@ struct device_link *device_link_add(struct device *consumer,
222 link->consumer = consumer; 224 link->consumer = consumer;
223 INIT_LIST_HEAD(&link->c_node); 225 INIT_LIST_HEAD(&link->c_node);
224 link->flags = flags; 226 link->flags = flags;
227 kref_init(&link->kref);
225 228
226 /* Determine the initial link state. */ 229 /* Determine the initial link state. */
227 if (flags & DL_FLAG_STATELESS) { 230 if (flags & DL_FLAG_STATELESS) {
@@ -292,8 +295,10 @@ static void __device_link_free_srcu(struct rcu_head *rhead)
292 device_link_free(container_of(rhead, struct device_link, rcu_head)); 295 device_link_free(container_of(rhead, struct device_link, rcu_head));
293} 296}
294 297
295static void __device_link_del(struct device_link *link) 298static void __device_link_del(struct kref *kref)
296{ 299{
300 struct device_link *link = container_of(kref, struct device_link, kref);
301
297 dev_info(link->consumer, "Dropping the link to %s\n", 302 dev_info(link->consumer, "Dropping the link to %s\n",
298 dev_name(link->supplier)); 303 dev_name(link->supplier));
299 304
@@ -305,8 +310,10 @@ static void __device_link_del(struct device_link *link)
305 call_srcu(&device_links_srcu, &link->rcu_head, __device_link_free_srcu); 310 call_srcu(&device_links_srcu, &link->rcu_head, __device_link_free_srcu);
306} 311}
307#else /* !CONFIG_SRCU */ 312#else /* !CONFIG_SRCU */
308static void __device_link_del(struct device_link *link) 313static void __device_link_del(struct kref *kref)
309{ 314{
315 struct device_link *link = container_of(kref, struct device_link, kref);
316
310 dev_info(link->consumer, "Dropping the link to %s\n", 317 dev_info(link->consumer, "Dropping the link to %s\n",
311 dev_name(link->supplier)); 318 dev_name(link->supplier));
312 319
@@ -324,13 +331,15 @@ static void __device_link_del(struct device_link *link)
324 * @link: Device link to delete. 331 * @link: Device link to delete.
325 * 332 *
326 * The caller must ensure proper synchronization of this function with runtime 333 * The caller must ensure proper synchronization of this function with runtime
327 * PM. 334 * PM. If the link was added multiple times, it needs to be deleted as often.
335 * Care is required for hotplugged devices: Their links are purged on removal
336 * and calling device_link_del() is then no longer allowed.
328 */ 337 */
329void device_link_del(struct device_link *link) 338void device_link_del(struct device_link *link)
330{ 339{
331 device_links_write_lock(); 340 device_links_write_lock();
332 device_pm_lock(); 341 device_pm_lock();
333 __device_link_del(link); 342 kref_put(&link->kref, __device_link_del);
334 device_pm_unlock(); 343 device_pm_unlock();
335 device_links_write_unlock(); 344 device_links_write_unlock();
336} 345}
@@ -444,7 +453,7 @@ static void __device_links_no_driver(struct device *dev)
444 continue; 453 continue;
445 454
446 if (link->flags & DL_FLAG_AUTOREMOVE) 455 if (link->flags & DL_FLAG_AUTOREMOVE)
447 __device_link_del(link); 456 kref_put(&link->kref, __device_link_del);
448 else if (link->status != DL_STATE_SUPPLIER_UNBIND) 457 else if (link->status != DL_STATE_SUPPLIER_UNBIND)
449 WRITE_ONCE(link->status, DL_STATE_AVAILABLE); 458 WRITE_ONCE(link->status, DL_STATE_AVAILABLE);
450 } 459 }
@@ -597,13 +606,13 @@ static void device_links_purge(struct device *dev)
597 606
598 list_for_each_entry_safe_reverse(link, ln, &dev->links.suppliers, c_node) { 607 list_for_each_entry_safe_reverse(link, ln, &dev->links.suppliers, c_node) {
599 WARN_ON(link->status == DL_STATE_ACTIVE); 608 WARN_ON(link->status == DL_STATE_ACTIVE);
600 __device_link_del(link); 609 __device_link_del(&link->kref);
601 } 610 }
602 611
603 list_for_each_entry_safe_reverse(link, ln, &dev->links.consumers, s_node) { 612 list_for_each_entry_safe_reverse(link, ln, &dev->links.consumers, s_node) {
604 WARN_ON(link->status != DL_STATE_DORMANT && 613 WARN_ON(link->status != DL_STATE_DORMANT &&
605 link->status != DL_STATE_NONE); 614 link->status != DL_STATE_NONE);
606 __device_link_del(link); 615 __device_link_del(&link->kref);
607 } 616 }
608 617
609 device_links_write_unlock(); 618 device_links_write_unlock();
diff --git a/include/linux/device.h b/include/linux/device.h
index b093405ed525..abf952c82c6d 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -769,6 +769,7 @@ enum device_link_state {
769 * @status: The state of the link (with respect to the presence of drivers). 769 * @status: The state of the link (with respect to the presence of drivers).
770 * @flags: Link flags. 770 * @flags: Link flags.
771 * @rpm_active: Whether or not the consumer device is runtime-PM-active. 771 * @rpm_active: Whether or not the consumer device is runtime-PM-active.
772 * @kref: Count repeated addition of the same link.
772 * @rcu_head: An RCU head to use for deferred execution of SRCU callbacks. 773 * @rcu_head: An RCU head to use for deferred execution of SRCU callbacks.
773 */ 774 */
774struct device_link { 775struct device_link {
@@ -779,6 +780,7 @@ struct device_link {
779 enum device_link_state status; 780 enum device_link_state status;
780 u32 flags; 781 u32 flags;
781 bool rpm_active; 782 bool rpm_active;
783 struct kref kref;
782#ifdef CONFIG_SRCU 784#ifdef CONFIG_SRCU
783 struct rcu_head rcu_head; 785 struct rcu_head rcu_head;
784#endif 786#endif