All of lore.kernel.org
 help / color / mirror / Atom feed
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: [ 22/46] PCI: pciehp: Use per-slot workqueues to avoid deadlock
Date: Thu, 24 Jan 2013 13:13:00 -0800	[thread overview]
Message-ID: <20130124211144.300426989@linuxfoundation.org> (raw)
In-Reply-To: <20130124211135.862755794@linuxfoundation.org>

3.7-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
@@ -773,23 +773,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);
 }
 



  parent reply	other threads:[~2013-01-24 21:16 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-24 21:12 [ 00/46] 3.7.5-stable review Greg Kroah-Hartman
2013-01-24 21:12 ` [ 01/46] make sure that /linuxrc has std{in,out,err} Greg Kroah-Hartman
2013-01-24 21:12 ` [ 02/46] Ensure that kernel_init_freeable() is not inlined into non __init code Greg Kroah-Hartman
2013-01-24 21:12 ` [ 03/46] drm/i915: Invalidate the relocation presumed_offsets along the slow path Greg Kroah-Hartman
2013-01-24 21:12 ` [ 04/46] libata: ahci: Fix lack of command retry after a success error handler Greg Kroah-Hartman
2013-01-24 21:12 ` [ 05/46] libata: ahci: Add support for Enmotus Bobcat device Greg Kroah-Hartman
2013-01-24 21:12 ` [ 06/46] libata: replace sata_settings with devslp_timing Greg Kroah-Hartman
2013-01-24 21:12 ` [ 07/46] ftrace: Be first to run code modification on modules Greg Kroah-Hartman
2013-01-24 21:12 ` [ 08/46] evm: checking if removexattr is not a NULL Greg Kroah-Hartman
2013-01-24 21:12 ` [ 09/46] virtio-blk: Dont free ida when disk is in use Greg Kroah-Hartman
2013-01-24 21:12 ` [ 10/46] async: fix __lowest_in_progress() Greg Kroah-Hartman
2013-01-24 21:12 ` [ 11/46] vfio-pci: Fix buffer overfill Greg Kroah-Hartman
2013-01-24 21:12 ` [ 12/46] perf x86: revert 20b279 - require exclude_guest to use PEBS - kernel side Greg Kroah-Hartman
2013-01-24 21:12 ` [ 13/46] ptrace: introduce signal_wake_up_state() and ptrace_signal_wake_up() Greg Kroah-Hartman
2013-01-24 21:12 ` [ 14/46] ptrace: ensure arch_ptrace/ptrace_request can never race with SIGKILL Greg Kroah-Hartman
2013-01-24 21:12 ` [ 15/46] wake_up_process() should be never used to wakeup a TASK_STOPPED/TRACED task Greg Kroah-Hartman
2013-01-24 21:12 ` [ 16/46] ALSA: hda - Fix mute led for another HP machine Greg Kroah-Hartman
2013-01-24 21:12 ` [ 17/46] ALSA: hda - Add Conexant CX20755/20756/20757 codec IDs Greg Kroah-Hartman
2013-01-24 21:12 ` [ 18/46] arm64: makefile: fix uname munging when setting ARCH on native machine Greg Kroah-Hartman
2013-01-24 21:12 ` [ 19/46] arm64: elf: fix core dumping to match what glibc expects Greg Kroah-Hartman
2013-01-24 21:12 ` [ 20/46] PCI/AER: pci_get_domain_bus_and_slot() call missing required pci_dev_put() Greg Kroah-Hartman
2013-01-24 21:12 ` [ 21/46] PCI: Allow pcie_aspm=force even when FADT indicates it is unsupported Greg Kroah-Hartman
2013-01-24 21:13 ` Greg Kroah-Hartman [this message]
2013-01-24 21:13 ` [ 23/46] PCI: shpchp: Handle push button event asynchronously Greg Kroah-Hartman
2013-01-24 21:13 ` [ 24/46] PCI: shpchp: Use per-slot workqueues to avoid deadlock Greg Kroah-Hartman
2013-01-24 21:13 ` [ 25/46] Revert "drivers/misc/ti-st: remove gpio handling" Greg Kroah-Hartman
2013-01-24 21:13 ` [ 26/46] media: gspca_kinect: add Kinect for Windows USB id Greg Kroah-Hartman
2013-01-24 21:13 ` [ 27/46] USB: UHCI: fix IRQ race during initialization Greg Kroah-Hartman
2013-01-24 21:13 ` [ 28/46] usb: dwc3: gadget: fix ep->maxburst for ep0 Greg Kroah-Hartman
2013-01-24 21:13 ` [ 29/46] usb: gadget: FunctionFS: Fix missing braces in parse_opts Greg Kroah-Hartman
2013-01-24 21:13 ` [ 30/46] usb: musb: cppi_dma: drop __init annotation Greg Kroah-Hartman
2013-01-24 21:13 ` [ 31/46] SCSI: sd: Reshuffle init_sd to avoid crash Greg Kroah-Hartman
2013-01-24 21:13 ` [ 32/46] drivers/firmware/dmi_scan.c: check dmi version when get system uuid Greg Kroah-Hartman
2013-01-24 21:13 ` [ 33/46] drivers/firmware/dmi_scan.c: fetch dmi version from SMBIOS if it exists Greg Kroah-Hartman
2013-01-24 21:13   ` Greg Kroah-Hartman
2013-01-24 21:13 ` [ 34/46] drm/i915: Implement WaDisableHiZPlanesWhenMSAAEnabled Greg Kroah-Hartman
2013-01-24 21:13 ` [ 35/46] module: add new state MODULE_STATE_UNFORMED Greg Kroah-Hartman
2013-01-24 21:13 ` [ 36/46] module: put modules in list much earlier Greg Kroah-Hartman
2013-01-24 21:13 ` [ 37/46] module: fix missing module_mutex unlock Greg Kroah-Hartman
2013-01-24 21:13 ` [ 38/46] powernow-k8: Add a kconfig dependency on acpi-cpufreq Greg Kroah-Hartman
2013-01-24 21:13 ` [ 39/46] cpufreq: Add module aliases for acpi-cpufreq Greg Kroah-Hartman
2013-01-24 21:13 ` [ 40/46] ACPI / cpuidle: Fix NULL pointer issues when cpuidle is disabled Greg Kroah-Hartman
2013-01-24 21:13 ` [ 41/46] ACPI / processor: Get power info before updating the C-states Greg Kroah-Hartman
2013-01-24 21:13 ` [ 42/46] ACPI: Check MSR valid bit before using P-state frequencies Greg Kroah-Hartman
2013-01-24 21:13 ` [ 43/46] i2c: mxs: Fix type of error code Greg Kroah-Hartman
2013-01-24 21:13 ` [ 44/46] intel_idle: Dont register CPU notifier if we are not running Greg Kroah-Hartman
2013-01-24 21:13 ` [ 45/46] ioat: Fix DMA memory sync direction correct flag Greg Kroah-Hartman
2013-01-24 21:13 ` [ 46/46] dma: tegra: implement flags parameters for cyclic transfer Greg Kroah-Hartman
2013-01-25 18:06 ` [ 00/46] 3.7.5-stable review Shuah Khan
2013-01-27  2:15 ` 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=20130124211144.300426989@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.