From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
alan@lxorguk.ukuu.org.uk,
Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>,
Yijing Wang <wangyijing@huawei.com>,
Bjorn Helgaas <bhelgaas@google.com>
Subject: [ 10/22] PCI: pciehp: Use per-slot workqueues to avoid deadlock
Date: Thu, 24 Jan 2013 13:15:31 -0800 [thread overview]
Message-ID: <20130124211508.145055581@linuxfoundation.org> (raw)
In-Reply-To: <20130124211504.701368763@linuxfoundation.org>
3.4-stable review patch. If anyone has any objections, please let me know.
------------------
From: Yijing Wang <wangyijing@huawei.com>
commit c2be6f93b383c873a4f9d521afa49b1b67d06085 upstream.
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>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/pci/hotplug/pciehp.h | 2 +-
drivers/pci/hotplug/pciehp_core.c | 11 ++---------
drivers/pci/hotplug/pciehp_ctrl.c | 8 ++++----
drivers/pci/hotplug/pciehp_hpc.c | 11 ++++++++++-
4 files changed, 17 insertions(+), 15 deletions(-)
--- a/drivers/pci/hotplug/pciehp.h
+++ b/drivers/pci/hotplug/pciehp.h
@@ -44,7 +44,6 @@ extern bool pciehp_poll_mode;
extern int pciehp_poll_time;
extern bool pciehp_debug;
extern bool pciehp_force;
-extern struct workqueue_struct *pciehp_wq;
#define dbg(format, arg...) \
do { \
@@ -78,6 +77,7 @@ struct slot {
struct hotplug_slot *hotplug_slot;
struct delayed_work work; /* work for button event */
struct mutex lock;
+ struct workqueue_struct *wq;
};
struct event_info {
--- a/drivers/pci/hotplug/pciehp_core.c
+++ b/drivers/pci/hotplug/pciehp_core.c
@@ -42,7 +42,6 @@ bool pciehp_debug;
bool pciehp_poll_mode;
int pciehp_poll_time;
bool pciehp_force;
-struct workqueue_struct *pciehp_wq;
#define DRIVER_VERSION "0.4"
#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)
{
int retval = 0;
- pciehp_wq = alloc_workqueue("pciehp", 0, 0);
- if (!pciehp_wq)
- return -ENOMEM;
-
pciehp_firmware_init();
retval = pcie_port_service_register(&hpdriver_portdrv);
dbg("pcie_port_service_register = %d\n", retval);
info(DRIVER_DESC " version: " DRIVER_VERSION "\n");
- if (retval) {
- destroy_workqueue(pciehp_wq);
+ if (retval)
dbg("Failure to register service\n");
- }
+
return retval;
}
@@ -359,7 +353,6 @@ static void __exit pcied_cleanup(void)
{
dbg("unload_pciehpd()\n");
pcie_port_service_unregister(&hpdriver_portdrv);
- destroy_workqueue(pciehp_wq);
info(DRIVER_DESC " version: " DRIVER_VERSION " unloaded\n");
}
--- a/drivers/pci/hotplug/pciehp_ctrl.c
+++ b/drivers/pci/hotplug/pciehp_ctrl.c
@@ -49,7 +49,7 @@ static int queue_interrupt_event(struct
info->p_slot = p_slot;
INIT_WORK(&info->work, interrupt_event_handler);
- queue_work(pciehp_wq, &info->work);
+ queue_work(p_slot->wq, &info->work);
return 0;
}
@@ -344,7 +344,7 @@ void pciehp_queue_pushbutton_work(struct
kfree(info);
goto out;
}
- queue_work(pciehp_wq, &info->work);
+ queue_work(p_slot->wq, &info->work);
out:
mutex_unlock(&p_slot->lock);
}
@@ -377,7 +377,7 @@ static void handle_button_press_event(st
if (ATTN_LED(ctrl))
pciehp_set_attention_status(p_slot, 0);
- queue_delayed_work(pciehp_wq, &p_slot->work, 5*HZ);
+ queue_delayed_work(p_slot->wq, &p_slot->work, 5*HZ);
break;
case BLINKINGOFF_STATE:
case BLINKINGON_STATE:
@@ -439,7 +439,7 @@ static void handle_surprise_event(struct
else
p_slot->state = POWERON_STATE;
- queue_work(pciehp_wq, &info->work);
+ queue_work(p_slot->wq, &info->work);
}
static void interrupt_event_handler(struct work_struct *work)
--- a/drivers/pci/hotplug/pciehp_hpc.c
+++ b/drivers/pci/hotplug/pciehp_hpc.c
@@ -874,23 +874,32 @@ static void pcie_shutdown_notification(s
static int pcie_init_slot(struct controller *ctrl)
{
struct slot *slot;
+ char name[32];
slot = kzalloc(sizeof(*slot), GFP_KERNEL);
if (!slot)
return -ENOMEM;
+ snprintf(name, sizeof(name), "pciehp-%u", PSN(ctrl));
+ slot->wq = alloc_workqueue(name, 0, 0);
+ if (!slot->wq)
+ goto abort;
+
slot->ctrl = ctrl;
mutex_init(&slot->lock);
INIT_DELAYED_WORK(&slot->work, pciehp_queue_pushbutton_work);
ctrl->slot = slot;
return 0;
+abort:
+ kfree(slot);
+ return -ENOMEM;
}
static void pcie_cleanup_slot(struct controller *ctrl)
{
struct slot *slot = ctrl->slot;
cancel_delayed_work(&slot->work);
- flush_workqueue(pciehp_wq);
+ destroy_workqueue(slot->wq);
kfree(slot);
}
next prev parent reply other threads:[~2013-01-24 21:26 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-01-24 21:15 [ 00/22] 3.4.28-stable review Greg Kroah-Hartman
2013-01-24 21:15 ` [ 01/22] drm/i915: Invalidate the relocation presumed_offsets along the slow path Greg Kroah-Hartman
2013-01-24 21:15 ` [ 02/22] libata: ahci: Add support for Enmotus Bobcat device Greg Kroah-Hartman
2013-01-24 21:15 ` [ 03/22] ftrace: Be first to run code modification on modules Greg Kroah-Hartman
2013-01-24 21:15 ` [ 04/22] evm: checking if removexattr is not a NULL Greg Kroah-Hartman
2013-01-24 21:15 ` [ 05/22] ptrace: introduce signal_wake_up_state() and ptrace_signal_wake_up() Greg Kroah-Hartman
2013-01-24 21:15 ` [ 06/22] ptrace: ensure arch_ptrace/ptrace_request can never race with SIGKILL Greg Kroah-Hartman
2013-01-24 21:15 ` [ 07/22] wake_up_process() should be never used to wakeup a TASK_STOPPED/TRACED task Greg Kroah-Hartman
2013-01-24 21:15 ` [ 08/22] PCI/AER: pci_get_domain_bus_and_slot() call missing required pci_dev_put() Greg Kroah-Hartman
2013-01-24 21:15 ` [ 09/22] PCI: Allow pcie_aspm=force even when FADT indicates it is unsupported Greg Kroah-Hartman
2013-01-24 21:15 ` Greg Kroah-Hartman [this message]
2013-01-24 21:15 ` [ 11/22] PCI: shpchp: Handle push button event asynchronously Greg Kroah-Hartman
2013-01-24 21:15 ` [ 12/22] USB: UHCI: fix IRQ race during initialization Greg Kroah-Hartman
2013-01-24 21:15 ` [ 13/22] usb: dwc3: gadget: fix ep->maxburst for ep0 Greg Kroah-Hartman
2013-01-24 21:15 ` [ 14/22] SCSI: sd: Reshuffle init_sd to avoid crash Greg Kroah-Hartman
2013-01-24 21:15 ` [ 15/22] drivers/firmware/dmi_scan.c: check dmi version when get system uuid Greg Kroah-Hartman
2013-01-24 21:15 ` [ 16/22] drivers/firmware/dmi_scan.c: fetch dmi version from SMBIOS if it exists Greg Kroah-Hartman
2013-01-24 21:15 ` Greg Kroah-Hartman
2013-01-24 21:15 ` [ 17/22] ahci: Add identifiers for ASM106x devices Greg Kroah-Hartman
2013-01-24 21:15 ` [ 18/22] ALSA: usb-audio: Fix regression by disconnection-race-fix patch Greg Kroah-Hartman
2013-01-24 21:15 ` [ 19/22] drm/i915: Implement WaDisableHiZPlanesWhenMSAAEnabled Greg Kroah-Hartman
2013-01-24 21:15 ` [ 20/22] ACPI / cpuidle: Fix NULL pointer issues when cpuidle is disabled Greg Kroah-Hartman
2013-01-24 21:15 ` [ 21/22] ACPI / processor: Get power info before updating the C-states Greg Kroah-Hartman
2013-01-24 21:15 ` [ 22/22] ioat: Fix DMA memory sync direction correct flag Greg Kroah-Hartman
2013-01-25 18:05 ` [ 00/22] 3.4.28-stable review Shuah Khan
2013-01-27 2:09 ` Satoru Takeuchi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130124211508.145055581@linuxfoundation.org \
--to=gregkh@linuxfoundation.org \
--cc=alan@lxorguk.ukuu.org.uk \
--cc=bhelgaas@google.com \
--cc=kaneshige.kenji@jp.fujitsu.com \
--cc=linux-kernel@vger.kernel.org \
--cc=stable@vger.kernel.org \
--cc=wangyijing@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.