summaryrefslogtreecommitdiff
path: root/drivers/acpi/dock.c
diff options
context:
space:
mode:
authorRafael J. Wysocki <rafael.j.wysocki@intel.com>2013-06-24 11:22:53 +0200
committerRafael J. Wysocki <rafael.j.wysocki@intel.com>2013-06-24 11:22:53 +0200
commit21a31013f774c726bd199526cd673acc6432b21d (patch)
treeba9659d45f91fda6f574b9488c1a273bd6c105c4 /drivers/acpi/dock.c
parentd66ecb7220a70ec3f6c0e38e4af28fb8b25d31c6 (diff)
ACPI / dock / PCI: Synchronous handling of dock events for PCI devices
The interactions between the ACPI dock driver and the ACPI-based PCI hotplug (acpiphp) are currently problematic because of ordering issues during hot-remove operations. First of all, the current ACPI glue code expects that physical devices will always be deleted before deleting the companion ACPI device objects. Otherwise, acpi_unbind_one() will fail with a warning message printed to the kernel log, for example: [ 185.026073] usb usb5: Oops, 'acpi_handle' corrupt [ 185.035150] pci 0000:1b:00.0: Oops, 'acpi_handle' corrupt [ 185.035515] pci 0000:18:02.0: Oops, 'acpi_handle' corrupt [ 180.013656] port1: Oops, 'acpi_handle' corrupt This means, in particular, that struct pci_dev objects have to be deleted before the struct acpi_device objects they are "glued" with. Now, the following happens the during the undocking of an ACPI-based dock station: 1) hotplug_dock_devices() invokes registered hotplug callbacks to destroy physical devices associated with the ACPI device objects depending on the dock station. It calls dd->ops->handler() for each of those device objects. 2) For PCI devices dd->ops->handler() points to handle_hotplug_event_func() that queues up a separate work item to execute _handle_hotplug_event_func() for the given device and returns immediately. That work item will be executed later. 3) hotplug_dock_devices() calls dock_remove_acpi_device() for each device depending on the dock station. This runs acpi_bus_trim() for each of them, which causes the underlying ACPI device object to be destroyed, but the work items queued up by handle_hotplug_event_func() haven't been started yet. 4) _handle_hotplug_event_func() queued up in step 2) are executed and cause the above failure to happen, because the PCI devices they handle do not have the companion ACPI device objects any more (those objects have been deleted in step 3). The possible breakage doesn't end here, though, because hotplug_dock_devices() may return before at least some of the _handle_hotplug_event_func() work items spawned by it have a chance to complete and then undock() will cause _DCK to be evaluated and that will cause the devices handled by the _handle_hotplug_event_func() to go away possibly while they are being accessed. This means that dd->ops->handler() for PCI devices should not point to handle_hotplug_event_func(). Instead, it should point to a function that will do the work of _handle_hotplug_event_func() synchronously. For this reason, introduce such a function, hotplug_event_func(), and modity acpiphp_dock_ops to point to it as the handler. Unfortunately, however, this is not sufficient, because if the dock code were not changed further, hotplug_event_func() would now deadlock with hotplug_dock_devices() that called it, since it would run unregister_hotplug_dock_device() which in turn would attempt to acquire the dock station's hp_lock mutex already acquired by hotplug_dock_devices(). To resolve that deadlock use the observation that unregister_hotplug_dock_device() won't need to acquire hp_lock if PCI bridges the devices on the dock station depend on are prevented from being removed prematurely while the first loop in hotplug_dock_devices() is in progress. To make that possible, introduce a mechanism by which the callers of register_hotplug_dock_device() can provide "init" and "release" routines that will be executed, respectively, during the addition and removal of the physical device object associated with the given ACPI device handle. Make acpiphp use two new functions, acpiphp_dock_init() and acpiphp_dock_release(), that call get_bridge() and put_bridge(), respectively, on the acpiphp bridge holding the given device, for this purpose. In addition to that, remove the dock station's list of "hotplug devices" and make the dock code always walk the whole list of "dependent devices" instead in such a way that the loops in hotplug_dock_devices() and dock_event() (replacing the loops over "hotplug devices") will take references to the list entries that register_hotplug_dock_device() has been called for. That prevents the "release" routines associated with those entries from being called while the given entry is being processed and for PCI devices this means that their bridges won't be removed (by a concurrent thread) while hotplug_event_func() handling them is being executed. This change is based on two earlier patches from Jiang Liu. References: https://bugzilla.kernel.org/show_bug.cgi?id=59501 Reported-and-tested-by: Alexander E. Patrakov <patrakov@gmail.com> Tracked-down-by: Jiang Liu <jiang.liu@huawei.com> Tested-by: Illya Klymov <xanf@xanf.me> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Acked-by: Yinghai Lu <yinghai@kernel.org> Cc: 3.9+ <stable@vger.kernel.org>
Diffstat (limited to 'drivers/acpi/dock.c')
-rw-r--r--drivers/acpi/dock.c137
1 files changed, 95 insertions, 42 deletions
diff --git a/drivers/acpi/dock.c b/drivers/acpi/dock.c
index c3b34f94382f..14de9f46972e 100644
--- a/drivers/acpi/dock.c
+++ b/drivers/acpi/dock.c
@@ -66,20 +66,21 @@ struct dock_station {
spinlock_t dd_lock;
struct mutex hp_lock;
struct list_head dependent_devices;
- struct list_head hotplug_devices;
struct list_head sibling;
struct platform_device *dock_device;
};
static LIST_HEAD(dock_stations);
static int dock_station_count;
+static DEFINE_MUTEX(hotplug_lock);
struct dock_dependent_device {
struct list_head list;
- struct list_head hotplug_list;
acpi_handle handle;
- const struct acpi_dock_ops *ops;
- void *context;
+ const struct acpi_dock_ops *hp_ops;
+ void *hp_context;
+ unsigned int hp_refcount;
+ void (*hp_release)(void *);
};
#define DOCK_DOCKING 0x00000001
@@ -111,7 +112,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);
@@ -121,35 +121,90 @@ add_dock_dependent_device(struct dock_station *ds, acpi_handle handle)
}
/**
- * 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
+ * dock_init_hotplug - Initialize a hotplug device on a docking station.
+ * @dd: Dock-dependent device.
+ * @ops: Dock operations to attach to the dependent device.
+ * @context: Data to pass to the @ops callbacks and @release.
+ * @init: Optional initialization routine to run after setting up context.
+ * @release: Optional release routine to run on removal.
*/
-static void
-dock_add_hotplug_device(struct dock_station *ds,
- struct dock_dependent_device *dd)
+static int dock_init_hotplug(struct dock_dependent_device *dd,
+ const struct acpi_dock_ops *ops, void *context,
+ void (*init)(void *), void (*release)(void *))
{
- mutex_lock(&ds->hp_lock);
- list_add_tail(&dd->hotplug_list, &ds->hotplug_devices);
- mutex_unlock(&ds->hp_lock);
+ int ret = 0;
+
+ mutex_lock(&hotplug_lock);
+
+ if (dd->hp_context) {
+ ret = -EEXIST;
+ } else {
+ dd->hp_refcount = 1;
+ dd->hp_ops = ops;
+ dd->hp_context = context;
+ dd->hp_release = release;
+ }
+
+ if (!WARN_ON(ret) && init)
+ init(context);
+
+ mutex_unlock(&hotplug_lock);
+ return ret;
}
/**
- * dock_del_hotplug_device - remove a hotplug handler from the dock station
- * @ds: The dock station
- * @dd: the dependent device struct
+ * dock_release_hotplug - Decrement hotplug reference counter of dock device.
+ * @dd: Dock-dependent device.
*
- * Delete the dependent device from the dock's hotplug device list
+ * Decrement the reference counter of @dd and if 0, detach its hotplug
+ * operations from it, reset its context pointer and run the optional release
+ * routine if present.
*/
-static void
-dock_del_hotplug_device(struct dock_station *ds,
- struct dock_dependent_device *dd)
+static void dock_release_hotplug(struct dock_dependent_device *dd)
{
- mutex_lock(&ds->hp_lock);
- list_del(&dd->hotplug_list);
- mutex_unlock(&ds->hp_lock);
+ void (*release)(void *) = NULL;
+ void *context = NULL;
+
+ mutex_lock(&hotplug_lock);
+
+ if (dd->hp_context && !--dd->hp_refcount) {
+ dd->hp_ops = NULL;
+ context = dd->hp_context;
+ dd->hp_context = NULL;
+ release = dd->hp_release;
+ dd->hp_release = NULL;
+ }
+
+ if (release && context)
+ release(context);
+
+ mutex_unlock(&hotplug_lock);
+}
+
+static void dock_hotplug_event(struct dock_dependent_device *dd, u32 event,
+ bool uevent)
+{
+ acpi_notify_handler cb = NULL;
+ bool run = false;
+
+ mutex_lock(&hotplug_lock);
+
+ if (dd->hp_context) {
+ run = true;
+ dd->hp_refcount++;
+ if (dd->hp_ops)
+ cb = uevent ? dd->hp_ops->uevent : dd->hp_ops->handler;
+ }
+
+ mutex_unlock(&hotplug_lock);
+
+ if (!run)
+ return;
+
+ if (cb)
+ cb(dd->handle, event, dd->hp_context);
+
+ dock_release_hotplug(dd);
}
/**
@@ -360,9 +415,8 @@ static void hotplug_dock_devices(struct dock_station *ds, u32 event)
/*
* 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);
+ list_for_each_entry(dd, &ds->dependent_devices, list)
+ dock_hotplug_event(dd, event, false);
/*
* Now make sure that an acpi_device is created for each
@@ -398,9 +452,8 @@ 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);
+ list_for_each_entry(dd, &ds->dependent_devices, list)
+ dock_hotplug_event(dd, event, true);
if (num != DOCK_EVENT)
kobject_uevent_env(&dev->kobj, KOBJ_CHANGE, envp);
@@ -570,19 +623,24 @@ EXPORT_SYMBOL_GPL(unregister_dock_notifier);
* @handle: the handle of the device
* @ops: handlers to call after docking
* @context: device specific data
+ * @init: Optional initialization routine to run after registration
+ * @release: Optional release routine to run on unregistration
*
* If a driver would like to perform a hotplug operation after a dock
* event, they can register an acpi_notifiy_handler to be called by
* the dock driver after _DCK is executed.
*/
-int
-register_hotplug_dock_device(acpi_handle handle, const struct acpi_dock_ops *ops,
- void *context)
+int register_hotplug_dock_device(acpi_handle handle,
+ const struct acpi_dock_ops *ops, void *context,
+ void (*init)(void *), void (*release)(void *))
{
struct dock_dependent_device *dd;
struct dock_station *dock_station;
int ret = -EINVAL;
+ if (WARN_ON(!context))
+ return -EINVAL;
+
if (!dock_station_count)
return -ENODEV;
@@ -597,12 +655,8 @@ register_hotplug_dock_device(acpi_handle handle, const struct acpi_dock_ops *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);
+ if (dd && !dock_init_hotplug(dd, ops, context, init, release))
ret = 0;
- }
}
return ret;
@@ -624,7 +678,7 @@ void unregister_hotplug_dock_device(acpi_handle handle)
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);
+ dock_release_hotplug(dd);
}
}
EXPORT_SYMBOL_GPL(unregister_hotplug_dock_device);
@@ -953,7 +1007,6 @@ 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);
ATOMIC_INIT_NOTIFIER_HEAD(&dock_notifier_list);
INIT_LIST_HEAD(&dock_station->dependent_devices);