* [PATCH v7 0/4] PCI: pciehp, shpchp deadlock fixes
@ 2013-01-13 23:27 Bjorn Helgaas
2013-01-13 23:27 ` [PATCH v7 1/4] PCI: pciehp: Use per-slot workqueues to avoid deadlock Bjorn Helgaas
` (4 more replies)
0 siblings, 5 replies; 7+ messages in thread
From: Bjorn Helgaas @ 2013-01-13 23:27 UTC (permalink / raw)
To: Yijing Wang, linux-pci
Cc: linux-kernel, Daniel J Blueman, Kenji Kaneshige, Tejun Heo,
Yinghai Lu, Jiang Liu
This is v7 of Yijing Wang's pciehp deadlock fix. Here's the history of
previous postings:
RFC Oct 31 2012
v2 Nov 9 2012 use slot name, not device name
v3 Nov 12 2012 create workqueues in pcie_init_slot(), pciehp-%u format
v4 Nov 13 2012 use alloc_workqueue(name, 0, 0) again
v5 Jan 9 2013 fix similar problem in shpchp
v6 Jan 10 2013 add pciehp backtrace from Daniel
This v7 series is functionally identical to v6. In v7, I split the single
shpchp patch into three:
PCI: shpchp: Make shpchp_wq non-ordered
PCI: shpchp: Handle push button event asynchronously
PCI: shpchp: Use per-slot workqueues to avoid deadlock
The first fixes what appears to be an error in e24dcbef93 ("shpchp: update
workqueue usage"). I split this out so Tejun could easily review it by itself.
I split the second out because it corresponds to a pciehp bugfix, 486b10b9f4
("PCI: pciehp: Handle push button event asynchronously"), that also affects
shpchp.
The "PCI: shpchp: Use per-slot workqueues to avoid deadlock" that remains
then corresponds exactly to Yijing's original pciehp deadlock fix.
I also rewrote the changelogs.
I propose to push the entire series for inclusion in v3.8, since it
fixes an easy-to-cause deadlock with Thunderbolt adapters.
---
Bjorn Helgaas (3):
PCI: shpchp: Make shpchp_wq non-ordered
PCI: shpchp: Handle push button event asynchronously
PCI: shpchp: Use per-slot workqueues to avoid deadlock
Yijing Wang (1):
PCI: pciehp: Use per-slot workqueues to avoid deadlock
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 ++++++++++-
drivers/pci/hotplug/shpchp.h | 3 +--
drivers/pci/hotplug/shpchp_core.c | 36 ++++++++++++++----------------------
drivers/pci/hotplug/shpchp_ctrl.c | 6 +++---
7 files changed, 35 insertions(+), 42 deletions(-)
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v7 1/4] PCI: pciehp: Use per-slot workqueues to avoid deadlock
2013-01-13 23:27 [PATCH v7 0/4] PCI: pciehp, shpchp deadlock fixes Bjorn Helgaas
@ 2013-01-13 23:27 ` Bjorn Helgaas
2013-01-13 23:27 ` [PATCH v7 2/4] PCI: shpchp: Make shpchp_wq non-ordered Bjorn Helgaas
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Bjorn Helgaas @ 2013-01-13 23:27 UTC (permalink / raw)
To: Yijing Wang, linux-pci
Cc: linux-kernel, stable, Daniel J Blueman, Kenji Kaneshige,
Tejun Heo, Yinghai Lu, Jiang Liu
From: Yijing Wang <wangyijing@huawei.com>
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
---
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(-)
diff --git a/drivers/pci/hotplug/pciehp.h b/drivers/pci/hotplug/pciehp.h
index 26ffd3e..2c113de 100644
--- 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 {
diff --git a/drivers/pci/hotplug/pciehp_core.c b/drivers/pci/hotplug/pciehp_core.c
index 916bf4f..939bd1d 100644
--- 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");
}
diff --git a/drivers/pci/hotplug/pciehp_ctrl.c b/drivers/pci/hotplug/pciehp_ctrl.c
index 27f4429..38f0186 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)
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 work_struct *work)
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(struct slot *p_slot)
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 slot *p_slot)
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)
diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
index 13b2eaf..5127f3f 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)
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);
}
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v7 2/4] PCI: shpchp: Make shpchp_wq non-ordered
2013-01-13 23:27 [PATCH v7 0/4] PCI: pciehp, shpchp deadlock fixes Bjorn Helgaas
2013-01-13 23:27 ` [PATCH v7 1/4] PCI: pciehp: Use per-slot workqueues to avoid deadlock Bjorn Helgaas
@ 2013-01-13 23:27 ` Bjorn Helgaas
2013-01-14 16:49 ` Tejun Heo
2013-01-13 23:27 ` [PATCH v7 3/4] PCI: shpchp: Handle push button event asynchronously Bjorn Helgaas
` (2 subsequent siblings)
4 siblings, 1 reply; 7+ messages in thread
From: Bjorn Helgaas @ 2013-01-13 23:27 UTC (permalink / raw)
To: Yijing Wang, linux-pci
Cc: linux-kernel, Daniel J Blueman, Kenji Kaneshige, Tejun Heo,
Yinghai Lu, Jiang Liu
e24dcbef93 ("shpchp: update workqueue usage") was described as adding
non-ordered shpchp_wq, but it actually made it an *ordered* workqueue.
This patch changes shpchp_wq to be non-ordered, as described in the
e24dcbef93 commit log and as was done for pciehp by a827ea307b ("pciehp:
update workqueue usage").
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
CC: Tejun Heo <tj@kernel.org>
---
drivers/pci/hotplug/shpchp_core.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/pci/hotplug/shpchp_core.c b/drivers/pci/hotplug/shpchp_core.c
index b6de307..59ca86c 100644
--- a/drivers/pci/hotplug/shpchp_core.c
+++ b/drivers/pci/hotplug/shpchp_core.c
@@ -366,7 +366,7 @@ static int __init shpcd_init(void)
{
int retval = 0;
- shpchp_wq = alloc_ordered_workqueue("shpchp", 0);
+ shpchp_wq = alloc_workqueue("shpchp", 0, 0);
if (!shpchp_wq)
return -ENOMEM;
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v7 3/4] PCI: shpchp: Handle push button event asynchronously
2013-01-13 23:27 [PATCH v7 0/4] PCI: pciehp, shpchp deadlock fixes Bjorn Helgaas
2013-01-13 23:27 ` [PATCH v7 1/4] PCI: pciehp: Use per-slot workqueues to avoid deadlock Bjorn Helgaas
2013-01-13 23:27 ` [PATCH v7 2/4] PCI: shpchp: Make shpchp_wq non-ordered Bjorn Helgaas
@ 2013-01-13 23:27 ` Bjorn Helgaas
2013-01-13 23:27 ` [PATCH v7 4/4] PCI: shpchp: Use per-slot workqueues to avoid deadlock Bjorn Helgaas
2013-01-14 1:30 ` [PATCH v7 0/4] PCI: pciehp, shpchp deadlock fixes Yijing Wang
4 siblings, 0 replies; 7+ messages in thread
From: Bjorn Helgaas @ 2013-01-13 23:27 UTC (permalink / raw)
To: Yijing Wang, linux-pci
Cc: linux-kernel, Daniel J Blueman, Kenji Kaneshige, Tejun Heo,
Yinghai Lu, Jiang Liu
Use non-ordered workqueue for attention button events.
Attention button events on each slot can be handled asynchronously. So
we should use non-ordered workqueue. This patch also removes ordered
workqueue in shpchp as a result.
486b10b9f4 ("PCI: pciehp: Handle push button event asynchronously") made
the same change to pciehp. I split this out from a patch by Yijing Wang
<wangyijing@huawei.com> so we fix one thing at a time and to make the
shpchp history correspond more closely with the pciehp history.
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
CC: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
---
drivers/pci/hotplug/shpchp.h | 1 -
drivers/pci/hotplug/shpchp_core.c | 10 ----------
drivers/pci/hotplug/shpchp_ctrl.c | 2 +-
3 files changed, 1 insertions(+), 12 deletions(-)
diff --git a/drivers/pci/hotplug/shpchp.h b/drivers/pci/hotplug/shpchp.h
index ca64932..1b69d95 100644
--- a/drivers/pci/hotplug/shpchp.h
+++ b/drivers/pci/hotplug/shpchp.h
@@ -47,7 +47,6 @@ extern bool shpchp_poll_mode;
extern int shpchp_poll_time;
extern bool shpchp_debug;
extern struct workqueue_struct *shpchp_wq;
-extern struct workqueue_struct *shpchp_ordered_wq;
#define dbg(format, arg...) \
do { \
diff --git a/drivers/pci/hotplug/shpchp_core.c b/drivers/pci/hotplug/shpchp_core.c
index 59ca86c..3774e0d 100644
--- a/drivers/pci/hotplug/shpchp_core.c
+++ b/drivers/pci/hotplug/shpchp_core.c
@@ -40,7 +40,6 @@ bool shpchp_debug;
bool shpchp_poll_mode;
int shpchp_poll_time;
struct workqueue_struct *shpchp_wq;
-struct workqueue_struct *shpchp_ordered_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>"
@@ -181,7 +180,6 @@ void cleanup_slots(struct controller *ctrl)
list_del(&slot->slot_list);
cancel_delayed_work(&slot->work);
flush_workqueue(shpchp_wq);
- flush_workqueue(shpchp_ordered_wq);
pci_hp_deregister(slot->hotplug_slot);
}
}
@@ -370,17 +368,10 @@ static int __init shpcd_init(void)
if (!shpchp_wq)
return -ENOMEM;
- shpchp_ordered_wq = alloc_ordered_workqueue("shpchp_ordered", 0);
- if (!shpchp_ordered_wq) {
- destroy_workqueue(shpchp_wq);
- return -ENOMEM;
- }
-
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_ordered_wq);
destroy_workqueue(shpchp_wq);
}
return retval;
@@ -390,7 +381,6 @@ static void __exit shpcd_cleanup(void)
{
dbg("unload_shpchpd()\n");
pci_unregister_driver(&shpc_driver);
- destroy_workqueue(shpchp_ordered_wq);
destroy_workqueue(shpchp_wq);
info(DRIVER_DESC " version: " DRIVER_VERSION " unloaded\n");
}
diff --git a/drivers/pci/hotplug/shpchp_ctrl.c b/drivers/pci/hotplug/shpchp_ctrl.c
index f9b5a52..fd2cae9 100644
--- a/drivers/pci/hotplug/shpchp_ctrl.c
+++ b/drivers/pci/hotplug/shpchp_ctrl.c
@@ -453,7 +453,7 @@ void shpchp_queue_pushbutton_work(struct work_struct *work)
kfree(info);
goto out;
}
- queue_work(shpchp_ordered_wq, &info->work);
+ queue_work(shpchp_wq, &info->work);
out:
mutex_unlock(&p_slot->lock);
}
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v7 4/4] PCI: shpchp: Use per-slot workqueues to avoid deadlock
2013-01-13 23:27 [PATCH v7 0/4] PCI: pciehp, shpchp deadlock fixes Bjorn Helgaas
` (2 preceding siblings ...)
2013-01-13 23:27 ` [PATCH v7 3/4] PCI: shpchp: Handle push button event asynchronously Bjorn Helgaas
@ 2013-01-13 23:27 ` Bjorn Helgaas
2013-01-14 1:30 ` [PATCH v7 0/4] PCI: pciehp, shpchp deadlock fixes Yijing Wang
4 siblings, 0 replies; 7+ messages in thread
From: Bjorn Helgaas @ 2013-01-13 23:27 UTC (permalink / raw)
To: Yijing Wang, linux-pci
Cc: linux-kernel, stable, Daniel J Blueman, Kenji Kaneshige,
Tejun Heo, Yinghai Lu, Jiang Liu
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>
CC: stable@vger.kernel.org
---
drivers/pci/hotplug/shpchp.h | 2 +-
drivers/pci/hotplug/shpchp_core.c | 26 ++++++++++++++------------
drivers/pci/hotplug/shpchp_ctrl.c | 6 +++---
3 files changed, 18 insertions(+), 16 deletions(-)
diff --git a/drivers/pci/hotplug/shpchp.h b/drivers/pci/hotplug/shpchp.h
index 1b69d95..b849f995 100644
--- 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;
};
diff --git a/drivers/pci/hotplug/shpchp_core.c b/drivers/pci/hotplug/shpchp_core.c
index 3774e0d..3100c52 100644
--- 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 *ctrl)
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 *ctrl)
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 *ctrl)
}
return 0;
+error_slotwq:
+ destroy_workqueue(slot->wq);
error_info:
kfree(info);
error_hpslot:
@@ -179,7 +188,7 @@ void cleanup_slots(struct controller *ctrl)
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,12 @@ static struct pci_driver shpc_driver = {
static int __init shpcd_init(void)
{
- int retval = 0;
-
- shpchp_wq = alloc_workqueue("shpchp", 0, 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 +384,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");
}
diff --git a/drivers/pci/hotplug/shpchp_ctrl.c b/drivers/pci/hotplug/shpchp_ctrl.c
index fd2cae9..5849927 100644
--- a/drivers/pci/hotplug/shpchp_ctrl.c
+++ b/drivers/pci/hotplug/shpchp_ctrl.c
@@ -51,7 +51,7 @@ static int queue_interrupt_event(struct slot *p_slot, u32 event_type)
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 work_struct *work)
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(struct slot *p_slot)
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:
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v7 0/4] PCI: pciehp, shpchp deadlock fixes
2013-01-13 23:27 [PATCH v7 0/4] PCI: pciehp, shpchp deadlock fixes Bjorn Helgaas
` (3 preceding siblings ...)
2013-01-13 23:27 ` [PATCH v7 4/4] PCI: shpchp: Use per-slot workqueues to avoid deadlock Bjorn Helgaas
@ 2013-01-14 1:30 ` Yijing Wang
4 siblings, 0 replies; 7+ messages in thread
From: Yijing Wang @ 2013-01-14 1:30 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-pci, linux-kernel, Daniel J Blueman, Kenji Kaneshige,
Tejun Heo, Yinghai Lu, Jiang Liu
On 2013/1/14 7:27, Bjorn Helgaas wrote:
> This is v7 of Yijing Wang's pciehp deadlock fix. Here's the history of
> previous postings:
>
> RFC Oct 31 2012
> v2 Nov 9 2012 use slot name, not device name
> v3 Nov 12 2012 create workqueues in pcie_init_slot(), pciehp-%u format
> v4 Nov 13 2012 use alloc_workqueue(name, 0, 0) again
> v5 Jan 9 2013 fix similar problem in shpchp
> v6 Jan 10 2013 add pciehp backtrace from Daniel
>
> This v7 series is functionally identical to v6. In v7, I split the single
> shpchp patch into three:
>
> PCI: shpchp: Make shpchp_wq non-ordered
> PCI: shpchp: Handle push button event asynchronously
> PCI: shpchp: Use per-slot workqueues to avoid deadlock
>
Nice, split the single patch into three will make the log clearer about the
code changes.
> The first fixes what appears to be an error in e24dcbef93 ("shpchp: update
> workqueue usage"). I split this out so Tejun could easily review it by itself.
>
> I split the second out because it corresponds to a pciehp bugfix, 486b10b9f4
> ("PCI: pciehp: Handle push button event asynchronously"), that also affects
> shpchp.
>
> The "PCI: shpchp: Use per-slot workqueues to avoid deadlock" that remains
> then corresponds exactly to Yijing's original pciehp deadlock fix.
>
> I also rewrote the changelogs.
>
> I propose to push the entire series for inclusion in v3.8, since it
> fixes an easy-to-cause deadlock with Thunderbolt adapters.
>
> ---
>
> Bjorn Helgaas (3):
> PCI: shpchp: Make shpchp_wq non-ordered
> PCI: shpchp: Handle push button event asynchronously
> PCI: shpchp: Use per-slot workqueues to avoid deadlock
>
> Yijing Wang (1):
> PCI: pciehp: Use per-slot workqueues to avoid deadlock
>
>
> 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 ++++++++++-
> drivers/pci/hotplug/shpchp.h | 3 +--
> drivers/pci/hotplug/shpchp_core.c | 36 ++++++++++++++----------------------
> drivers/pci/hotplug/shpchp_ctrl.c | 6 +++---
> 7 files changed, 35 insertions(+), 42 deletions(-)
>
>
--
Thanks!
Yijing
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v7 2/4] PCI: shpchp: Make shpchp_wq non-ordered
2013-01-13 23:27 ` [PATCH v7 2/4] PCI: shpchp: Make shpchp_wq non-ordered Bjorn Helgaas
@ 2013-01-14 16:49 ` Tejun Heo
0 siblings, 0 replies; 7+ messages in thread
From: Tejun Heo @ 2013-01-14 16:49 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Yijing Wang, linux-pci, linux-kernel, Daniel J Blueman,
Kenji Kaneshige, Yinghai Lu, Jiang Liu
Hello,
On Sun, Jan 13, 2013 at 04:27:43PM -0700, Bjorn Helgaas wrote:
> e24dcbef93 ("shpchp: update workqueue usage") was described as adding
> non-ordered shpchp_wq, but it actually made it an *ordered* workqueue.
>
> This patch changes shpchp_wq to be non-ordered, as described in the
> e24dcbef93 commit log and as was done for pciehp by a827ea307b ("pciehp:
> update workqueue usage").
>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> CC: Tejun Heo <tj@kernel.org>
Yeap, my mistake.
Acked-by: Tejun Heo <tj@kernel.org>
Thanks!
--
tejun
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-01-14 16:49 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-13 23:27 [PATCH v7 0/4] PCI: pciehp, shpchp deadlock fixes Bjorn Helgaas
2013-01-13 23:27 ` [PATCH v7 1/4] PCI: pciehp: Use per-slot workqueues to avoid deadlock Bjorn Helgaas
2013-01-13 23:27 ` [PATCH v7 2/4] PCI: shpchp: Make shpchp_wq non-ordered Bjorn Helgaas
2013-01-14 16:49 ` Tejun Heo
2013-01-13 23:27 ` [PATCH v7 3/4] PCI: shpchp: Handle push button event asynchronously Bjorn Helgaas
2013-01-13 23:27 ` [PATCH v7 4/4] PCI: shpchp: Use per-slot workqueues to avoid deadlock Bjorn Helgaas
2013-01-14 1:30 ` [PATCH v7 0/4] PCI: pciehp, shpchp deadlock fixes Yijing Wang
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.