| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 20 Jun 2013 21:06:30 +0200
From: "Rafael J. Wysocki" <rjw@...k.pl>
To: "Alexander E. Patrakov" <patrakov@...il.com>
Cc: Jiang Liu <jiang.liu@...wei.com>, alexdeucher@...il.com,
Bjorn Helgaas <bhelgaas@...gle.com>,
Yinghai Lu <yinghai@...nel.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Yijing Wang <wangyijing@...wei.com>,
linux-acpi@...r.kernel.org, Jiang Liu <liuj97@...il.com>,
"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [BUGFIX v2 0/4] fix bug 56531, 59501 and 59581
On Wednesday, June 19, 2013 11:18:41 AM Alexander E. Patrakov wrote:
> 2013/6/19 Rafael J. Wysocki <rjw@...k.pl>:
> > OK, let's try to untangle this a bit.
> >
> > If you applyt patches [1/4] and [4/4] from the $subject series only, what
> > does remain unfixed?
>
> [not tested, can do so in 12 hours if needed]
>
> I think there will be problems on undocking and/or on the second
> docking, as described in comments #6 - #8 of
> https://bugzilla.kernel.org/show_bug.cgi?id=59501
OK, I think I have something that might work. It may not solve all problems,
but maybe it helps a bit. Unfortunately, I can't really test it, so please do
if you can.
Please apply [1/4] and [4/4] and the one below and see what happens.
Thanks,
Rafael
---
Rationale:
acpiphp_glue.c:disable_device() trims the underlying ACPI device objects
after removing the companion PCI devices, so the dock station code
doesn't need to trim them separately for the dependent devices handled
by acpiphp.
Moreover, acpiphp_glue.c is the only user of
[un]register_hotplug_dock_device(), so *all* devices on the
ds->hotplug_devices list are handled by acpiphp and ops is set for all
of them.
This means that (1) the ds->hotplug_devices list is not necessary (we
can always walk ds->dependent_devices instead and look for those that
have dd->ops set) and (2) we don't need to call
dock_remove_acpi_device(dd->handle) on eject for any of those devices,
because dd->ops->handler() is going to take care of the ACPI device
objects trimming for them anyway.
Taking the above into account make the following changes:
(1) Drop hotplug_devices from struct dock_station.
(2) Drop dock_{add|del}_hotplug_device()
(3) Make [un]register_hotplug_dock_device() [un]set 'ops' and
'context' for the given device under ds->hp_lock.
(4) Add hot_remove_dock_devices() that walks ds->dependent_devices and
either calls dd->ops->handler(), if present, or trims the underlying
ACPI device object, otherwise.
(5) Replace hotplug_dock_devices(ds, ACPI_NOTIFY_EJECT_REQUEST) calls
with hot_remove_dock_devices(ds).
(6) Rename hotplug_dock_devices() to hot_add_dock_devices() and make
it only handle bus check and device check requests. Make it walk
ds->dependent_devices instead of ds->hotplug devices.
(7) Make dock_event() walk ds->dependent_devices (instead of
ds->hotplug devices) under ds->hp_lock.
---
drivers/acpi/dock.c | 111 ++++++++++++++++++++++++----------------------------
1 file changed, 53 insertions(+), 58 deletions(-)
Index: linux-pm/drivers/acpi/dock.c
===================================================================
--- linux-pm.orig/drivers/acpi/dock.c
+++ linux-pm/drivers/acpi/dock.c
@@ -66,7 +66,6 @@ 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;
@@ -121,38 +120,6 @@ add_dock_dependent_device(struct dock_st
}
/**
- * 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)
-{
- mutex_lock(&ds->hp_lock);
- list_add_tail(&dd->hotplug_list, &ds->hotplug_devices);
- mutex_unlock(&ds->hp_lock);
-}
-
-/**
- * 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)
-{
- mutex_lock(&ds->hp_lock);
- list_del(&dd->hotplug_list);
- mutex_unlock(&ds->hp_lock);
-}
-
-/**
* find_dock_dependent_device - get a device dependent on this dock
* @ds: the dock station
* @handle: the acpi_handle of the device we want
@@ -342,40 +309,60 @@ static void dock_remove_acpi_device(acpi
}
/**
- * hotplug_dock_devices - insert or remove devices on the dock station
- * @ds: the dock station
- * @event: either bus check or eject request
+ * hot_remove_dock_devices - Remove devices on a dock station.
+ * @ds: Dock station to remove devices for.
+ *
+ * For each device depending on @ds, if a dock event handler is registered,
+ * call it for the device, or trim the underlying ACPI device object otherwise.
+ *
+ * Dock event handlers are responsible for trimming the underlying ACPI device
+ * objects if present.
+ */
+static void hot_remove_dock_devices(struct dock_station *ds)
+{
+ struct dock_dependent_device *dd;
+
+ mutex_lock(&ds->hp_lock);
+
+ list_for_each_entry(dd, &ds->dependent_devices, list) {
+ if (dd->ops && dd->ops->handler)
+ dd->ops->handler(dd->handle, ACPI_NOTIFY_EJECT_REQUEST,
+ dd->context);
+ else
+ dock_remove_acpi_device(dd->handle);
+ }
+
+ mutex_unlock(&ds->hp_lock);
+}
+
+/**
+ * hot_add_dock_devices - Insert devices on a dock station.
+ * @ds: Dock station to insert devices for.
*
* Some devices on the dock station need to have drivers called
* to perform hotplug operations after a dock event has occurred.
* Traverse the list of dock devices that have registered a
* hotplug handler, and call the handler.
*/
-static void hotplug_dock_devices(struct dock_station *ds, u32 event)
+static void hot_add_dock_devices(struct dock_station *ds, u32 event)
{
struct dock_dependent_device *dd;
mutex_lock(&ds->hp_lock);
- /*
- * First call driver specific hotplug functions
- */
- list_for_each_entry(dd, &ds->hotplug_devices, hotplug_list)
+ /* First call driver specific hotplug event handlers. */
+ list_for_each_entry(dd, &ds->dependent_devices, list)
if (dd->ops && dd->ops->handler)
dd->ops->handler(dd->handle, event, dd->context);
/*
- * Now make sure that an acpi_device is created for each
- * dependent device, or removed if this is an eject request.
- * This will cause acpi_drivers to be stopped/started if they
- * exist
+ * Now make sure that an acpi_device is created for each dependent
+ * device. That will cause scan handlers to attach to devices objects
+ * or acpi_drivers to be started if they exist.
*/
- list_for_each_entry(dd, &ds->dependent_devices, list) {
- if (event == ACPI_NOTIFY_EJECT_REQUEST)
- dock_remove_acpi_device(dd->handle);
- else
- dock_create_acpi_device(dd->handle);
- }
+ list_for_each_entry(dd, &ds->dependent_devices, list)
+ dock_create_acpi_device(dd->handle);
+
mutex_unlock(&ds->hp_lock);
}
@@ -398,10 +385,14 @@ static void dock_event(struct dock_stati
if (num == DOCK_EVENT)
kobject_uevent_env(&dev->kobj, KOBJ_CHANGE, envp);
- list_for_each_entry(dd, &ds->hotplug_devices, hotplug_list)
+ mutex_lock(&ds->hp_lock);
+
+ list_for_each_entry(dd, &ds->dependent_devices, list)
if (dd->ops && dd->ops->uevent)
dd->ops->uevent(dd->handle, event, dd->context);
+ mutex_unlock(&ds->hp_lock);
+
if (num != DOCK_EVENT)
kobject_uevent_env(&dev->kobj, KOBJ_CHANGE, envp);
}
@@ -598,9 +589,10 @@ register_hotplug_dock_device(acpi_handle
*/
dd = find_dock_dependent_device(dock_station, handle);
if (dd) {
+ mutex_lock(&dock_station->hp_lock);
dd->ops = ops;
dd->context = context;
- dock_add_hotplug_device(dock_station, dd);
+ mutex_unlock(&dock_station->hp_lock);
ret = 0;
}
}
@@ -623,8 +615,12 @@ void unregister_hotplug_dock_device(acpi
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);
+ if (dd) {
+ mutex_lock(&dock_station->hp_lock);
+ dd->ops = NULL;
+ dd->context = NULL;
+ mutex_unlock(&dock_station->hp_lock);
+ }
}
}
EXPORT_SYMBOL_GPL(unregister_hotplug_dock_device);
@@ -649,7 +645,7 @@ static int handle_eject_request(struct d
*/
dock_event(ds, event, UNDOCK_EVENT);
- hotplug_dock_devices(ds, ACPI_NOTIFY_EJECT_REQUEST);
+ hot_remove_dock_devices(ds);
undock(ds);
dock_lock(ds, 0);
eject_dock(ds);
@@ -708,7 +704,7 @@ static void dock_notify(acpi_handle hand
}
atomic_notifier_call_chain(&dock_notifier_list,
event, NULL);
- hotplug_dock_devices(ds, event);
+ hot_add_dock_devices(ds, event);
complete_dock(ds);
dock_event(ds, event, DOCK_EVENT);
dock_lock(ds, 1);
@@ -953,7 +949,6 @@ static int __init dock_add(acpi_handle h
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);
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists