diff options
author | Lukas Wunner <lukas@wunner.de> | 2018-02-10 13:27:12 -0500 |
---|---|---|
committer | Rafael J. Wysocki <rafael.j.wysocki@intel.com> | 2018-02-27 12:10:42 -0500 |
commit | ead18c23c263374ed098a7d955b29b4a466d4573 (patch) | |
tree | 17e0951e486bd5ba4b757028c0ffef88c4d34715 | |
parent | da997b22c40473b7db60bde6ea188d35565d10c8 (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.c | 25 | ||||
-rw-r--r-- | include/linux/device.h | 2 |
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 | ||
295 | static void __device_link_del(struct device_link *link) | 298 | static 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 */ |
308 | static void __device_link_del(struct device_link *link) | 313 | static 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 | */ |
329 | void device_link_del(struct device_link *link) | 338 | void 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 | */ |
774 | struct device_link { | 775 | struct 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 |