diff options
author | Jiang Liu <liuj97@gmail.com> | 2013-04-12 01:44:28 -0400 |
---|---|---|
committer | Bjorn Helgaas <bhelgaas@google.com> | 2013-04-16 12:27:14 -0400 |
commit | 3d54a3160fb6ba877324ffffa5d301dec8038fd9 (patch) | |
tree | 03589d2bc780b2a410336fb5b47cb1af94f8b340 /drivers/pci/hotplug | |
parent | ad41dd9dd0c8ca1876f30b62c5c79625ffe83174 (diff) |
PCI: acpiphp: Protect acpiphp data structures from concurrent updates
Now acpiphp_enumerate_slots() and acpiphp_remove_slots() may be invoked
concurrently by the PCI core, so add a bridge_mutex and reference count
mechanism to protect acpiphp bridge/slot/function data structures.
To avoid deadlock, handle_hotplug_event_bridge() will requeue the
hotplug event onto the kacpi_hotplug_wq by calling alloc_acpi_hp_work().
But the workaround has introduced a minor race window because the
'bridge' passed to _handle_hotplug_event_bridge() may have already been
destroyed when _handle_hotplug_event_bridge() is actually executed by
the kacpi_hotplug_wq. So hold a reference count on the passed 'bridge'.
Fix the same issue for handle_hotplug_event_func() too.
Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Yinghai Lu <yinghai@kernel.org>
Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Cc: Toshi Kani <toshi.kani@hp.com>
Diffstat (limited to 'drivers/pci/hotplug')
-rw-r--r-- | drivers/pci/hotplug/acpiphp.h | 1 | ||||
-rw-r--r-- | drivers/pci/hotplug/acpiphp_glue.c | 95 |
2 files changed, 78 insertions, 18 deletions
diff --git a/drivers/pci/hotplug/acpiphp.h b/drivers/pci/hotplug/acpiphp.h index 4ff716b6a6b3..6a319f42b30c 100644 --- a/drivers/pci/hotplug/acpiphp.h +++ b/drivers/pci/hotplug/acpiphp.h | |||
@@ -74,6 +74,7 @@ static inline const char *slot_name(struct slot *slot) | |||
74 | struct acpiphp_bridge { | 74 | struct acpiphp_bridge { |
75 | struct list_head list; | 75 | struct list_head list; |
76 | struct list_head slots; | 76 | struct list_head slots; |
77 | struct kref ref; | ||
77 | acpi_handle handle; | 78 | acpi_handle handle; |
78 | 79 | ||
79 | /* Ejectable PCI-to-PCI bridge (PCI bridge and PCI function) */ | 80 | /* Ejectable PCI-to-PCI bridge (PCI bridge and PCI function) */ |
diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c index 453a749d9595..96fed19c6d90 100644 --- a/drivers/pci/hotplug/acpiphp_glue.c +++ b/drivers/pci/hotplug/acpiphp_glue.c | |||
@@ -54,6 +54,7 @@ | |||
54 | #include "acpiphp.h" | 54 | #include "acpiphp.h" |
55 | 55 | ||
56 | static LIST_HEAD(bridge_list); | 56 | static LIST_HEAD(bridge_list); |
57 | static DEFINE_MUTEX(bridge_mutex); | ||
57 | 58 | ||
58 | #define MY_NAME "acpiphp_glue" | 59 | #define MY_NAME "acpiphp_glue" |
59 | 60 | ||
@@ -61,6 +62,7 @@ static void handle_hotplug_event_bridge (acpi_handle, u32, void *); | |||
61 | static void acpiphp_sanitize_bus(struct pci_bus *bus); | 62 | static void acpiphp_sanitize_bus(struct pci_bus *bus); |
62 | static void acpiphp_set_hpp_values(struct pci_bus *bus); | 63 | static void acpiphp_set_hpp_values(struct pci_bus *bus); |
63 | static void handle_hotplug_event_func(acpi_handle handle, u32 type, void *context); | 64 | static void handle_hotplug_event_func(acpi_handle handle, u32 type, void *context); |
65 | static void free_bridge(struct kref *kref); | ||
64 | 66 | ||
65 | /* callback routine to check for the existence of a pci dock device */ | 67 | /* callback routine to check for the existence of a pci dock device */ |
66 | static acpi_status | 68 | static acpi_status |
@@ -76,6 +78,39 @@ is_pci_dock_device(acpi_handle handle, u32 lvl, void *context, void **rv) | |||
76 | } | 78 | } |
77 | } | 79 | } |
78 | 80 | ||
81 | static inline void get_bridge(struct acpiphp_bridge *bridge) | ||
82 | { | ||
83 | kref_get(&bridge->ref); | ||
84 | } | ||
85 | |||
86 | static inline void put_bridge(struct acpiphp_bridge *bridge) | ||
87 | { | ||
88 | kref_put(&bridge->ref, free_bridge); | ||
89 | } | ||
90 | |||
91 | static void free_bridge(struct kref *kref) | ||
92 | { | ||
93 | struct acpiphp_bridge *bridge; | ||
94 | struct acpiphp_slot *slot, *next; | ||
95 | struct acpiphp_func *func, *tmp; | ||
96 | |||
97 | bridge = container_of(kref, struct acpiphp_bridge, ref); | ||
98 | |||
99 | list_for_each_entry_safe(slot, next, &bridge->slots, node) { | ||
100 | list_for_each_entry_safe(func, tmp, &slot->funcs, sibling) { | ||
101 | kfree(func); | ||
102 | } | ||
103 | kfree(slot); | ||
104 | } | ||
105 | |||
106 | /* Release reference acquired by acpiphp_bridge_handle_to_function() */ | ||
107 | if ((bridge->flags & BRIDGE_HAS_EJ0) && bridge->func) | ||
108 | put_bridge(bridge->func->slot->bridge); | ||
109 | put_device(&bridge->pci_bus->dev); | ||
110 | pci_dev_put(bridge->pci_dev); | ||
111 | kfree(bridge); | ||
112 | } | ||
113 | |||
79 | /* | 114 | /* |
80 | * the _DCK method can do funny things... and sometimes not | 115 | * the _DCK method can do funny things... and sometimes not |
81 | * hah-hah funny. | 116 | * hah-hah funny. |
@@ -171,7 +206,7 @@ register_slot(acpi_handle handle, u32 lvl, void *context, void **rv) | |||
171 | device = (adr >> 16) & 0xffff; | 206 | device = (adr >> 16) & 0xffff; |
172 | function = adr & 0xffff; | 207 | function = adr & 0xffff; |
173 | 208 | ||
174 | pdev = pbus->self; | 209 | pdev = bridge->pci_dev; |
175 | if (pdev && device_is_managed_by_native_pciehp(pdev)) | 210 | if (pdev && device_is_managed_by_native_pciehp(pdev)) |
176 | return AE_OK; | 211 | return AE_OK; |
177 | 212 | ||
@@ -179,7 +214,6 @@ register_slot(acpi_handle handle, u32 lvl, void *context, void **rv) | |||
179 | if (!newfunc) | 214 | if (!newfunc) |
180 | return AE_NO_MEMORY; | 215 | return AE_NO_MEMORY; |
181 | 216 | ||
182 | INIT_LIST_HEAD(&newfunc->sibling); | ||
183 | newfunc->handle = handle; | 217 | newfunc->handle = handle; |
184 | newfunc->function = function; | 218 | newfunc->function = function; |
185 | 219 | ||
@@ -229,7 +263,9 @@ register_slot(acpi_handle handle, u32 lvl, void *context, void **rv) | |||
229 | INIT_LIST_HEAD(&slot->funcs); | 263 | INIT_LIST_HEAD(&slot->funcs); |
230 | mutex_init(&slot->crit_sect); | 264 | mutex_init(&slot->crit_sect); |
231 | 265 | ||
266 | mutex_lock(&bridge_mutex); | ||
232 | list_add_tail(&slot->node, &bridge->slots); | 267 | list_add_tail(&slot->node, &bridge->slots); |
268 | mutex_unlock(&bridge_mutex); | ||
233 | bridge->nr_slots++; | 269 | bridge->nr_slots++; |
234 | 270 | ||
235 | dbg("found ACPI PCI Hotplug slot %llu at PCI %04x:%02x:%02x\n", | 271 | dbg("found ACPI PCI Hotplug slot %llu at PCI %04x:%02x:%02x\n", |
@@ -247,7 +283,9 @@ register_slot(acpi_handle handle, u32 lvl, void *context, void **rv) | |||
247 | } | 283 | } |
248 | 284 | ||
249 | newfunc->slot = slot; | 285 | newfunc->slot = slot; |
286 | mutex_lock(&bridge_mutex); | ||
250 | list_add_tail(&newfunc->sibling, &slot->funcs); | 287 | list_add_tail(&newfunc->sibling, &slot->funcs); |
288 | mutex_unlock(&bridge_mutex); | ||
251 | 289 | ||
252 | if (pci_bus_read_dev_vendor_id(pbus, PCI_DEVFN(device, function), | 290 | if (pci_bus_read_dev_vendor_id(pbus, PCI_DEVFN(device, function), |
253 | &val, 60*1000)) | 291 | &val, 60*1000)) |
@@ -288,7 +326,9 @@ register_slot(acpi_handle handle, u32 lvl, void *context, void **rv) | |||
288 | 326 | ||
289 | err_exit: | 327 | err_exit: |
290 | bridge->nr_slots--; | 328 | bridge->nr_slots--; |
329 | mutex_lock(&bridge_mutex); | ||
291 | list_del(&slot->node); | 330 | list_del(&slot->node); |
331 | mutex_unlock(&bridge_mutex); | ||
292 | kfree(slot); | 332 | kfree(slot); |
293 | kfree(newfunc); | 333 | kfree(newfunc); |
294 | 334 | ||
@@ -313,13 +353,17 @@ static void init_bridge_misc(struct acpiphp_bridge *bridge) | |||
313 | acpi_status status; | 353 | acpi_status status; |
314 | 354 | ||
315 | /* must be added to the list prior to calling register_slot */ | 355 | /* must be added to the list prior to calling register_slot */ |
356 | mutex_lock(&bridge_mutex); | ||
316 | list_add(&bridge->list, &bridge_list); | 357 | list_add(&bridge->list, &bridge_list); |
358 | mutex_unlock(&bridge_mutex); | ||
317 | 359 | ||
318 | /* register all slot objects under this bridge */ | 360 | /* register all slot objects under this bridge */ |
319 | status = acpi_walk_namespace(ACPI_TYPE_DEVICE, bridge->handle, (u32)1, | 361 | status = acpi_walk_namespace(ACPI_TYPE_DEVICE, bridge->handle, (u32)1, |
320 | register_slot, NULL, bridge, NULL); | 362 | register_slot, NULL, bridge, NULL); |
321 | if (ACPI_FAILURE(status)) { | 363 | if (ACPI_FAILURE(status)) { |
364 | mutex_lock(&bridge_mutex); | ||
322 | list_del(&bridge->list); | 365 | list_del(&bridge->list); |
366 | mutex_unlock(&bridge_mutex); | ||
323 | return; | 367 | return; |
324 | } | 368 | } |
325 | 369 | ||
@@ -349,16 +393,21 @@ static struct acpiphp_func *acpiphp_bridge_handle_to_function(acpi_handle handle | |||
349 | { | 393 | { |
350 | struct acpiphp_bridge *bridge; | 394 | struct acpiphp_bridge *bridge; |
351 | struct acpiphp_slot *slot; | 395 | struct acpiphp_slot *slot; |
352 | struct acpiphp_func *func; | 396 | struct acpiphp_func *func = NULL; |
353 | 397 | ||
398 | mutex_lock(&bridge_mutex); | ||
354 | list_for_each_entry(bridge, &bridge_list, list) { | 399 | list_for_each_entry(bridge, &bridge_list, list) { |
355 | list_for_each_entry(slot, &bridge->slots, node) { | 400 | list_for_each_entry(slot, &bridge->slots, node) { |
356 | list_for_each_entry(func, &slot->funcs, sibling) { | 401 | list_for_each_entry(func, &slot->funcs, sibling) { |
357 | if (func->handle == handle) | 402 | if (func->handle == handle) { |
403 | get_bridge(func->slot->bridge); | ||
404 | mutex_unlock(&bridge_mutex); | ||
358 | return func; | 405 | return func; |
406 | } | ||
359 | } | 407 | } |
360 | } | 408 | } |
361 | } | 409 | } |
410 | mutex_unlock(&bridge_mutex); | ||
362 | 411 | ||
363 | return NULL; | 412 | return NULL; |
364 | } | 413 | } |
@@ -368,17 +417,22 @@ static struct acpiphp_bridge *acpiphp_handle_to_bridge(acpi_handle handle) | |||
368 | { | 417 | { |
369 | struct acpiphp_bridge *bridge; | 418 | struct acpiphp_bridge *bridge; |
370 | 419 | ||
420 | mutex_lock(&bridge_mutex); | ||
371 | list_for_each_entry(bridge, &bridge_list, list) | 421 | list_for_each_entry(bridge, &bridge_list, list) |
372 | if (bridge->handle == handle) | 422 | if (bridge->handle == handle) { |
423 | get_bridge(bridge); | ||
424 | mutex_unlock(&bridge_mutex); | ||
373 | return bridge; | 425 | return bridge; |
426 | } | ||
427 | mutex_unlock(&bridge_mutex); | ||
374 | 428 | ||
375 | return NULL; | 429 | return NULL; |
376 | } | 430 | } |
377 | 431 | ||
378 | static void cleanup_bridge(struct acpiphp_bridge *bridge) | 432 | static void cleanup_bridge(struct acpiphp_bridge *bridge) |
379 | { | 433 | { |
380 | struct acpiphp_slot *slot, *next; | 434 | struct acpiphp_slot *slot; |
381 | struct acpiphp_func *func, *tmp; | 435 | struct acpiphp_func *func; |
382 | acpi_status status; | 436 | acpi_status status; |
383 | acpi_handle handle = bridge->handle; | 437 | acpi_handle handle = bridge->handle; |
384 | 438 | ||
@@ -399,8 +453,8 @@ static void cleanup_bridge(struct acpiphp_bridge *bridge) | |||
399 | err("failed to install interrupt notify handler\n"); | 453 | err("failed to install interrupt notify handler\n"); |
400 | } | 454 | } |
401 | 455 | ||
402 | list_for_each_entry_safe(slot, next, &bridge->slots, node) { | 456 | list_for_each_entry(slot, &bridge->slots, node) { |
403 | list_for_each_entry_safe(func, tmp, &slot->funcs, sibling) { | 457 | list_for_each_entry(func, &slot->funcs, sibling) { |
404 | if (is_dock_device(func->handle)) { | 458 | if (is_dock_device(func->handle)) { |
405 | unregister_hotplug_dock_device(func->handle); | 459 | unregister_hotplug_dock_device(func->handle); |
406 | unregister_dock_notifier(&func->nb); | 460 | unregister_dock_notifier(&func->nb); |
@@ -412,18 +466,13 @@ static void cleanup_bridge(struct acpiphp_bridge *bridge) | |||
412 | if (ACPI_FAILURE(status)) | 466 | if (ACPI_FAILURE(status)) |
413 | err("failed to remove notify handler\n"); | 467 | err("failed to remove notify handler\n"); |
414 | } | 468 | } |
415 | list_del(&func->sibling); | ||
416 | kfree(func); | ||
417 | } | 469 | } |
418 | acpiphp_unregister_hotplug_slot(slot); | 470 | acpiphp_unregister_hotplug_slot(slot); |
419 | list_del(&slot->funcs); | ||
420 | kfree(slot); | ||
421 | } | 471 | } |
422 | 472 | ||
423 | put_device(&bridge->pci_bus->dev); | 473 | mutex_lock(&bridge_mutex); |
424 | pci_dev_put(bridge->pci_dev); | ||
425 | list_del(&bridge->list); | 474 | list_del(&bridge->list); |
426 | kfree(bridge); | 475 | mutex_unlock(&bridge_mutex); |
427 | } | 476 | } |
428 | 477 | ||
429 | static int power_on_slot(struct acpiphp_slot *slot) | 478 | static int power_on_slot(struct acpiphp_slot *slot) |
@@ -620,7 +669,6 @@ static int __ref enable_device(struct acpiphp_slot *slot) | |||
620 | struct pci_dev *dev; | 669 | struct pci_dev *dev; |
621 | struct pci_bus *bus = slot->bridge->pci_bus; | 670 | struct pci_bus *bus = slot->bridge->pci_bus; |
622 | struct acpiphp_func *func; | 671 | struct acpiphp_func *func; |
623 | int retval = 0; | ||
624 | int num, max, pass; | 672 | int num, max, pass; |
625 | 673 | ||
626 | if (slot->flags & SLOT_ENABLED) | 674 | if (slot->flags & SLOT_ENABLED) |
@@ -680,7 +728,7 @@ static int __ref enable_device(struct acpiphp_slot *slot) | |||
680 | 728 | ||
681 | 729 | ||
682 | err_exit: | 730 | err_exit: |
683 | return retval; | 731 | return 0; |
684 | } | 732 | } |
685 | 733 | ||
686 | /* return first device in slot, acquiring a reference on it */ | 734 | /* return first device in slot, acquiring a reference on it */ |
@@ -897,6 +945,7 @@ check_sub_bridges(acpi_handle handle, u32 lvl, void *context, void **rv) | |||
897 | dbg("%s: re-enumerating slots under %s\n", | 945 | dbg("%s: re-enumerating slots under %s\n", |
898 | __func__, objname); | 946 | __func__, objname); |
899 | acpiphp_check_bridge(bridge); | 947 | acpiphp_check_bridge(bridge); |
948 | put_bridge(bridge); | ||
900 | } | 949 | } |
901 | return AE_OK ; | 950 | return AE_OK ; |
902 | } | 951 | } |
@@ -974,6 +1023,7 @@ static void _handle_hotplug_event_bridge(struct work_struct *work) | |||
974 | 1023 | ||
975 | acpi_scan_lock_release(); | 1024 | acpi_scan_lock_release(); |
976 | kfree(hp_work); /* allocated in handle_hotplug_event_bridge */ | 1025 | kfree(hp_work); /* allocated in handle_hotplug_event_bridge */ |
1026 | put_bridge(bridge); | ||
977 | } | 1027 | } |
978 | 1028 | ||
979 | /** | 1029 | /** |
@@ -987,6 +1037,8 @@ static void _handle_hotplug_event_bridge(struct work_struct *work) | |||
987 | static void handle_hotplug_event_bridge(acpi_handle handle, u32 type, | 1037 | static void handle_hotplug_event_bridge(acpi_handle handle, u32 type, |
988 | void *context) | 1038 | void *context) |
989 | { | 1039 | { |
1040 | struct acpiphp_bridge *bridge = context; | ||
1041 | |||
990 | /* | 1042 | /* |
991 | * Currently the code adds all hotplug events to the kacpid_wq | 1043 | * Currently the code adds all hotplug events to the kacpid_wq |
992 | * queue when it should add hotplug events to the kacpi_hotplug_wq. | 1044 | * queue when it should add hotplug events to the kacpi_hotplug_wq. |
@@ -995,6 +1047,7 @@ static void handle_hotplug_event_bridge(acpi_handle handle, u32 type, | |||
995 | * For now just re-add this work to the kacpi_hotplug_wq so we | 1047 | * For now just re-add this work to the kacpi_hotplug_wq so we |
996 | * don't deadlock on hotplug actions. | 1048 | * don't deadlock on hotplug actions. |
997 | */ | 1049 | */ |
1050 | get_bridge(bridge); | ||
998 | alloc_acpi_hp_work(handle, type, context, _handle_hotplug_event_bridge); | 1051 | alloc_acpi_hp_work(handle, type, context, _handle_hotplug_event_bridge); |
999 | } | 1052 | } |
1000 | 1053 | ||
@@ -1049,6 +1102,7 @@ static void _handle_hotplug_event_func(struct work_struct *work) | |||
1049 | 1102 | ||
1050 | acpi_scan_lock_release(); | 1103 | acpi_scan_lock_release(); |
1051 | kfree(hp_work); /* allocated in handle_hotplug_event_func */ | 1104 | kfree(hp_work); /* allocated in handle_hotplug_event_func */ |
1105 | put_bridge(func->slot->bridge); | ||
1052 | } | 1106 | } |
1053 | 1107 | ||
1054 | /** | 1108 | /** |
@@ -1062,6 +1116,8 @@ static void _handle_hotplug_event_func(struct work_struct *work) | |||
1062 | static void handle_hotplug_event_func(acpi_handle handle, u32 type, | 1116 | static void handle_hotplug_event_func(acpi_handle handle, u32 type, |
1063 | void *context) | 1117 | void *context) |
1064 | { | 1118 | { |
1119 | struct acpiphp_func *func = context; | ||
1120 | |||
1065 | /* | 1121 | /* |
1066 | * Currently the code adds all hotplug events to the kacpid_wq | 1122 | * Currently the code adds all hotplug events to the kacpid_wq |
1067 | * queue when it should add hotplug events to the kacpi_hotplug_wq. | 1123 | * queue when it should add hotplug events to the kacpi_hotplug_wq. |
@@ -1070,6 +1126,7 @@ static void handle_hotplug_event_func(acpi_handle handle, u32 type, | |||
1070 | * For now just re-add this work to the kacpi_hotplug_wq so we | 1126 | * For now just re-add this work to the kacpi_hotplug_wq so we |
1071 | * don't deadlock on hotplug actions. | 1127 | * don't deadlock on hotplug actions. |
1072 | */ | 1128 | */ |
1129 | get_bridge(func->slot->bridge); | ||
1073 | alloc_acpi_hp_work(handle, type, context, _handle_hotplug_event_func); | 1130 | alloc_acpi_hp_work(handle, type, context, _handle_hotplug_event_func); |
1074 | } | 1131 | } |
1075 | 1132 | ||
@@ -1095,6 +1152,7 @@ void acpiphp_enumerate_slots(struct pci_bus *bus, acpi_handle handle) | |||
1095 | } | 1152 | } |
1096 | 1153 | ||
1097 | INIT_LIST_HEAD(&bridge->slots); | 1154 | INIT_LIST_HEAD(&bridge->slots); |
1155 | kref_init(&bridge->ref); | ||
1098 | bridge->handle = handle; | 1156 | bridge->handle = handle; |
1099 | bridge->pci_dev = pci_dev_get(bus->self); | 1157 | bridge->pci_dev = pci_dev_get(bus->self); |
1100 | bridge->pci_bus = bus; | 1158 | bridge->pci_bus = bus; |
@@ -1128,6 +1186,7 @@ void acpiphp_remove_slots(struct pci_bus *bus) | |||
1128 | list_for_each_entry_safe(bridge, tmp, &bridge_list, list) | 1186 | list_for_each_entry_safe(bridge, tmp, &bridge_list, list) |
1129 | if (bridge->pci_bus == bus) { | 1187 | if (bridge->pci_bus == bus) { |
1130 | cleanup_bridge(bridge); | 1188 | cleanup_bridge(bridge); |
1189 | put_bridge(bridge); | ||
1131 | break; | 1190 | break; |
1132 | } | 1191 | } |
1133 | } | 1192 | } |