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, 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:15 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 ` [ 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   ` 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 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.