stable.vger.kernel.org archive mirror
 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, Bjorn Helgaas <bhelgaas@google.com>
Subject: [ 24/46] PCI: shpchp: Use per-slot workqueues to avoid deadlock
Date: Thu, 24 Jan 2013 13:13:02 -0800	[thread overview]
Message-ID: <20130124211144.930208886@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: Bjorn Helgaas <bhelgaas@google.com>

commit f652e7d2916fe2fcf9e7d709aa5b7476b431e2dd upstream.

When we have an SHPC-capable bridge with a second SHPC-capable bridge
below it, pushing the upstream bridge's attention button causes a
deadlock.

The deadlock happens because we use the shpchp_wq workqueue to run
shpchp_pushbutton_thread(), which uses shpchp_disable_slot() to remove
devices below the upstream bridge.  When we remove the downstream bridge,
we call shpc_remove(), the shpchp driver's .remove() method.  That calls
flush_workqueue(shpchp_wq), which deadlocks because the
shpchp_pushbutton_thread() work item is still running.

This patch avoids the deadlock by creating a workqueue for every slot
and removing the single shared workqueue.

Here's the call path that leads to the deadlock:

  shpchp_queue_pushbutton_work
    queue_work(shpchp_wq)		# shpchp_pushbutton_thread
    ...

  shpchp_pushbutton_thread
    shpchp_disable_slot
      remove_board
        shpchp_unconfigure_device
          pci_stop_and_remove_bus_device
            ...
              shpc_remove		# shpchp driver .remove method
                hpc_release_ctlr
                  cleanup_slots
                    flush_workqueue(shpchp_wq)

This change is based on code inspection, since we don't have hardware
with this topology.

Based-on-patch-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/shpchp.h      |    2 +-
 drivers/pci/hotplug/shpchp_core.c |   25 +++++++++++++------------
 drivers/pci/hotplug/shpchp_ctrl.c |    6 +++---
 3 files changed, 17 insertions(+), 16 deletions(-)

--- a/drivers/pci/hotplug/shpchp.h
+++ b/drivers/pci/hotplug/shpchp.h
@@ -46,7 +46,6 @@
 extern bool shpchp_poll_mode;
 extern int shpchp_poll_time;
 extern bool shpchp_debug;
-extern struct workqueue_struct *shpchp_wq;
 
 #define dbg(format, arg...)						\
 do {									\
@@ -90,6 +89,7 @@ struct slot {
 	struct list_head	slot_list;
 	struct delayed_work work;	/* work for button event */
 	struct mutex lock;
+	struct workqueue_struct *wq;
 	u8 hp_slot;
 };
 
--- a/drivers/pci/hotplug/shpchp_core.c
+++ b/drivers/pci/hotplug/shpchp_core.c
@@ -39,7 +39,6 @@
 bool shpchp_debug;
 bool shpchp_poll_mode;
 int shpchp_poll_time;
-struct workqueue_struct *shpchp_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>"
@@ -128,6 +127,14 @@ static int init_slots(struct controller
 		slot->device = ctrl->slot_device_offset + i;
 		slot->hpc_ops = ctrl->hpc_ops;
 		slot->number = ctrl->first_slot + (ctrl->slot_num_inc * i);
+
+		snprintf(name, sizeof(name), "shpchp-%d", slot->number);
+		slot->wq = alloc_workqueue(name, 0, 0);
+		if (!slot->wq) {
+			retval = -ENOMEM;
+			goto error_info;
+		}
+
 		mutex_init(&slot->lock);
 		INIT_DELAYED_WORK(&slot->work, shpchp_queue_pushbutton_work);
 
@@ -147,7 +154,7 @@ static int init_slots(struct controller
 		if (retval) {
 			ctrl_err(ctrl, "pci_hp_register failed with error %d\n",
 				 retval);
-			goto error_info;
+			goto error_slotwq;
 		}
 
 		get_power_status(hotplug_slot, &info->power_status);
@@ -159,6 +166,8 @@ static int init_slots(struct controller
 	}
 
 	return 0;
+error_slotwq:
+	destroy_workqueue(slot->wq);
 error_info:
 	kfree(info);
 error_hpslot:
@@ -179,7 +188,7 @@ void cleanup_slots(struct controller *ct
 		slot = list_entry(tmp, struct slot, slot_list);
 		list_del(&slot->slot_list);
 		cancel_delayed_work(&slot->work);
-		flush_workqueue(shpchp_wq);
+		destroy_workqueue(slot->wq);
 		pci_hp_deregister(slot->hotplug_slot);
 	}
 }
@@ -362,18 +371,11 @@ static struct pci_driver shpc_driver = {
 
 static int __init shpcd_init(void)
 {
-	int retval = 0;
-
-	shpchp_wq = alloc_ordered_workqueue("shpchp", 0);
-	if (!shpchp_wq)
-		return -ENOMEM;
+	int retval;
 
 	retval = pci_register_driver(&shpc_driver);
 	dbg("%s: pci_register_driver = %d\n", __func__, retval);
 	info(DRIVER_DESC " version: " DRIVER_VERSION "\n");
-	if (retval) {
-		destroy_workqueue(shpchp_wq);
-	}
 	return retval;
 }
 
@@ -381,7 +383,6 @@ static void __exit shpcd_cleanup(void)
 {
 	dbg("unload_shpchpd()\n");
 	pci_unregister_driver(&shpc_driver);
-	destroy_workqueue(shpchp_wq);
 	info(DRIVER_DESC " version: " DRIVER_VERSION " unloaded\n");
 }
 
--- a/drivers/pci/hotplug/shpchp_ctrl.c
+++ b/drivers/pci/hotplug/shpchp_ctrl.c
@@ -51,7 +51,7 @@ static int queue_interrupt_event(struct
 	info->p_slot = p_slot;
 	INIT_WORK(&info->work, interrupt_event_handler);
 
-	queue_work(shpchp_wq, &info->work);
+	queue_work(p_slot->wq, &info->work);
 
 	return 0;
 }
@@ -453,7 +453,7 @@ void shpchp_queue_pushbutton_work(struct
 		kfree(info);
 		goto out;
 	}
-	queue_work(shpchp_wq, &info->work);
+	queue_work(p_slot->wq, &info->work);
  out:
 	mutex_unlock(&p_slot->lock);
 }
@@ -501,7 +501,7 @@ static void handle_button_press_event(st
 		p_slot->hpc_ops->green_led_blink(p_slot);
 		p_slot->hpc_ops->set_attention_status(p_slot, 0);
 
-		queue_delayed_work(shpchp_wq, &p_slot->work, 5*HZ);
+		queue_delayed_work(p_slot->wq, &p_slot->work, 5*HZ);
 		break;
 	case BLINKINGOFF_STATE:
 	case BLINKINGON_STATE:



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

Thread overview: 49+ 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 ` [ 22/46] PCI: pciehp: Use per-slot workqueues to avoid deadlock Greg Kroah-Hartman
2013-01-24 21:13 ` [ 23/46] PCI: shpchp: Handle push button event asynchronously Greg Kroah-Hartman
2013-01-24 21:13 ` Greg Kroah-Hartman [this message]
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 ` [ 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.930208886@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=bhelgaas@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).