diff options
author | Yijing Wang <wangyijing@huawei.com> | 2013-01-10 21:15:54 -0500 |
---|---|---|
committer | Bjorn Helgaas <bhelgaas@google.com> | 2013-01-12 15:56:33 -0500 |
commit | c2be6f93b383c873a4f9d521afa49b1b67d06085 (patch) | |
tree | dacb5ae1d88abf8f5e0daec176abdd2710d5555e /drivers/pci/hotplug | |
parent | d1c3ed669a2d452cacfb48c2d171a1f364dae2ed (diff) |
PCI: pciehp: Use per-slot workqueues to avoid deadlock
When we have a hotplug-capable PCIe port with a second hotplug-capable
PCIe port below it, removing the device below the upstream port causes
a deadlock.
The deadlock happens because we use the pciehp_wq workqueue to run
pciehp_power_thread(), which uses pciehp_disable_slot() to remove devices
below the upstream port. When we remove the downstream PCIe port, we call
pciehp_remove(), the pciehp driver's .remove() method. That calls
flush_workqueue(pciehp_wq), which deadlocks because the
pciehp_power_thread() work item is still running.
This patch avoids the deadlock by creating a workqueue for every PCIe port
and removing the single shared workqueue.
Here's the call path that leads to the deadlock:
pciehp_queue_pushbutton_work
queue_work(pciehp_wq) # queue pciehp_power_thread
...
pciehp_power_thread
pciehp_disable_slot
remove_board
pciehp_unconfigure_device
pci_stop_and_remove_bus_device
...
pciehp_remove # pciehp driver .remove method
pciehp_release_ctrl
pcie_cleanup_slot
flush_workqueue(pciehp_wq)
This is fairly urgent because it can be caused by simply unplugging a
Thunderbolt adapter, as reported by Daniel below.
[bhelgaas: changelog]
Reference: http://lkml.kernel.org/r/CAMVG2ssiRgcTD1bej2tkUUfsWmpL5eNtPcNif9va2-Gzb2u8nQ@mail.gmail.com
Reported-and-tested-by: Daniel J Blueman <daniel@quora.org>
Reviewed-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
CC: stable@vger.kernel.org
Diffstat (limited to 'drivers/pci/hotplug')
-rw-r--r-- | drivers/pci/hotplug/pciehp.h | 2 | ||||
-rw-r--r-- | drivers/pci/hotplug/pciehp_core.c | 11 | ||||
-rw-r--r-- | drivers/pci/hotplug/pciehp_ctrl.c | 8 | ||||
-rw-r--r-- | drivers/pci/hotplug/pciehp_hpc.c | 11 |
4 files changed, 17 insertions, 15 deletions
diff --git a/drivers/pci/hotplug/pciehp.h b/drivers/pci/hotplug/pciehp.h index 26ffd3e3fb74..2c113de94323 100644 --- a/drivers/pci/hotplug/pciehp.h +++ b/drivers/pci/hotplug/pciehp.h | |||
@@ -44,7 +44,6 @@ extern bool pciehp_poll_mode; | |||
44 | extern int pciehp_poll_time; | 44 | extern int pciehp_poll_time; |
45 | extern bool pciehp_debug; | 45 | extern bool pciehp_debug; |
46 | extern bool pciehp_force; | 46 | extern bool pciehp_force; |
47 | extern struct workqueue_struct *pciehp_wq; | ||
48 | 47 | ||
49 | #define dbg(format, arg...) \ | 48 | #define dbg(format, arg...) \ |
50 | do { \ | 49 | do { \ |
@@ -78,6 +77,7 @@ struct slot { | |||
78 | struct hotplug_slot *hotplug_slot; | 77 | struct hotplug_slot *hotplug_slot; |
79 | struct delayed_work work; /* work for button event */ | 78 | struct delayed_work work; /* work for button event */ |
80 | struct mutex lock; | 79 | struct mutex lock; |
80 | struct workqueue_struct *wq; | ||
81 | }; | 81 | }; |
82 | 82 | ||
83 | struct event_info { | 83 | struct event_info { |
diff --git a/drivers/pci/hotplug/pciehp_core.c b/drivers/pci/hotplug/pciehp_core.c index 916bf4f53aba..939bd1d4b5b1 100644 --- a/drivers/pci/hotplug/pciehp_core.c +++ b/drivers/pci/hotplug/pciehp_core.c | |||
@@ -42,7 +42,6 @@ bool pciehp_debug; | |||
42 | bool pciehp_poll_mode; | 42 | bool pciehp_poll_mode; |
43 | int pciehp_poll_time; | 43 | int pciehp_poll_time; |
44 | bool pciehp_force; | 44 | bool pciehp_force; |
45 | struct workqueue_struct *pciehp_wq; | ||
46 | 45 | ||
47 | #define DRIVER_VERSION "0.4" | 46 | #define DRIVER_VERSION "0.4" |
48 | #define DRIVER_AUTHOR "Dan Zink <dan.zink@compaq.com>, Greg Kroah-Hartman <greg@kroah.com>, Dely Sy <dely.l.sy@intel.com>" | 47 | #define DRIVER_AUTHOR "Dan Zink <dan.zink@compaq.com>, Greg Kroah-Hartman <greg@kroah.com>, Dely Sy <dely.l.sy@intel.com>" |
@@ -340,18 +339,13 @@ static int __init pcied_init(void) | |||
340 | { | 339 | { |
341 | int retval = 0; | 340 | int retval = 0; |
342 | 341 | ||
343 | pciehp_wq = alloc_workqueue("pciehp", 0, 0); | ||
344 | if (!pciehp_wq) | ||
345 | return -ENOMEM; | ||
346 | |||
347 | pciehp_firmware_init(); | 342 | pciehp_firmware_init(); |
348 | retval = pcie_port_service_register(&hpdriver_portdrv); | 343 | retval = pcie_port_service_register(&hpdriver_portdrv); |
349 | dbg("pcie_port_service_register = %d\n", retval); | 344 | dbg("pcie_port_service_register = %d\n", retval); |
350 | info(DRIVER_DESC " version: " DRIVER_VERSION "\n"); | 345 | info(DRIVER_DESC " version: " DRIVER_VERSION "\n"); |
351 | if (retval) { | 346 | if (retval) |
352 | destroy_workqueue(pciehp_wq); | ||
353 | dbg("Failure to register service\n"); | 347 | dbg("Failure to register service\n"); |
354 | } | 348 | |
355 | return retval; | 349 | return retval; |
356 | } | 350 | } |
357 | 351 | ||
@@ -359,7 +353,6 @@ static void __exit pcied_cleanup(void) | |||
359 | { | 353 | { |
360 | dbg("unload_pciehpd()\n"); | 354 | dbg("unload_pciehpd()\n"); |
361 | pcie_port_service_unregister(&hpdriver_portdrv); | 355 | pcie_port_service_unregister(&hpdriver_portdrv); |
362 | destroy_workqueue(pciehp_wq); | ||
363 | info(DRIVER_DESC " version: " DRIVER_VERSION " unloaded\n"); | 356 | info(DRIVER_DESC " version: " DRIVER_VERSION " unloaded\n"); |
364 | } | 357 | } |
365 | 358 | ||
diff --git a/drivers/pci/hotplug/pciehp_ctrl.c b/drivers/pci/hotplug/pciehp_ctrl.c index 27f44295a657..38f018679175 100644 --- a/drivers/pci/hotplug/pciehp_ctrl.c +++ b/drivers/pci/hotplug/pciehp_ctrl.c | |||
@@ -49,7 +49,7 @@ static int queue_interrupt_event(struct slot *p_slot, u32 event_type) | |||
49 | info->p_slot = p_slot; | 49 | info->p_slot = p_slot; |
50 | INIT_WORK(&info->work, interrupt_event_handler); | 50 | INIT_WORK(&info->work, interrupt_event_handler); |
51 | 51 | ||
52 | queue_work(pciehp_wq, &info->work); | 52 | queue_work(p_slot->wq, &info->work); |
53 | 53 | ||
54 | return 0; | 54 | return 0; |
55 | } | 55 | } |
@@ -344,7 +344,7 @@ void pciehp_queue_pushbutton_work(struct work_struct *work) | |||
344 | kfree(info); | 344 | kfree(info); |
345 | goto out; | 345 | goto out; |
346 | } | 346 | } |
347 | queue_work(pciehp_wq, &info->work); | 347 | queue_work(p_slot->wq, &info->work); |
348 | out: | 348 | out: |
349 | mutex_unlock(&p_slot->lock); | 349 | mutex_unlock(&p_slot->lock); |
350 | } | 350 | } |
@@ -377,7 +377,7 @@ static void handle_button_press_event(struct slot *p_slot) | |||
377 | if (ATTN_LED(ctrl)) | 377 | if (ATTN_LED(ctrl)) |
378 | pciehp_set_attention_status(p_slot, 0); | 378 | pciehp_set_attention_status(p_slot, 0); |
379 | 379 | ||
380 | queue_delayed_work(pciehp_wq, &p_slot->work, 5*HZ); | 380 | queue_delayed_work(p_slot->wq, &p_slot->work, 5*HZ); |
381 | break; | 381 | break; |
382 | case BLINKINGOFF_STATE: | 382 | case BLINKINGOFF_STATE: |
383 | case BLINKINGON_STATE: | 383 | case BLINKINGON_STATE: |
@@ -439,7 +439,7 @@ static void handle_surprise_event(struct slot *p_slot) | |||
439 | else | 439 | else |
440 | p_slot->state = POWERON_STATE; | 440 | p_slot->state = POWERON_STATE; |
441 | 441 | ||
442 | queue_work(pciehp_wq, &info->work); | 442 | queue_work(p_slot->wq, &info->work); |
443 | } | 443 | } |
444 | 444 | ||
445 | static void interrupt_event_handler(struct work_struct *work) | 445 | static void interrupt_event_handler(struct work_struct *work) |
diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c index 13b2eaf7ba43..5127f3f41821 100644 --- a/drivers/pci/hotplug/pciehp_hpc.c +++ b/drivers/pci/hotplug/pciehp_hpc.c | |||
@@ -773,23 +773,32 @@ static void pcie_shutdown_notification(struct controller *ctrl) | |||
773 | static int pcie_init_slot(struct controller *ctrl) | 773 | static int pcie_init_slot(struct controller *ctrl) |
774 | { | 774 | { |
775 | struct slot *slot; | 775 | struct slot *slot; |
776 | char name[32]; | ||
776 | 777 | ||
777 | slot = kzalloc(sizeof(*slot), GFP_KERNEL); | 778 | slot = kzalloc(sizeof(*slot), GFP_KERNEL); |
778 | if (!slot) | 779 | if (!slot) |
779 | return -ENOMEM; | 780 | return -ENOMEM; |
780 | 781 | ||
782 | snprintf(name, sizeof(name), "pciehp-%u", PSN(ctrl)); | ||
783 | slot->wq = alloc_workqueue(name, 0, 0); | ||
784 | if (!slot->wq) | ||
785 | goto abort; | ||
786 | |||
781 | slot->ctrl = ctrl; | 787 | slot->ctrl = ctrl; |
782 | mutex_init(&slot->lock); | 788 | mutex_init(&slot->lock); |
783 | INIT_DELAYED_WORK(&slot->work, pciehp_queue_pushbutton_work); | 789 | INIT_DELAYED_WORK(&slot->work, pciehp_queue_pushbutton_work); |
784 | ctrl->slot = slot; | 790 | ctrl->slot = slot; |
785 | return 0; | 791 | return 0; |
792 | abort: | ||
793 | kfree(slot); | ||
794 | return -ENOMEM; | ||
786 | } | 795 | } |
787 | 796 | ||
788 | static void pcie_cleanup_slot(struct controller *ctrl) | 797 | static void pcie_cleanup_slot(struct controller *ctrl) |
789 | { | 798 | { |
790 | struct slot *slot = ctrl->slot; | 799 | struct slot *slot = ctrl->slot; |
791 | cancel_delayed_work(&slot->work); | 800 | cancel_delayed_work(&slot->work); |
792 | flush_workqueue(pciehp_wq); | 801 | destroy_workqueue(slot->wq); |
793 | kfree(slot); | 802 | kfree(slot); |
794 | } | 803 | } |
795 | 804 | ||