aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRafael J. Wysocki <rafael.j.wysocki@intel.com>2013-06-24 05:22:53 -0400
committerRafael J. Wysocki <rafael.j.wysocki@intel.com>2013-06-24 05:22:53 -0400
commit21a31013f774c726bd199526cd673acc6432b21d (patch)
treeba9659d45f91fda6f574b9488c1a273bd6c105c4
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>
-rw-r--r--drivers/acpi/dock.c137
-rw-r--r--drivers/pci/hotplug/acpiphp_glue.c46
-rw-r--r--include/acpi/acpi_drivers.h8
3 files changed, 133 insertions, 58 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 {
66 spinlock_t dd_lock; 66 spinlock_t dd_lock;
67 struct mutex hp_lock; 67 struct mutex hp_lock;
68 struct list_head dependent_devices; 68 struct list_head dependent_devices;
69 struct list_head hotplug_devices;
70 69
71 struct list_head sibling; 70 struct list_head sibling;
72 struct platform_device *dock_device; 71 struct platform_device *dock_device;
73}; 72};
74static LIST_HEAD(dock_stations); 73static LIST_HEAD(dock_stations);
75static int dock_station_count; 74static int dock_station_count;
75static DEFINE_MUTEX(hotplug_lock);
76 76
77struct dock_dependent_device { 77struct dock_dependent_device {
78 struct list_head list; 78 struct list_head list;
79 struct list_head hotplug_list;
80 acpi_handle handle; 79 acpi_handle handle;
81 const struct acpi_dock_ops *ops; 80 const struct acpi_dock_ops *hp_ops;
82 void *context; 81 void *hp_context;
82 unsigned int hp_refcount;
83 void (*hp_release)(void *);
83}; 84};
84 85
85#define DOCK_DOCKING 0x00000001 86#define DOCK_DOCKING 0x00000001
@@ -111,7 +112,6 @@ add_dock_dependent_device(struct dock_station *ds, acpi_handle handle)
111 112
112 dd->handle = handle; 113 dd->handle = handle;
113 INIT_LIST_HEAD(&dd->list); 114 INIT_LIST_HEAD(&dd->list);
114 INIT_LIST_HEAD(&dd->hotplug_list);
115 115
116 spin_lock(&ds->dd_lock); 116 spin_lock(&ds->dd_lock);
117 list_add_tail(&dd->list, &ds->dependent_devices); 117 list_add_tail(&dd->list, &ds->dependent_devices);
@@ -121,35 +121,90 @@ add_dock_dependent_device(struct dock_station *ds, acpi_handle handle)
121} 121}
122 122
123/** 123/**
124 * dock_add_hotplug_device - associate a hotplug handler with the dock station 124 * dock_init_hotplug - Initialize a hotplug device on a docking station.
125 * @ds: The dock station 125 * @dd: Dock-dependent device.
126 * @dd: The dependent device struct 126 * @ops: Dock operations to attach to the dependent device.
127 * 127 * @context: Data to pass to the @ops callbacks and @release.
128 * Add the dependent device to the dock's hotplug device list 128 * @init: Optional initialization routine to run after setting up context.
129 * @release: Optional release routine to run on removal.
129 */ 130 */
130static void 131static int dock_init_hotplug(struct dock_dependent_device *dd,
131dock_add_hotplug_device(struct dock_station *ds, 132 const struct acpi_dock_ops *ops, void *context,
132 struct dock_dependent_device *dd) 133 void (*init)(void *), void (*release)(void *))
133{ 134{
134 mutex_lock(&ds->hp_lock); 135 int ret = 0;
135 list_add_tail(&dd->hotplug_list, &ds->hotplug_devices); 136
136 mutex_unlock(&ds->hp_lock); 137 mutex_lock(&hotplug_lock);
138
139 if (dd->hp_context) {
140 ret = -EEXIST;
141 } else {
142 dd->hp_refcount = 1;
143 dd->hp_ops = ops;
144 dd->hp_context = context;
145 dd->hp_release = release;
146 }
147
148 if (!WARN_ON(ret) && init)
149 init(context);
150
151 mutex_unlock(&hotplug_lock);
152 return ret;
137} 153}
138 154
139/** 155/**
140 * dock_del_hotplug_device - remove a hotplug handler from the dock station 156 * dock_release_hotplug - Decrement hotplug reference counter of dock device.
141 * @ds: The dock station 157 * @dd: Dock-dependent device.
142 * @dd: the dependent device struct
143 * 158 *
144 * Delete the dependent device from the dock's hotplug device list 159 * Decrement the reference counter of @dd and if 0, detach its hotplug
160 * operations from it, reset its context pointer and run the optional release
161 * routine if present.
145 */ 162 */
146static void 163static void dock_release_hotplug(struct dock_dependent_device *dd)
147dock_del_hotplug_device(struct dock_station *ds,
148 struct dock_dependent_device *dd)
149{ 164{
150 mutex_lock(&ds->hp_lock); 165 void (*release)(void *) = NULL;
151 list_del(&dd->hotplug_list); 166 void *context = NULL;
152 mutex_unlock(&ds->hp_lock); 167
168 mutex_lock(&hotplug_lock);
169
170 if (dd->hp_context && !--dd->hp_refcount) {
171 dd->hp_ops = NULL;
172 context = dd->hp_context;
173 dd->hp_context = NULL;
174 release = dd->hp_release;
175 dd->hp_release = NULL;
176 }
177
178 if (release && context)
179 release(context);
180
181 mutex_unlock(&hotplug_lock);
182}
183
184static void dock_hotplug_event(struct dock_dependent_device *dd, u32 event,
185 bool uevent)
186{
187 acpi_notify_handler cb = NULL;
188 bool run = false;
189
190 mutex_lock(&hotplug_lock);
191
192 if (dd->hp_context) {
193 run = true;
194 dd->hp_refcount++;
195 if (dd->hp_ops)
196 cb = uevent ? dd->hp_ops->uevent : dd->hp_ops->handler;
197 }
198
199 mutex_unlock(&hotplug_lock);
200
201 if (!run)
202 return;
203
204 if (cb)
205 cb(dd->handle, event, dd->hp_context);
206
207 dock_release_hotplug(dd);
153} 208}
154 209
155/** 210/**
@@ -360,9 +415,8 @@ static void hotplug_dock_devices(struct dock_station *ds, u32 event)
360 /* 415 /*
361 * First call driver specific hotplug functions 416 * First call driver specific hotplug functions
362 */ 417 */
363 list_for_each_entry(dd, &ds->hotplug_devices, hotplug_list) 418 list_for_each_entry(dd, &ds->dependent_devices, list)
364 if (dd->ops && dd->ops->handler) 419 dock_hotplug_event(dd, event, false);
365 dd->ops->handler(dd->handle, event, dd->context);
366 420
367 /* 421 /*
368 * Now make sure that an acpi_device is created for each 422 * 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)
398 if (num == DOCK_EVENT) 452 if (num == DOCK_EVENT)
399 kobject_uevent_env(&dev->kobj, KOBJ_CHANGE, envp); 453 kobject_uevent_env(&dev->kobj, KOBJ_CHANGE, envp);
400 454
401 list_for_each_entry(dd, &ds->hotplug_devices, hotplug_list) 455 list_for_each_entry(dd, &ds->dependent_devices, list)
402 if (dd->ops && dd->ops->uevent) 456 dock_hotplug_event(dd, event, true);
403 dd->ops->uevent(dd->handle, event, dd->context);
404 457
405 if (num != DOCK_EVENT) 458 if (num != DOCK_EVENT)
406 kobject_uevent_env(&dev->kobj, KOBJ_CHANGE, envp); 459 kobject_uevent_env(&dev->kobj, KOBJ_CHANGE, envp);
@@ -570,19 +623,24 @@ EXPORT_SYMBOL_GPL(unregister_dock_notifier);
570 * @handle: the handle of the device 623 * @handle: the handle of the device
571 * @ops: handlers to call after docking 624 * @ops: handlers to call after docking
572 * @context: device specific data 625 * @context: device specific data
626 * @init: Optional initialization routine to run after registration
627 * @release: Optional release routine to run on unregistration
573 * 628 *
574 * If a driver would like to perform a hotplug operation after a dock 629 * If a driver would like to perform a hotplug operation after a dock
575 * event, they can register an acpi_notifiy_handler to be called by 630 * event, they can register an acpi_notifiy_handler to be called by
576 * the dock driver after _DCK is executed. 631 * the dock driver after _DCK is executed.
577 */ 632 */
578int 633int register_hotplug_dock_device(acpi_handle handle,
579register_hotplug_dock_device(acpi_handle handle, const struct acpi_dock_ops *ops, 634 const struct acpi_dock_ops *ops, void *context,
580 void *context) 635 void (*init)(void *), void (*release)(void *))
581{ 636{
582 struct dock_dependent_device *dd; 637 struct dock_dependent_device *dd;
583 struct dock_station *dock_station; 638 struct dock_station *dock_station;
584 int ret = -EINVAL; 639 int ret = -EINVAL;
585 640
641 if (WARN_ON(!context))
642 return -EINVAL;
643
586 if (!dock_station_count) 644 if (!dock_station_count)
587 return -ENODEV; 645 return -ENODEV;
588 646
@@ -597,12 +655,8 @@ register_hotplug_dock_device(acpi_handle handle, const struct acpi_dock_ops *ops
597 * ops 655 * ops
598 */ 656 */
599 dd = find_dock_dependent_device(dock_station, handle); 657 dd = find_dock_dependent_device(dock_station, handle);
600 if (dd) { 658 if (dd && !dock_init_hotplug(dd, ops, context, init, release))
601 dd->ops = ops;
602 dd->context = context;
603 dock_add_hotplug_device(dock_station, dd);
604 ret = 0; 659 ret = 0;
605 }
606 } 660 }
607 661
608 return ret; 662 return ret;
@@ -624,7 +678,7 @@ void unregister_hotplug_dock_device(acpi_handle handle)
624 list_for_each_entry(dock_station, &dock_stations, sibling) { 678 list_for_each_entry(dock_station, &dock_stations, sibling) {
625 dd = find_dock_dependent_device(dock_station, handle); 679 dd = find_dock_dependent_device(dock_station, handle);
626 if (dd) 680 if (dd)
627 dock_del_hotplug_device(dock_station, dd); 681 dock_release_hotplug(dd);
628 } 682 }
629} 683}
630EXPORT_SYMBOL_GPL(unregister_hotplug_dock_device); 684EXPORT_SYMBOL_GPL(unregister_hotplug_dock_device);
@@ -953,7 +1007,6 @@ static int __init dock_add(acpi_handle handle)
953 mutex_init(&dock_station->hp_lock); 1007 mutex_init(&dock_station->hp_lock);
954 spin_lock_init(&dock_station->dd_lock); 1008 spin_lock_init(&dock_station->dd_lock);
955 INIT_LIST_HEAD(&dock_station->sibling); 1009 INIT_LIST_HEAD(&dock_station->sibling);
956 INIT_LIST_HEAD(&dock_station->hotplug_devices);
957 ATOMIC_INIT_NOTIFIER_HEAD(&dock_notifier_list); 1010 ATOMIC_INIT_NOTIFIER_HEAD(&dock_notifier_list);
958 INIT_LIST_HEAD(&dock_station->dependent_devices); 1011 INIT_LIST_HEAD(&dock_station->dependent_devices);
959 1012
diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
index 050b6f9abfd1..59df8575a48c 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -61,6 +61,7 @@ static DEFINE_MUTEX(bridge_mutex);
61static void handle_hotplug_event_bridge (acpi_handle, u32, void *); 61static void handle_hotplug_event_bridge (acpi_handle, u32, void *);
62static void acpiphp_sanitize_bus(struct pci_bus *bus); 62static void acpiphp_sanitize_bus(struct pci_bus *bus);
63static void acpiphp_set_hpp_values(struct pci_bus *bus); 63static void acpiphp_set_hpp_values(struct pci_bus *bus);
64static void hotplug_event_func(acpi_handle handle, u32 type, void *context);
64static void handle_hotplug_event_func(acpi_handle handle, u32 type, void *context); 65static void handle_hotplug_event_func(acpi_handle handle, u32 type, void *context);
65static void free_bridge(struct kref *kref); 66static void free_bridge(struct kref *kref);
66 67
@@ -147,7 +148,7 @@ static int post_dock_fixups(struct notifier_block *nb, unsigned long val,
147 148
148 149
149static const struct acpi_dock_ops acpiphp_dock_ops = { 150static const struct acpi_dock_ops acpiphp_dock_ops = {
150 .handler = handle_hotplug_event_func, 151 .handler = hotplug_event_func,
151}; 152};
152 153
153/* Check whether the PCI device is managed by native PCIe hotplug driver */ 154/* Check whether the PCI device is managed by native PCIe hotplug driver */
@@ -179,6 +180,20 @@ static bool device_is_managed_by_native_pciehp(struct pci_dev *pdev)
179 return true; 180 return true;
180} 181}
181 182
183static void acpiphp_dock_init(void *data)
184{
185 struct acpiphp_func *func = data;
186
187 get_bridge(func->slot->bridge);
188}
189
190static void acpiphp_dock_release(void *data)
191{
192 struct acpiphp_func *func = data;
193
194 put_bridge(func->slot->bridge);
195}
196
182/* callback routine to register each ACPI PCI slot object */ 197/* callback routine to register each ACPI PCI slot object */
183static acpi_status 198static acpi_status
184register_slot(acpi_handle handle, u32 lvl, void *context, void **rv) 199register_slot(acpi_handle handle, u32 lvl, void *context, void **rv)
@@ -298,7 +313,8 @@ register_slot(acpi_handle handle, u32 lvl, void *context, void **rv)
298 */ 313 */
299 newfunc->flags &= ~FUNC_HAS_EJ0; 314 newfunc->flags &= ~FUNC_HAS_EJ0;
300 if (register_hotplug_dock_device(handle, 315 if (register_hotplug_dock_device(handle,
301 &acpiphp_dock_ops, newfunc)) 316 &acpiphp_dock_ops, newfunc,
317 acpiphp_dock_init, acpiphp_dock_release))
302 dbg("failed to register dock device\n"); 318 dbg("failed to register dock device\n");
303 319
304 /* we need to be notified when dock events happen 320 /* we need to be notified when dock events happen
@@ -1068,22 +1084,12 @@ static void handle_hotplug_event_bridge(acpi_handle handle, u32 type,
1068 alloc_acpi_hp_work(handle, type, context, _handle_hotplug_event_bridge); 1084 alloc_acpi_hp_work(handle, type, context, _handle_hotplug_event_bridge);
1069} 1085}
1070 1086
1071static void _handle_hotplug_event_func(struct work_struct *work) 1087static void hotplug_event_func(acpi_handle handle, u32 type, void *context)
1072{ 1088{
1073 struct acpiphp_func *func; 1089 struct acpiphp_func *func = context;
1074 char objname[64]; 1090 char objname[64];
1075 struct acpi_buffer buffer = { .length = sizeof(objname), 1091 struct acpi_buffer buffer = { .length = sizeof(objname),
1076 .pointer = objname }; 1092 .pointer = objname };
1077 struct acpi_hp_work *hp_work;
1078 acpi_handle handle;
1079 u32 type;
1080
1081 hp_work = container_of(work, struct acpi_hp_work, work);
1082 handle = hp_work->handle;
1083 type = hp_work->type;
1084 func = (struct acpiphp_func *)hp_work->context;
1085
1086 acpi_scan_lock_acquire();
1087 1093
1088 acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer); 1094 acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
1089 1095
@@ -1116,6 +1122,18 @@ static void _handle_hotplug_event_func(struct work_struct *work)
1116 warn("notify_handler: unknown event type 0x%x for %s\n", type, objname); 1122 warn("notify_handler: unknown event type 0x%x for %s\n", type, objname);
1117 break; 1123 break;
1118 } 1124 }
1125}
1126
1127static void _handle_hotplug_event_func(struct work_struct *work)
1128{
1129 struct acpi_hp_work *hp_work;
1130 struct acpiphp_func *func;
1131
1132 hp_work = container_of(work, struct acpi_hp_work, work);
1133 func = hp_work->context;
1134 acpi_scan_lock_acquire();
1135
1136 hotplug_event_func(hp_work->handle, hp_work->type, func);
1119 1137
1120 acpi_scan_lock_release(); 1138 acpi_scan_lock_release();
1121 kfree(hp_work); /* allocated in handle_hotplug_event_func */ 1139 kfree(hp_work); /* allocated in handle_hotplug_event_func */
diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
index e6168a24b9f0..b420939f5eb5 100644
--- a/include/acpi/acpi_drivers.h
+++ b/include/acpi/acpi_drivers.h
@@ -123,7 +123,9 @@ extern int register_dock_notifier(struct notifier_block *nb);
123extern void unregister_dock_notifier(struct notifier_block *nb); 123extern void unregister_dock_notifier(struct notifier_block *nb);
124extern int register_hotplug_dock_device(acpi_handle handle, 124extern int register_hotplug_dock_device(acpi_handle handle,
125 const struct acpi_dock_ops *ops, 125 const struct acpi_dock_ops *ops,
126 void *context); 126 void *context,
127 void (*init)(void *),
128 void (*release)(void *));
127extern void unregister_hotplug_dock_device(acpi_handle handle); 129extern void unregister_hotplug_dock_device(acpi_handle handle);
128#else 130#else
129static inline int is_dock_device(acpi_handle handle) 131static inline int is_dock_device(acpi_handle handle)
@@ -139,7 +141,9 @@ static inline void unregister_dock_notifier(struct notifier_block *nb)
139} 141}
140static inline int register_hotplug_dock_device(acpi_handle handle, 142static inline int register_hotplug_dock_device(acpi_handle handle,
141 const struct acpi_dock_ops *ops, 143 const struct acpi_dock_ops *ops,
142 void *context) 144 void *context,
145 void (*init)(void *),
146 void (*release)(void *))
143{ 147{
144 return -ENODEV; 148 return -ENODEV;
145} 149}