This is a preparation for next patch to avoid breaking bisecting.
If next patch is applied without this one, it will cause deadlock
as below:
Case 1:
[ 31.015593] Possible unsafe locking scenario:
[ 31.018350] CPU0 CPU1
[ 31.019691] ---- ----
[ 31.021002] lock(&dock_station->hp_lock);
[ 31.022327] lock(&slot->crit_sect);
[ 31.023650] lock(&dock_station->hp_lock);
[ 31.025010] lock(&slot->crit_sect);
[ 31.026342]
Case 2:
hotplug_dock_devices()
mutex_lock(&ds->hp_lock)
dd->ops->handler()
register_hotplug_dock_device()
mutex_lock(&ds->hp_lock)
[ 34.316570] [ INFO: possible recursive locking detected ]
[ 34.316573] 3.10.0-rc4 #6 Tainted: G C
[ 34.316575] ---------------------------------------------
[ 34.316577] kworker/0:0/4 is trying to acquire lock:
[ 34.316579] (&dock_station->hp_lock){+.+.+.}, at:
[<ffffffff813c766b>] register_hotplug_dock_device+0x6a/0xbf
[ 34.316588]
but task is already holding lock:
[ 34.316590] (&dock_station->hp_lock){+.+.+.}, at:
[<ffffffff813c7270>] hotplug_dock_devices+0x2c/0xda
[ 34.316595]
other info that might help us debug this:
[ 34.316597] Possible unsafe locking scenario:
[ 34.316599] CPU0
[ 34.316601] ----
[ 34.316602] lock(&dock_station->hp_lock);
[ 34.316605] lock(&dock_station->hp_lock);
[ 34.316608]
*** DEADLOCK ***
So fix this deadlock by not taking ds->hp_lock in function
register_hotplug_dock_device(). This patch also fixes a possible
race conditions in function dock_event() because previously it
accesses ds->hotplug_devices list without holding ds->hp_lock.
Signed-off-by: Jiang Liu <jiang.liu@xxxxxxxxxx>
Cc: Len Brown <lenb@xxxxxxxxxx>
Cc: "Rafael J. Wysocki" <rjw@xxxxxxx>
Cc: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
Cc: Yinghai Lu <yinghai@xxxxxxxxxx>
Cc: Yijing Wang <wangyijing@xxxxxxxxxx>
Cc: linux-acpi@xxxxxxxxxxxxxxx
Cc: linux-kernel@xxxxxxxxxxxxxxx
Cc: linux-pci@xxxxxxxxxxxxxxx
Cc: <stable@xxxxxxxxxxxxxxx> # 3.8+
---
drivers/acpi/dock.c | 109 ++++++++++++++++++++++---------------
drivers/pci/hotplug/acpiphp_glue.c | 15 +++++
include/acpi/acpi_drivers.h | 2 +
3 files changed, 82 insertions(+), 44 deletions(-)
diff --git a/drivers/acpi/dock.c b/drivers/acpi/dock.c
index 02b0563..602bce5 100644
--- a/drivers/acpi/dock.c
+++ b/drivers/acpi/dock.c
@@ -66,7 +66,7 @@ struct dock_station {
spinlock_t dd_lock;
struct mutex hp_lock;
struct list_head dependent_devices;
- struct list_head hotplug_devices;
+ struct klist hotplug_devices;
struct list_head sibling;
struct platform_device *dock_device;
@@ -76,12 +76,18 @@ static int dock_station_count;
struct dock_dependent_device {
struct list_head list;
- struct list_head hotplug_list;
+ acpi_handle handle;
+};
+
+struct dock_hotplug_info {
+ struct klist_node node;
acpi_handle handle;
const struct acpi_dock_ops *ops;
void *context;
};
+#define node_to_info(n) container_of((n), struct dock_hotplug_info, node)
+
#define DOCK_DOCKING 0x00000001
#define DOCK_UNDOCKING 0x00000002
#define DOCK_IS_DOCK 0x00000010
@@ -111,7 +117,6 @@ add_dock_dependent_device(struct dock_station *ds, acpi_handle handle)
dd->handle = handle;
INIT_LIST_HEAD(&dd->list);
- INIT_LIST_HEAD(&dd->hotplug_list);
spin_lock(&ds->dd_lock);
list_add_tail(&dd->list, &ds->dependent_devices);
@@ -120,36 +125,19 @@ add_dock_dependent_device(struct dock_station *ds, acpi_handle handle)
return 0;
}
-/**
- * dock_add_hotplug_device - associate a hotplug handler with the dock station
- * @ds: The dock station
- * @dd: The dependent device struct
- *
- * Add the dependent device to the dock's hotplug device list
- */
-static void
-dock_add_hotplug_device(struct dock_station *ds,
- struct dock_dependent_device *dd)
+static void hotplug_info_get(struct klist_node *node)
{
- mutex_lock(&ds->hp_lock);
- list_add_tail(&dd->hotplug_list, &ds->hotplug_devices);
- mutex_unlock(&ds->hp_lock);
+ struct dock_hotplug_info *info = node_to_info(node);
+
+ info->ops->get(info->context);
}
-/**
- * dock_del_hotplug_device - remove a hotplug handler from the dock station
- * @ds: The dock station
- * @dd: the dependent device struct
- *
- * Delete the dependent device from the dock's hotplug device list
- */
-static void
-dock_del_hotplug_device(struct dock_station *ds,
- struct dock_dependent_device *dd)
+static void hotplug_info_put(struct klist_node *node)
{
- mutex_lock(&ds->hp_lock);
- list_del(&dd->hotplug_list);
- mutex_unlock(&ds->hp_lock);
+ struct dock_hotplug_info *info = node_to_info(node);
+
+ info->ops->put(info->context);
+ kfree(info);
}
/**
@@ -354,15 +342,22 @@ static void dock_remove_acpi_device(acpi_handle handle)
static void hotplug_dock_devices(struct dock_station *ds, u32 event)
{
struct dock_dependent_device *dd;
+ struct klist_iter iter;
+ struct klist_node *node;
+ struct dock_hotplug_info *info;
mutex_lock(&ds->hp_lock);
/*
* First call driver specific hotplug functions
*/
- list_for_each_entry(dd, &ds->hotplug_devices, hotplug_list)
- if (dd->ops && dd->ops->handler)
- dd->ops->handler(dd->handle, event, dd->context);
+ klist_iter_init(&ds->hotplug_devices, &iter);
+ while ((node = klist_next(&iter))) {
+ info = node_to_info(node);
+ if (info->ops && info->ops->handler)
+ info->ops->handler(info->handle, event, info->context);
+ }
+ klist_iter_exit(&iter);
/*
* Now make sure that an acpi_device is created for each
@@ -384,7 +379,9 @@ static void dock_event(struct dock_station *ds, u32 event, int num)
struct device *dev = &ds->dock_device->dev;
char event_string[13];
char *envp[] = { event_string, NULL };
- struct dock_dependent_device *dd;
+ struct klist_iter iter;
+ struct klist_node *node;
+ struct dock_hotplug_info *info;
if (num == UNDOCK_EVENT)
sprintf(event_string, "EVENT=undock");
@@ -398,9 +395,13 @@ static void dock_event(struct dock_station *ds, u32 event, int num)
if (num == DOCK_EVENT)
kobject_uevent_env(&dev->kobj, KOBJ_CHANGE, envp);
- list_for_each_entry(dd, &ds->hotplug_devices, hotplug_list)
- if (dd->ops && dd->ops->uevent)
- dd->ops->uevent(dd->handle, event, dd->context);
+ klist_iter_init(&ds->hotplug_devices, &iter);
+ while ((node = klist_next(&iter))) {
+ info = node_to_info(node);
+ if (info->ops && info->ops->handler)
+ info->ops->handler(info->handle, event, info->context);
+ }
+ klist_iter_exit(&iter);
if (num != DOCK_EVENT)
kobject_uevent_env(&dev->kobj, KOBJ_CHANGE, envp);
@@ -580,12 +581,16 @@ register_hotplug_dock_device(acpi_handle handle, const struct acpi_dock_ops *ops
void *context)
{
struct dock_dependent_device *dd;
+ struct dock_hotplug_info *info;
struct dock_station *dock_station;
int ret = -EINVAL;
if (!dock_station_count)
return -ENODEV;
+ if (ops == NULL || ops->get == NULL || ops->put == NULL)
+ return -EINVAL;
+
/*
* make sure this handle is for a device dependent on the dock,
* this would include the dock station itself
@@ -598,9 +603,18 @@ register_hotplug_dock_device(acpi_handle handle, const struct acpi_dock_ops *ops
*/
dd = find_dock_dependent_device(dock_station, handle);
if (dd) {
- dd->ops = ops;
- dd->context = context;
- dock_add_hotplug_device(dock_station, dd);
+ info = kzalloc(sizeof(*info), GFP_KERNEL);
+ if (!info) {
+ unregister_hotplug_dock_device(handle);
+ ret = -ENOMEM;
+ break;
+ }
+
+ info->ops = ops;
+ info->context = context;
+ info->handle = dd->handle;
+ klist_add_tail(&info->node,
+ &dock_station->hotplug_devices);
ret = 0;
}
}
@@ -615,16 +629,22 @@ EXPORT_SYMBOL_GPL(register_hotplug_dock_device);
*/
void unregister_hotplug_dock_device(acpi_handle handle)
{
- struct dock_dependent_device *dd;
struct dock_station *dock_station;
+ struct klist_iter iter;
+ struct klist_node *node;
+ struct dock_hotplug_info *info;
if (!dock_station_count)
return;
list_for_each_entry(dock_station, &dock_stations, sibling) {
- dd = find_dock_dependent_device(dock_station, handle);
- if (dd)
- dock_del_hotplug_device(dock_station, dd);
+ klist_iter_init(&dock_station->hotplug_devices, &iter);
+ while ((node = klist_next(&iter))) {
+ info = node_to_info(node);
+ if (info->handle == handle)
+ klist_del(&info->node);
+ }
+ klist_iter_exit(&iter);
}
}
EXPORT_SYMBOL_GPL(unregister_hotplug_dock_device);
@@ -951,7 +971,8 @@ static int __init dock_add(acpi_handle handle)
mutex_init(&dock_station->hp_lock);
spin_lock_init(&dock_station->dd_lock);
INIT_LIST_HEAD(&dock_station->sibling);
- INIT_LIST_HEAD(&dock_station->hotplug_devices);
+ klist_init(&dock_station->hotplug_devices,
+ hotplug_info_get, hotplug_info_put);
ATOMIC_INIT_NOTIFIER_HEAD(&dock_notifier_list);
INIT_LIST_HEAD(&dock_station->dependent_devices);
diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
index 716aa93..5d696f5 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -145,9 +145,24 @@ static int post_dock_fixups(struct notifier_block *nb, unsigned long val,
return NOTIFY_OK;
}
+static void acpiphp_dock_get(void *data)
+{
+ struct acpiphp_func *func = data;
+
+ get_bridge(func->slot->bridge);
+}
+
+static void acpiphp_dock_put(void *data)
+{
+ struct acpiphp_func *func = data;
+
+ put_bridge(func->slot->bridge);
+}
static const struct acpi_dock_ops acpiphp_dock_ops = {
.handler = handle_hotplug_event_func,
+ .get = acpiphp_dock_get,
+ .put = acpiphp_dock_put,
};
/* Check whether the PCI device is managed by native PCIe hotplug driver */
diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
index e6168a2..8fcc9ac 100644
--- a/include/acpi/acpi_drivers.h
+++ b/include/acpi/acpi_drivers.h
@@ -115,6 +115,8 @@ void pci_acpi_crs_quirks(void);
struct acpi_dock_ops {
acpi_notify_handler handler;
acpi_notify_handler uevent;
+ void (*get)(void *data);
+ void (*put)(void *data);
};
#if defined(CONFIG_ACPI_DOCK) || defined(CONFIG_ACPI_DOCK_MODULE)
--
1.8.1.2
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
