[PATCH 0/3] ACPI scan and hotplug fixes for 3.14

[PATCH 0/3] ACPI scan and hotplug fixes for 3.14

[Rafael J. Wysocki]

Hi,

The following three patches fix some issues that we have in the common ACPI
hotplug infrastructure.

If anyone sees any problems with them, please let me know.

Thanks!

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

[PATCH 1/3] ACPI / scan: Start matching drivers after trying scan handlers

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

ACPI scan handlers should always be attached to struct acpi_device
objects before any ACPI drivers, but there is a window during which
a driver may be attached to a struct acpi_device before checking if
there is a matching scan handler.  Namely, that will happen if an
ACPI driver module is loaded during acpi_bus_scan() right after
the first namespace walk is complete and before the given device
is processed by the second namespace walk.

To prevent that from happening, set the match_driver flags of
struct acpi_device objects right before running device_attach()
for them in acpi_bus_device_attach().

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/scan.c |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -1676,7 +1676,6 @@ void acpi_init_device_object(struct acpi
 
 void acpi_device_add_finalize(struct acpi_device *device)
 {
-	device->flags.match_driver = true;
 	dev_set_uevent_suppress(&device->dev, false);
 	kobject_uevent(&device->dev.kobj, KOBJ_ADD);
 }
@@ -1915,8 +1914,12 @@ static acpi_status acpi_bus_device_attac
 		return AE_OK;
 
 	ret = acpi_scan_attach_handler(device);
-	if (ret)
-		return ret > 0 ? AE_OK : AE_CTRL_DEPTH;
+	if (ret < 0)
+		return AE_CTRL_DEPTH;
+
+	device->flags.match_driver = true;
+	if (ret > 0)
+		return AE_OK;
 
 	ret = device_attach(&device->dev);
 	return ret >= 0 ? AE_OK : AE_CTRL_DEPTH;

[PATCH 2/3] ACPI / hotplug: Refuse to hot-remove all objects with disabled hotplug

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

In theory, an ACPI device object may be the parent of another
device object whose hotplug is disabled by user space thorugh its
scan handler.  In that case, the eject operation targeting the
parent should fail as though the parent's own hotplug was disabled,
but currently this is not the case, because acpi_scan_hot_remove()
doesn't check the disable/enable hotplug status of the children
of the top-most object passed to it.

To fix this, modify acpi_bus_offline_companions() to return an
error code if hotplug is disabled for the given device object.
[Also change the name of the function to acpi_bus_offline(),
because it is not only about companions any more, and change
the name of acpi_bus_online_companions() accordingly.]  Make
acpi_scan_hot_remove() propagate that error to its callers.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/scan.c |   57 ++++++++++++++++++++++++++++++++--------------------
 1 file changed, 36 insertions(+), 21 deletions(-)

Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -125,8 +125,8 @@ acpi_device_modalias_show(struct device
 }
 static DEVICE_ATTR(modalias, 0444, acpi_device_modalias_show, NULL);
 
-static acpi_status acpi_bus_offline_companions(acpi_handle handle, u32 lvl,
-					       void *data, void **ret_p)
+static acpi_status acpi_bus_offline(acpi_handle handle, u32 lvl, void *data,
+				    void **ret_p)
 {
 	struct acpi_device *device = NULL;
 	struct acpi_device_physical_node *pn;
@@ -136,6 +136,11 @@ static acpi_status acpi_bus_offline_comp
 	if (acpi_bus_get_device(handle, &device))
 		return AE_OK;
 
+	if (device->handler && !device->handler->hotplug.enabled) {
+		*ret_p = &device->dev;
+		return AE_SUPPORT;
+	}
+
 	mutex_lock(&device->physical_node_lock);
 
 	list_for_each_entry(pn, &device->physical_node_list, node) {
@@ -168,8 +173,8 @@ static acpi_status acpi_bus_offline_comp
 	return status;
 }
 
-static acpi_status acpi_bus_online_companions(acpi_handle handle, u32 lvl,
-					      void *data, void **ret_p)
+static acpi_status acpi_bus_online(acpi_handle handle, u32 lvl, void *data,
+				   void **ret_p)
 {
 	struct acpi_device *device = NULL;
 	struct acpi_device_physical_node *pn;
@@ -214,26 +219,32 @@ static int acpi_scan_hot_remove(struct a
 	 * If the first pass is successful, the second one isn't needed, though.
 	 */
 	errdev = NULL;
-	acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX,
-			    NULL, acpi_bus_offline_companions,
-			    (void *)false, (void **)&errdev);
-	acpi_bus_offline_companions(handle, 0, (void *)false, (void **)&errdev);
+	status = acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX,
+				     NULL, acpi_bus_offline, (void *)false,
+				     (void **)&errdev);
+	if (status == AE_SUPPORT) {
+		dev_warn(errdev, "Offline disabled.\n");
+		acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX,
+				    acpi_bus_online, NULL, NULL, NULL);
+		put_device(&device->dev);
+		return -EPERM;
+	}
+	acpi_bus_offline(handle, 0, (void *)false, (void **)&errdev);
 	if (errdev) {
 		errdev = NULL;
 		acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX,
-				    NULL, acpi_bus_offline_companions,
-				    (void *)true , (void **)&errdev);
+				    NULL, acpi_bus_offline, (void *)true,
+				    (void **)&errdev);
 		if (!errdev || acpi_force_hot_remove)
-			acpi_bus_offline_companions(handle, 0, (void *)true,
-						    (void **)&errdev);
+			acpi_bus_offline(handle, 0, (void *)true,
+					 (void **)&errdev);
 
 		if (errdev && !acpi_force_hot_remove) {
 			dev_warn(errdev, "Offline failed.\n");
-			acpi_bus_online_companions(handle, 0, NULL, NULL);
+			acpi_bus_online(handle, 0, NULL, NULL);
 			acpi_walk_namespace(ACPI_TYPE_ANY, handle,
-					    ACPI_UINT32_MAX,
-					    acpi_bus_online_companions, NULL,
-					    NULL, NULL);
+					    ACPI_UINT32_MAX, acpi_bus_online,
+					    NULL, NULL, NULL);
 			put_device(&device->dev);
 			return -EBUSY;
 		}
@@ -290,10 +301,9 @@ static void acpi_bus_device_eject(void *
 		goto err_out;
 
 	handler = device->handler;
-	if (!handler || !handler->hotplug.enabled) {
-		ost_code = ACPI_OST_SC_EJECT_NOT_SUPPORTED;
-		goto err_out;
-	}
+	if (!handler || !handler->hotplug.enabled)
+		goto err_support;
+
 	acpi_evaluate_hotplug_ost(handle, ACPI_NOTIFY_EJECT_REQUEST,
 				  ACPI_OST_SC_EJECT_IN_PROGRESS, NULL);
 	if (handler->hotplug.mode == AHM_CONTAINER)
@@ -301,14 +311,19 @@ static void acpi_bus_device_eject(void *
 
 	get_device(&device->dev);
 	error = acpi_scan_hot_remove(device);
-	if (error)
+	if (error == -EPERM) {
+		goto err_support;
+	} else if (error) {
 		goto err_out;
+	}
 
  out:
 	mutex_unlock(&acpi_scan_lock);
 	unlock_device_hotplug();
 	return;
 
+ err_support:
+	ost_code = ACPI_OST_SC_EJECT_NOT_SUPPORTED;
  err_out:
 	acpi_evaluate_hotplug_ost(handle, ACPI_NOTIFY_EJECT_REQUEST, ost_code,
 				  NULL);

[PATCH 3/3] ACPI / hotplug: Merge device hot-removal routines

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

The only substantial difference between acpi_bus_device_eject() and
acpi_bus_hot_remove_device() is the get_device() done by the former
which is supposed to be done by callers of the latter.  However,
at least one caller of acpi_bus_hot_remove_device(), which is
handle_root_bridge_removal(), doesn't do that and generally it
won't be necessary to do that at all if ACPI handles, rather than
struct acpi_device objects, are passed between ACPI hotplug routines,
because the correctness of those routines already depends on the
persistence of ACPI handles (that may not be an entirely correct
assumption in theory, but in practice it has been good enough for a
long time).

For this reason, modify acpi_bus_device_eject() to take two
arguments, an ACPI handle and event code, and make
acpi_bus_hot_remove_device() call it internally.  Moreover, rework
acpi_bus_hot_remove_device() so that it only takes one argument,
an ACPI handle, and make acpi_scan_bus_device_check() queue up
acpi_bus_hot_remove_device() instead of acpi_bus_device_eject()
itself.  After these changes, handle_root_bridge_removal() may
be replaced with async execution of acpi_bus_hot_remove_device()
with the ACPI handle of the root passed to it as the argument.

However, since acpi_eject_store() also needs to execute
acpi_bus_device_eject() asynchronously and it needs to pass a
special event code, ACPI_OST_EC_OSPM_EJECT, to that function,
introduce a separate wrapper around acpi_bus_device_eject(),
acpi_eject_store_work(), that will be queued up by
acpi_eject_store() instead of acpi_bus_hot_remove_device().
This allows acpi_eject_store() to be simplified and it allows
struct acpi_eject_event to be dropped entirely, as there's
no more users of that structure.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/internal.h |    1 
 drivers/acpi/pci_root.c |   28 ++--------------
 drivers/acpi/scan.c     |   81 ++++++++++++++++--------------------------------
 include/acpi/acpi_bus.h |    6 ---
 4 files changed, 32 insertions(+), 84 deletions(-)

Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -285,9 +285,8 @@ static int acpi_scan_hot_remove(struct a
 	return 0;
 }
 
-static void acpi_bus_device_eject(void *context)
+static void acpi_bus_device_eject(acpi_handle handle, u32 ost_source)
 {
-	acpi_handle handle = context;
 	struct acpi_device *device = NULL;
 	struct acpi_scan_handler *handler;
 	u32 ost_code = ACPI_OST_SC_NON_SPECIFIC_FAILURE;
@@ -304,8 +303,10 @@ static void acpi_bus_device_eject(void *
 	if (!handler || !handler->hotplug.enabled)
 		goto err_support;
 
-	acpi_evaluate_hotplug_ost(handle, ACPI_NOTIFY_EJECT_REQUEST,
-				  ACPI_OST_SC_EJECT_IN_PROGRESS, NULL);
+	if (ost_source == ACPI_NOTIFY_EJECT_REQUEST)
+		acpi_evaluate_hotplug_ost(handle, ACPI_NOTIFY_EJECT_REQUEST,
+					  ACPI_OST_SC_EJECT_IN_PROGRESS, NULL);
+
 	if (handler->hotplug.mode == AHM_CONTAINER)
 		kobject_uevent(&device->dev.kobj, KOBJ_OFFLINE);
 
@@ -325,11 +326,22 @@ static void acpi_bus_device_eject(void *
  err_support:
 	ost_code = ACPI_OST_SC_EJECT_NOT_SUPPORTED;
  err_out:
-	acpi_evaluate_hotplug_ost(handle, ACPI_NOTIFY_EJECT_REQUEST, ost_code,
-				  NULL);
+	acpi_evaluate_hotplug_ost(handle, ost_source, ost_code, NULL);
 	goto out;
 }
 
+/**
+ * acpi_bus_hot_remove_device: Eject a device and its children.
+ * @context: ACPI handle of the device to eject.
+ *
+ * Hot-remove a device and its children. This function is to be called
+ * asynchronously through acpi_os_hotplug_execute().
+ */
+void acpi_bus_hot_remove_device(void *context)
+{
+	acpi_bus_device_eject(context, ACPI_NOTIFY_EJECT_REQUEST);
+}
+
 static void acpi_scan_bus_device_check(acpi_handle handle, u32 ost_source)
 {
 	struct acpi_device *device = NULL;
@@ -428,7 +440,7 @@ static void acpi_hotplug_notify_cb(acpi_
 		break;
 	case ACPI_NOTIFY_EJECT_REQUEST:
 		acpi_handle_debug(handle, "ACPI_NOTIFY_EJECT_REQUEST event\n");
-		callback = acpi_bus_device_eject;
+		callback = acpi_bus_hot_remove_device;
 		break;
 	default:
 		/* non-hotplug event; possibly handled by other handler */
@@ -441,36 +453,6 @@ static void acpi_hotplug_notify_cb(acpi_
 					  NULL);
 }
 
-/**
- * acpi_bus_hot_remove_device: hot-remove a device and its children
- * @context: struct acpi_eject_event pointer (freed in this func)
- *
- * Hot-remove a device and its children. This function frees up the
- * memory space passed by arg context, so that the caller may call
- * this function asynchronously through acpi_os_hotplug_execute().
- */
-void acpi_bus_hot_remove_device(void *context)
-{
-	struct acpi_eject_event *ej_event = context;
-	struct acpi_device *device = ej_event->device;
-	acpi_handle handle = device->handle;
-	int error;
-
-	lock_device_hotplug();
-	mutex_lock(&acpi_scan_lock);
-
-	error = acpi_scan_hot_remove(device);
-	if (error && handle)
-		acpi_evaluate_hotplug_ost(handle, ej_event->event,
-					  ACPI_OST_SC_NON_SPECIFIC_FAILURE,
-					  NULL);
-
-	mutex_unlock(&acpi_scan_lock);
-	unlock_device_hotplug();
-	kfree(context);
-}
-EXPORT_SYMBOL(acpi_bus_hot_remove_device);
-
 static ssize_t real_power_state_show(struct device *dev,
 				     struct device_attribute *attr, char *buf)
 {
@@ -497,15 +479,18 @@ static ssize_t power_state_show(struct d
 
 static DEVICE_ATTR(power_state, 0444, power_state_show, NULL);
 
+static void acpi_eject_store_work(void *context)
+{
+	acpi_bus_device_eject(context, ACPI_OST_EC_OSPM_EJECT);
+}
+
 static ssize_t
 acpi_eject_store(struct device *d, struct device_attribute *attr,
 		const char *buf, size_t count)
 {
 	struct acpi_device *acpi_device = to_acpi_device(d);
-	struct acpi_eject_event *ej_event;
 	acpi_object_type not_used;
 	acpi_status status;
-	int ret;
 
 	if (!count || buf[0] != '1')
 		return -EINVAL;
@@ -518,28 +503,16 @@ acpi_eject_store(struct device *d, struc
 	if (ACPI_FAILURE(status) || !acpi_device->flags.ejectable)
 		return -ENODEV;
 
-	ej_event = kmalloc(sizeof(*ej_event), GFP_KERNEL);
-	if (!ej_event) {
-		ret = -ENOMEM;
-		goto err_out;
-	}
 	acpi_evaluate_hotplug_ost(acpi_device->handle, ACPI_OST_EC_OSPM_EJECT,
 				  ACPI_OST_SC_EJECT_IN_PROGRESS, NULL);
-	ej_event->device = acpi_device;
-	ej_event->event = ACPI_OST_EC_OSPM_EJECT;
-	get_device(&acpi_device->dev);
-	status = acpi_os_hotplug_execute(acpi_bus_hot_remove_device, ej_event);
+	status = acpi_os_hotplug_execute(acpi_eject_store_work,
+					 acpi_device->handle);
 	if (ACPI_SUCCESS(status))
 		return count;
 
-	put_device(&acpi_device->dev);
-	kfree(ej_event);
-	ret = status == AE_NO_MEMORY ? -ENOMEM : -EAGAIN;
-
- err_out:
 	acpi_evaluate_hotplug_ost(acpi_device->handle, ACPI_OST_EC_OSPM_EJECT,
 				  ACPI_OST_SC_NON_SPECIFIC_FAILURE, NULL);
-	return ret;
+	return -EAGAIN;
 }
 
 static DEVICE_ATTR(eject, 0200, NULL, acpi_eject_store);
Index: linux-pm/drivers/acpi/internal.h
===================================================================
--- linux-pm.orig/drivers/acpi/internal.h
+++ linux-pm/drivers/acpi/internal.h
@@ -92,6 +92,7 @@ void acpi_device_add_finalize(struct acp
 void acpi_free_pnp_ids(struct acpi_device_pnp *pnp);
 int acpi_bind_one(struct device *dev, acpi_handle handle);
 int acpi_unbind_one(struct device *dev);
+void acpi_bus_hot_remove_device(void *context);
 
 /* --------------------------------------------------------------------------
                                   Power Resource
Index: linux-pm/drivers/acpi/pci_root.c
===================================================================
--- linux-pm.orig/drivers/acpi/pci_root.c
+++ linux-pm/drivers/acpi/pci_root.c
@@ -39,6 +39,8 @@
 #include <acpi/acpi_drivers.h>
 #include <acpi/apei.h>
 
+#include "internal.h"
+
 #define PREFIX "ACPI: "
 
 #define _COMPONENT		ACPI_PCI_COMPONENT
@@ -590,29 +592,6 @@ static void handle_root_bridge_insertion
 		acpi_handle_err(handle, "cannot add bridge to acpi list\n");
 }
 
-static void handle_root_bridge_removal(struct acpi_device *device)
-{
-	acpi_status status;
-	struct acpi_eject_event *ej_event;
-
-	ej_event = kmalloc(sizeof(*ej_event), GFP_KERNEL);
-	if (!ej_event) {
-		/* Inform firmware the hot-remove operation has error */
-		(void) acpi_evaluate_hotplug_ost(device->handle,
-					ACPI_NOTIFY_EJECT_REQUEST,
-					ACPI_OST_SC_NON_SPECIFIC_FAILURE,
-					NULL);
-		return;
-	}
-
-	ej_event->device = device;
-	ej_event->event = ACPI_NOTIFY_EJECT_REQUEST;
-
-	status = acpi_os_hotplug_execute(acpi_bus_hot_remove_device, ej_event);
-	if (ACPI_FAILURE(status))
-		kfree(ej_event);
-}
-
 static void _handle_hotplug_event_root(struct work_struct *work)
 {
 	struct acpi_pci_root *root;
@@ -653,7 +632,8 @@ static void _handle_hotplug_event_root(s
 		acpi_handle_printk(KERN_DEBUG, handle,
 				   "Device eject notify on %s\n", __func__);
 		if (root)
-			handle_root_bridge_removal(root->device);
+			acpi_os_hotplug_execute(acpi_bus_hot_remove_device,
+						handle);
 		break;
 	default:
 		acpi_handle_warn(handle,
Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -339,11 +339,6 @@ struct acpi_bus_event {
 	u32 data;
 };
 
-struct acpi_eject_event {
-	struct acpi_device	*device;
-	u32		event;
-};
-
 struct acpi_hp_work {
 	struct work_struct work;
 	acpi_handle handle;
@@ -391,7 +386,6 @@ int acpi_scan_add_handler(struct acpi_sc
 int acpi_bus_register_driver(struct acpi_driver *driver);
 void acpi_bus_unregister_driver(struct acpi_driver *driver);
 int acpi_bus_scan(acpi_handle handle);
-void acpi_bus_hot_remove_device(void *context);
 void acpi_bus_trim(struct acpi_device *start);
 acpi_status acpi_bus_get_ejd(acpi_handle handle, acpi_handle * ejd);
 int acpi_match_device_ids(struct acpi_device *device,

[PATCH on top of 3/3] ACPI / hotplug: Remove unnecessary get_device() and put_device()

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

The get_device() and put_device() calls made by acpi_bus_device_eject()
and acpi_scan_hot_remove(), respectively, are not necessary any more,
so remove them.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

Actually, after https://patchwork.kernel.org/patch/3134091/ the
get_device() and put_device() calls may be dropped just fine.

Thanks,
Rafael

---
 drivers/acpi/scan.c |    5 -----
 1 file changed, 5 deletions(-)

Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -205,7 +205,6 @@ static int acpi_scan_hot_remove(struct a
 	/* If there is no handle, the device node has been unregistered. */
 	if (!handle) {
 		dev_dbg(&device->dev, "ACPI handle missing\n");
-		put_device(&device->dev);
 		return -EINVAL;
 	}
 
@@ -226,7 +225,6 @@ static int acpi_scan_hot_remove(struct a
 		dev_warn(errdev, "Offline disabled.\n");
 		acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX,
 				    acpi_bus_online, NULL, NULL, NULL);
-		put_device(&device->dev);
 		return -EPERM;
 	}
 	acpi_bus_offline(handle, 0, (void *)false, (void **)&errdev);
@@ -245,7 +243,6 @@ static int acpi_scan_hot_remove(struct a
 			acpi_walk_namespace(ACPI_TYPE_ANY, handle,
 					    ACPI_UINT32_MAX, acpi_bus_online,
 					    NULL, NULL, NULL);
-			put_device(&device->dev);
 			return -EBUSY;
 		}
 	}
@@ -256,7 +253,6 @@ static int acpi_scan_hot_remove(struct a
 	acpi_bus_trim(device);
 
 	/* Device node has been unregistered. */
-	put_device(&device->dev);
 	device = NULL;
 
 	acpi_evaluate_lck(handle, 0);
@@ -310,7 +306,6 @@ static void acpi_bus_device_eject(acpi_h
 	if (handler->hotplug.mode == AHM_CONTAINER)
 		kobject_uevent(&device->dev.kobj, KOBJ_OFFLINE);
 
-	get_device(&device->dev);
 	error = acpi_scan_hot_remove(device);
 	if (error == -EPERM) {
 		goto err_support;

Re: [PATCH 3/3] ACPI / hotplug: Merge device hot-removal routines

[Rafael J. Wysocki]

On Monday, November 04, 2013 01:21:46 AM Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> The only substantial difference between acpi_bus_device_eject() and
> acpi_bus_hot_remove_device() is the get_device() done by the former
> which is supposed to be done by callers of the latter.  However,
> at least one caller of acpi_bus_hot_remove_device(), which is
> handle_root_bridge_removal(), doesn't do that and generally it
> won't be necessary to do that at all if ACPI handles, rather than
> struct acpi_device objects, are passed between ACPI hotplug routines,
> because the correctness of those routines already depends on the
> persistence of ACPI handles (that may not be an entirely correct
> assumption in theory, but in practice it has been good enough for a
> long time).
> 
> For this reason, modify acpi_bus_device_eject() to take two
> arguments, an ACPI handle and event code, and make
> acpi_bus_hot_remove_device() call it internally.  Moreover, rework
> acpi_bus_hot_remove_device() so that it only takes one argument,
> an ACPI handle, and make acpi_scan_bus_device_check() queue up
> acpi_bus_hot_remove_device() instead of acpi_bus_device_eject()
> itself.  After these changes, handle_root_bridge_removal() may
> be replaced with async execution of acpi_bus_hot_remove_device()
> with the ACPI handle of the root passed to it as the argument.
> 
> However, since acpi_eject_store() also needs to execute
> acpi_bus_device_eject() asynchronously and it needs to pass a
> special event code, ACPI_OST_EC_OSPM_EJECT, to that function,
> introduce a separate wrapper around acpi_bus_device_eject(),
> acpi_eject_store_work(), that will be queued up by
> acpi_eject_store() instead of acpi_bus_hot_remove_device().
> This allows acpi_eject_store() to be simplified and it allows
> struct acpi_eject_event to be dropped entirely, as there's
> no more users of that structure.

Well, scratch this (and the follow-up), it didn't go in the right direction.

I'll send a new series shortly.

Rafael

[Update][PATCH 0/6] ACPI scan and hotplug fixes

[Rafael J. Wysocki]

On Monday, November 04, 2013 01:17:12 AM Rafael J. Wysocki wrote:
> Hi,
> 
> The following three patches fix some issues that we have in the common ACPI
> hotplug infrastructure.
> 
> If anyone sees any problems with them, please let me know.

Well, the last patch from the last version didn't really do the right thing in
my opinion, so new series follows.

I actually would like to squeeze it into 3.13 if there are no objections, since
at least patch [3/6] is kind of urgent.

Thanks!

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

[Update][PATCH 1/6] ACPI / scan: Start matching drivers after trying scan handlers

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

ACPI scan handlers should always be attached to struct acpi_device
objects before any ACPI drivers, but there is a window during which
a driver may be attached to a struct acpi_device before checking if
there is a matching scan handler.  Namely, that will happen if an
ACPI driver module is loaded during acpi_bus_scan() right after
the first namespace walk is complete and before the given device
is processed by the second namespace walk.

To prevent that from happening, set the match_driver flags of
struct acpi_device objects right before running device_attach()
for them in acpi_bus_device_attach().

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/scan.c |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -1676,7 +1676,6 @@ void acpi_init_device_object(struct acpi
 
 void acpi_device_add_finalize(struct acpi_device *device)
 {
-	device->flags.match_driver = true;
 	dev_set_uevent_suppress(&device->dev, false);
 	kobject_uevent(&device->dev.kobj, KOBJ_ADD);
 }
@@ -1915,8 +1914,12 @@ static acpi_status acpi_bus_device_attac
 		return AE_OK;
 
 	ret = acpi_scan_attach_handler(device);
-	if (ret)
-		return ret > 0 ? AE_OK : AE_CTRL_DEPTH;
+	if (ret < 0)
+		return AE_CTRL_DEPTH;
+
+	device->flags.match_driver = true;
+	if (ret > 0)
+		return AE_OK;
 
 	ret = device_attach(&device->dev);
 	return ret >= 0 ? AE_OK : AE_CTRL_DEPTH;

Re: [Update][PATCH 1/6] ACPI / scan: Start matching drivers after trying scan handlers

[Toshi Kani]

On Mon, 2013-11-04 at 14:30 +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> ACPI scan handlers should always be attached to struct acpi_device
> objects before any ACPI drivers, but there is a window during which
> a driver may be attached to a struct acpi_device before checking if
> there is a matching scan handler.  Namely, that will happen if an
> ACPI driver module is loaded during acpi_bus_scan() right after
> the first namespace walk is complete and before the given device
> is processed by the second namespace walk.
> 
> To prevent that from happening, set the match_driver flags of
> struct acpi_device objects right before running device_attach()
> for them in acpi_bus_device_attach().
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Acked-by: Toshi Kani <toshi.kani@hp.com>

Thanks,
-Toshi

[Update][PATCH 2/6] ACPI / hotplug: Refuse to hot-remove all objects with disabled hotplug

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

In theory, an ACPI device object may be the parent of another
device object whose hotplug is disabled by user space thorugh its
scan handler.  In that case, the eject operation targeting the
parent should fail as though the parent's own hotplug was disabled,
but currently this is not the case, because acpi_scan_hot_remove()
doesn't check the disable/enable hotplug status of the children
of the top-most object passed to it.

To fix this, modify acpi_bus_offline_companions() to return an
error code if hotplug is disabled for the given device object.
[Also change the name of the function to acpi_bus_offline(),
because it is not only about companions any more, and change
the name of acpi_bus_online_companions() accordingly.]  Make
acpi_scan_hot_remove() propagate that error to its callers.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/scan.c |   57 ++++++++++++++++++++++++++++++++--------------------
 1 file changed, 36 insertions(+), 21 deletions(-)

Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -125,8 +125,8 @@ acpi_device_modalias_show(struct device
 }
 static DEVICE_ATTR(modalias, 0444, acpi_device_modalias_show, NULL);
 
-static acpi_status acpi_bus_offline_companions(acpi_handle handle, u32 lvl,
-					       void *data, void **ret_p)
+static acpi_status acpi_bus_offline(acpi_handle handle, u32 lvl, void *data,
+				    void **ret_p)
 {
 	struct acpi_device *device = NULL;
 	struct acpi_device_physical_node *pn;
@@ -136,6 +136,11 @@ static acpi_status acpi_bus_offline_comp
 	if (acpi_bus_get_device(handle, &device))
 		return AE_OK;
 
+	if (device->handler && !device->handler->hotplug.enabled) {
+		*ret_p = &device->dev;
+		return AE_SUPPORT;
+	}
+
 	mutex_lock(&device->physical_node_lock);
 
 	list_for_each_entry(pn, &device->physical_node_list, node) {
@@ -168,8 +173,8 @@ static acpi_status acpi_bus_offline_comp
 	return status;
 }
 
-static acpi_status acpi_bus_online_companions(acpi_handle handle, u32 lvl,
-					      void *data, void **ret_p)
+static acpi_status acpi_bus_online(acpi_handle handle, u32 lvl, void *data,
+				   void **ret_p)
 {
 	struct acpi_device *device = NULL;
 	struct acpi_device_physical_node *pn;
@@ -214,26 +219,32 @@ static int acpi_scan_hot_remove(struct a
 	 * If the first pass is successful, the second one isn't needed, though.
 	 */
 	errdev = NULL;
-	acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX,
-			    NULL, acpi_bus_offline_companions,
-			    (void *)false, (void **)&errdev);
-	acpi_bus_offline_companions(handle, 0, (void *)false, (void **)&errdev);
+	status = acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX,
+				     NULL, acpi_bus_offline, (void *)false,
+				     (void **)&errdev);
+	if (status == AE_SUPPORT) {
+		dev_warn(errdev, "Offline disabled.\n");
+		acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX,
+				    acpi_bus_online, NULL, NULL, NULL);
+		put_device(&device->dev);
+		return -EPERM;
+	}
+	acpi_bus_offline(handle, 0, (void *)false, (void **)&errdev);
 	if (errdev) {
 		errdev = NULL;
 		acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX,
-				    NULL, acpi_bus_offline_companions,
-				    (void *)true , (void **)&errdev);
+				    NULL, acpi_bus_offline, (void *)true,
+				    (void **)&errdev);
 		if (!errdev || acpi_force_hot_remove)
-			acpi_bus_offline_companions(handle, 0, (void *)true,
-						    (void **)&errdev);
+			acpi_bus_offline(handle, 0, (void *)true,
+					 (void **)&errdev);
 
 		if (errdev && !acpi_force_hot_remove) {
 			dev_warn(errdev, "Offline failed.\n");
-			acpi_bus_online_companions(handle, 0, NULL, NULL);
+			acpi_bus_online(handle, 0, NULL, NULL);
 			acpi_walk_namespace(ACPI_TYPE_ANY, handle,
-					    ACPI_UINT32_MAX,
-					    acpi_bus_online_companions, NULL,
-					    NULL, NULL);
+					    ACPI_UINT32_MAX, acpi_bus_online,
+					    NULL, NULL, NULL);
 			put_device(&device->dev);
 			return -EBUSY;
 		}
@@ -290,10 +301,9 @@ static void acpi_bus_device_eject(void *
 		goto err_out;
 
 	handler = device->handler;
-	if (!handler || !handler->hotplug.enabled) {
-		ost_code = ACPI_OST_SC_EJECT_NOT_SUPPORTED;
-		goto err_out;
-	}
+	if (!handler || !handler->hotplug.enabled)
+		goto err_support;
+
 	acpi_evaluate_hotplug_ost(handle, ACPI_NOTIFY_EJECT_REQUEST,
 				  ACPI_OST_SC_EJECT_IN_PROGRESS, NULL);
 	if (handler->hotplug.mode == AHM_CONTAINER)
@@ -301,14 +311,19 @@ static void acpi_bus_device_eject(void *
 
 	get_device(&device->dev);
 	error = acpi_scan_hot_remove(device);
-	if (error)
+	if (error == -EPERM) {
+		goto err_support;
+	} else if (error) {
 		goto err_out;
+	}
 
  out:
 	mutex_unlock(&acpi_scan_lock);
 	unlock_device_hotplug();
 	return;
 
+ err_support:
+	ost_code = ACPI_OST_SC_EJECT_NOT_SUPPORTED;
  err_out:
 	acpi_evaluate_hotplug_ost(handle, ACPI_NOTIFY_EJECT_REQUEST, ost_code,
 				  NULL);

[fixup][PATCH 2/6] ACPI / hotplug: Refuse to hot-remove all objects with disabled hotplug

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

In theory, an ACPI device object may be the parent of another
device object whose hotplug is disabled by user space through its
scan handler.  In that case, the eject operation targeting the
parent should fail as though the parent's own hotplug was disabled,
but currently this is not the case, because acpi_scan_hot_remove()
doesn't check the disable/enable hotplug status of the children
of the top-most object passed to it.

To fix this, modify acpi_bus_offline_companions() to return an
error code if hotplug is disabled for the given device object.
[Also change the name of the function to acpi_bus_offline(),
because it is not only about companions any more, and change
the name of acpi_bus_online_companions() accordingly.]  Make
acpi_scan_hot_remove() propagate that error to its callers.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

acpi_force_hot_remove has to be honored by the new code.

Thanks,
Rafael

---
 drivers/acpi/scan.c |   58 +++++++++++++++++++++++++++++++++-------------------
 1 file changed, 37 insertions(+), 21 deletions(-)

Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -125,8 +125,8 @@ acpi_device_modalias_show(struct device
 }
 static DEVICE_ATTR(modalias, 0444, acpi_device_modalias_show, NULL);
 
-static acpi_status acpi_bus_offline_companions(acpi_handle handle, u32 lvl,
-					       void *data, void **ret_p)
+static acpi_status acpi_bus_offline(acpi_handle handle, u32 lvl, void *data,
+				    void **ret_p)
 {
 	struct acpi_device *device = NULL;
 	struct acpi_device_physical_node *pn;
@@ -136,6 +136,12 @@ static acpi_status acpi_bus_offline_comp
 	if (acpi_bus_get_device(handle, &device))
 		return AE_OK;
 
+	if (device->handler && !device->handler->hotplug.enabled
+	    && !acpi_force_hot_remove) {
+		*ret_p = &device->dev;
+		return AE_SUPPORT;
+	}
+
 	mutex_lock(&device->physical_node_lock);
 
 	list_for_each_entry(pn, &device->physical_node_list, node) {
@@ -168,8 +174,8 @@ static acpi_status acpi_bus_offline_comp
 	return status;
 }
 
-static acpi_status acpi_bus_online_companions(acpi_handle handle, u32 lvl,
-					      void *data, void **ret_p)
+static acpi_status acpi_bus_online(acpi_handle handle, u32 lvl, void *data,
+				   void **ret_p)
 {
 	struct acpi_device *device = NULL;
 	struct acpi_device_physical_node *pn;
@@ -214,26 +220,32 @@ static int acpi_scan_hot_remove(struct a
 	 * If the first pass is successful, the second one isn't needed, though.
 	 */
 	errdev = NULL;
-	acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX,
-			    NULL, acpi_bus_offline_companions,
-			    (void *)false, (void **)&errdev);
-	acpi_bus_offline_companions(handle, 0, (void *)false, (void **)&errdev);
+	status = acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX,
+				     NULL, acpi_bus_offline, (void *)false,
+				     (void **)&errdev);
+	if (status == AE_SUPPORT) {
+		dev_warn(errdev, "Offline disabled.\n");
+		acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX,
+				    acpi_bus_online, NULL, NULL, NULL);
+		put_device(&device->dev);
+		return -EPERM;
+	}
+	acpi_bus_offline(handle, 0, (void *)false, (void **)&errdev);
 	if (errdev) {
 		errdev = NULL;
 		acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX,
-				    NULL, acpi_bus_offline_companions,
-				    (void *)true , (void **)&errdev);
+				    NULL, acpi_bus_offline, (void *)true,
+				    (void **)&errdev);
 		if (!errdev || acpi_force_hot_remove)
-			acpi_bus_offline_companions(handle, 0, (void *)true,
-						    (void **)&errdev);
+			acpi_bus_offline(handle, 0, (void *)true,
+					 (void **)&errdev);
 
 		if (errdev && !acpi_force_hot_remove) {
 			dev_warn(errdev, "Offline failed.\n");
-			acpi_bus_online_companions(handle, 0, NULL, NULL);
+			acpi_bus_online(handle, 0, NULL, NULL);
 			acpi_walk_namespace(ACPI_TYPE_ANY, handle,
-					    ACPI_UINT32_MAX,
-					    acpi_bus_online_companions, NULL,
-					    NULL, NULL);
+					    ACPI_UINT32_MAX, acpi_bus_online,
+					    NULL, NULL, NULL);
 			put_device(&device->dev);
 			return -EBUSY;
 		}
@@ -290,10 +302,9 @@ static void acpi_bus_device_eject(void *
 		goto err_out;
 
 	handler = device->handler;
-	if (!handler || !handler->hotplug.enabled) {
-		ost_code = ACPI_OST_SC_EJECT_NOT_SUPPORTED;
-		goto err_out;
-	}
+	if (!handler || !handler->hotplug.enabled)
+		goto err_support;
+
 	acpi_evaluate_hotplug_ost(handle, ACPI_NOTIFY_EJECT_REQUEST,
 				  ACPI_OST_SC_EJECT_IN_PROGRESS, NULL);
 	if (handler->hotplug.mode == AHM_CONTAINER)
@@ -301,14 +312,19 @@ static void acpi_bus_device_eject(void *
 
 	get_device(&device->dev);
 	error = acpi_scan_hot_remove(device);
-	if (error)
+	if (error == -EPERM) {
+		goto err_support;
+	} else if (error) {
 		goto err_out;
+	}
 
  out:
 	mutex_unlock(&acpi_scan_lock);
 	unlock_device_hotplug();
 	return;
 
+ err_support:
+	ost_code = ACPI_OST_SC_EJECT_NOT_SUPPORTED;
  err_out:
 	acpi_evaluate_hotplug_ost(handle, ACPI_NOTIFY_EJECT_REQUEST, ost_code,
 				  NULL);

Re: [fixup][PATCH 2/6] ACPI / hotplug: Refuse to hot-remove all objects with disabled hotplug

[Toshi Kani]

On Wed, 2013-11-06 at 00:27 +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> In theory, an ACPI device object may be the parent of another
> device object whose hotplug is disabled by user space through its
> scan handler.  In that case, the eject operation targeting the
> parent should fail as though the parent's own hotplug was disabled,
> but currently this is not the case, because acpi_scan_hot_remove()
> doesn't check the disable/enable hotplug status of the children
> of the top-most object passed to it.
> 
> To fix this, modify acpi_bus_offline_companions() to return an
> error code if hotplug is disabled for the given device object.
> [Also change the name of the function to acpi_bus_offline(),
> because it is not only about companions any more, and change
> the name of acpi_bus_online_companions() accordingly.]  Make
> acpi_scan_hot_remove() propagate that error to its callers.
> 
 :
> +static acpi_status acpi_bus_online(acpi_handle handle, u32 lvl, void *data,
> +				   void **ret_p)
>  {
>  	struct acpi_device *device = NULL;
>  	struct acpi_device_physical_node *pn;
> @@ -214,26 +220,32 @@ static int acpi_scan_hot_remove(struct a
>  	 * If the first pass is successful, the second one isn't needed, though.
>  	 */
>  	errdev = NULL;
> -	acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX,
> -			    NULL, acpi_bus_offline_companions,
> -			    (void *)false, (void **)&errdev);
> -	acpi_bus_offline_companions(handle, 0, (void *)false, (void **)&errdev);
> +	status = acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX,
> +				     NULL, acpi_bus_offline, (void *)false,
> +				     (void **)&errdev);
> +	if (status == AE_SUPPORT) {
> +		dev_warn(errdev, "Offline disabled.\n");
> +		acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX,
> +				    acpi_bus_online, NULL, NULL, NULL);
> +		put_device(&device->dev);
> +		return -EPERM;
> +	}
> +	acpi_bus_offline(handle, 0, (void *)false, (void **)&errdev);
>  	if (errdev) {

If the target object failed with AE_SUPPORT, shouldn't we skip the 2nd
pass and return with -EPERM after rollback? 

Thanks,
-Toshi


>  		errdev = NULL;
>  		acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX,
> -				    NULL, acpi_bus_offline_companions,
> -				    (void *)true , (void **)&errdev);
> +				    NULL, acpi_bus_offline, (void *)true,
> +				    (void **)&errdev);
>  		if (!errdev || acpi_force_hot_remove)
> -			acpi_bus_offline_companions(handle, 0, (void *)true,
> -						    (void **)&errdev);
> +			acpi_bus_offline(handle, 0, (void *)true,
> +					 (void **)&errdev);
>  
>  		if (errdev && !acpi_force_hot_remove) {
>  			dev_warn(errdev, "Offline failed.\n");
> -			acpi_bus_online_companions(handle, 0, NULL, NULL);
> +			acpi_bus_online(handle, 0, NULL, NULL);
>  			acpi_walk_namespace(ACPI_TYPE_ANY, handle,
> -					    ACPI_UINT32_MAX,
> -					    acpi_bus_online_companions, NULL,
> -					    NULL, NULL);
> +					    ACPI_UINT32_MAX, acpi_bus_online,
> +					    NULL, NULL, NULL);
>  			put_device(&device->dev);
>  			return -EBUSY;
>  		}

Re: [fixup][PATCH 2/6] ACPI / hotplug: Refuse to hot-remove all objects with disabled hotplug

[Rafael J. Wysocki]

On Tuesday, November 05, 2013 05:39:27 PM Toshi Kani wrote:
> On Wed, 2013-11-06 at 00:27 +0100, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > 
> > In theory, an ACPI device object may be the parent of another
> > device object whose hotplug is disabled by user space through its
> > scan handler.  In that case, the eject operation targeting the
> > parent should fail as though the parent's own hotplug was disabled,
> > but currently this is not the case, because acpi_scan_hot_remove()
> > doesn't check the disable/enable hotplug status of the children
> > of the top-most object passed to it.
> > 
> > To fix this, modify acpi_bus_offline_companions() to return an
> > error code if hotplug is disabled for the given device object.
> > [Also change the name of the function to acpi_bus_offline(),
> > because it is not only about companions any more, and change
> > the name of acpi_bus_online_companions() accordingly.]  Make
> > acpi_scan_hot_remove() propagate that error to its callers.
> > 
>  :
> > +static acpi_status acpi_bus_online(acpi_handle handle, u32 lvl, void *data,
> > +				   void **ret_p)
> >  {
> >  	struct acpi_device *device = NULL;
> >  	struct acpi_device_physical_node *pn;
> > @@ -214,26 +220,32 @@ static int acpi_scan_hot_remove(struct a
> >  	 * If the first pass is successful, the second one isn't needed, though.
> >  	 */
> >  	errdev = NULL;
> > -	acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX,
> > -			    NULL, acpi_bus_offline_companions,
> > -			    (void *)false, (void **)&errdev);
> > -	acpi_bus_offline_companions(handle, 0, (void *)false, (void **)&errdev);
> > +	status = acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX,
> > +				     NULL, acpi_bus_offline, (void *)false,
> > +				     (void **)&errdev);
> > +	if (status == AE_SUPPORT) {
> > +		dev_warn(errdev, "Offline disabled.\n");
> > +		acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX,
> > +				    acpi_bus_online, NULL, NULL, NULL);
> > +		put_device(&device->dev);
> > +		return -EPERM;
> > +	}
> > +	acpi_bus_offline(handle, 0, (void *)false, (void **)&errdev);
> >  	if (errdev) {
> 
> If the target object failed with AE_SUPPORT, shouldn't we skip the 2nd
> pass and return with -EPERM after rollback?

We've checked the target object already in acpi_hotplug_notify_cb() or in
acpi_eject_store().

Which is telling me that the previous version of the patch was better after
all, because the hotplug.enabled thing takes precedence over
acpi_force_hot_remove in the other places.  So this:

https://patchwork.kernel.org/patch/3135841/

is the right version.  Sorry for the confusion.

Thanks,
Rafael

Re: [fixup][PATCH 2/6] ACPI / hotplug: Refuse to hot-remove all objects with disabled hotplug

[Toshi Kani]

On Wed, 2013-11-06 at 02:35 +0100, Rafael J. Wysocki wrote:
> On Tuesday, November 05, 2013 05:39:27 PM Toshi Kani wrote:
> > On Wed, 2013-11-06 at 00:27 +0100, Rafael J. Wysocki wrote:
> > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > 
> > > In theory, an ACPI device object may be the parent of another
> > > device object whose hotplug is disabled by user space through its
> > > scan handler.  In that case, the eject operation targeting the
> > > parent should fail as though the parent's own hotplug was disabled,
> > > but currently this is not the case, because acpi_scan_hot_remove()
> > > doesn't check the disable/enable hotplug status of the children
> > > of the top-most object passed to it.
> > > 
> > > To fix this, modify acpi_bus_offline_companions() to return an
> > > error code if hotplug is disabled for the given device object.
> > > [Also change the name of the function to acpi_bus_offline(),
> > > because it is not only about companions any more, and change
> > > the name of acpi_bus_online_companions() accordingly.]  Make
> > > acpi_scan_hot_remove() propagate that error to its callers.
> > > 
> >  :
> > > +static acpi_status acpi_bus_online(acpi_handle handle, u32 lvl, void *data,
> > > +				   void **ret_p)
> > >  {
> > >  	struct acpi_device *device = NULL;
> > >  	struct acpi_device_physical_node *pn;
> > > @@ -214,26 +220,32 @@ static int acpi_scan_hot_remove(struct a
> > >  	 * If the first pass is successful, the second one isn't needed, though.
> > >  	 */
> > >  	errdev = NULL;
> > > -	acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX,
> > > -			    NULL, acpi_bus_offline_companions,
> > > -			    (void *)false, (void **)&errdev);
> > > -	acpi_bus_offline_companions(handle, 0, (void *)false, (void **)&errdev);
> > > +	status = acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX,
> > > +				     NULL, acpi_bus_offline, (void *)false,
> > > +				     (void **)&errdev);
> > > +	if (status == AE_SUPPORT) {
> > > +		dev_warn(errdev, "Offline disabled.\n");
> > > +		acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX,
> > > +				    acpi_bus_online, NULL, NULL, NULL);
> > > +		put_device(&device->dev);
> > > +		return -EPERM;
> > > +	}
> > > +	acpi_bus_offline(handle, 0, (void *)false, (void **)&errdev);
> > >  	if (errdev) {
> > 
> > If the target object failed with AE_SUPPORT, shouldn't we skip the 2nd
> > pass and return with -EPERM after rollback?
> 
> We've checked the target object already in acpi_hotplug_notify_cb() or in
> acpi_eject_store().

Oh, I see.  Thanks for the clarification.

> 
> Which is telling me that the previous version of the patch was better after
> all, because the hotplug.enabled thing takes precedence over
> acpi_force_hot_remove in the other places.  So this:
> 
> https://patchwork.kernel.org/patch/3135841/
> 
> is the right version.  Sorry for the confusion.

Agreed.

Acked-by: Toshi Kani <toshi.kani@hp.com>

-Toshi

[Update][PATCH 3/6] ACPI / hotplug: Fix handle_root_bridge_removal()

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

It is required to do get_device() on the struct acpi_device in
question before passing it to acpi_bus_hot_remove_device() through
acpi_os_hotplug_execute(), because acpi_bus_hot_remove_device()
calls acpi_scan_hot_remove() that does put_device() on that
object.

The ACPI PCI root removal routine, handle_root_bridge_removal(),
doesn't do that, which may lead to premature freeing of the
device object or to executing put_device() on an object that
has been freed already.

Fix this problem by making handle_root_bridge_removal() use
get_device() as appropriate.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/pci_root.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Index: linux-pm/drivers/acpi/pci_root.c
===================================================================
--- linux-pm.orig/drivers/acpi/pci_root.c
+++ linux-pm/drivers/acpi/pci_root.c
@@ -608,9 +608,12 @@ static void handle_root_bridge_removal(s
 	ej_event->device = device;
 	ej_event->event = ACPI_NOTIFY_EJECT_REQUEST;
 
+	get_device(&device->dev);
 	status = acpi_os_hotplug_execute(acpi_bus_hot_remove_device, ej_event);
-	if (ACPI_FAILURE(status))
+	if (ACPI_FAILURE(status)) {
+		put_device(&device->dev);
 		kfree(ej_event);
+	}
 }
 
 static void _handle_hotplug_event_root(struct work_struct *work)

Re: [Update][PATCH 3/6] ACPI / hotplug: Fix handle_root_bridge_removal()

[Toshi Kani]

On Mon, 2013-11-04 at 14:33 +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> It is required to do get_device() on the struct acpi_device in
> question before passing it to acpi_bus_hot_remove_device() through
> acpi_os_hotplug_execute(), because acpi_bus_hot_remove_device()
> calls acpi_scan_hot_remove() that does put_device() on that
> object.
> 
> The ACPI PCI root removal routine, handle_root_bridge_removal(),
> doesn't do that, which may lead to premature freeing of the
> device object or to executing put_device() on an object that
> has been freed already.
> 
> Fix this problem by making handle_root_bridge_removal() use
> get_device() as appropriate.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Acked-by: Toshi Kani <toshi.kani@hp.com>

Thanks,
-Toshi

[Update][PATCH 4/6] ACPI / hotplug: Simplify device ejection routines

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Simplify handle_root_bridge_removal() and acpi_eject_store() by
getting rid of struct acpi_eject_event and passing device objects
directly to async routines executed via acpi_os_hotplug_execute().

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/pci_root.c |   20 ++------------------
 drivers/acpi/scan.c     |   46 ++++++++++++++++++----------------------------
 include/acpi/acpi_bus.h |    5 -----
 3 files changed, 20 insertions(+), 51 deletions(-)

Index: linux-pm/drivers/acpi/pci_root.c
===================================================================
--- linux-pm.orig/drivers/acpi/pci_root.c
+++ linux-pm/drivers/acpi/pci_root.c
@@ -593,27 +593,11 @@ static void handle_root_bridge_insertion
 static void handle_root_bridge_removal(struct acpi_device *device)
 {
 	acpi_status status;
-	struct acpi_eject_event *ej_event;
-
-	ej_event = kmalloc(sizeof(*ej_event), GFP_KERNEL);
-	if (!ej_event) {
-		/* Inform firmware the hot-remove operation has error */
-		(void) acpi_evaluate_hotplug_ost(device->handle,
-					ACPI_NOTIFY_EJECT_REQUEST,
-					ACPI_OST_SC_NON_SPECIFIC_FAILURE,
-					NULL);
-		return;
-	}
-
-	ej_event->device = device;
-	ej_event->event = ACPI_NOTIFY_EJECT_REQUEST;
 
 	get_device(&device->dev);
-	status = acpi_os_hotplug_execute(acpi_bus_hot_remove_device, ej_event);
-	if (ACPI_FAILURE(status)) {
+	status = acpi_os_hotplug_execute(acpi_bus_hot_remove_device, device);
+	if (ACPI_FAILURE(status))
 		put_device(&device->dev);
-		kfree(ej_event);
-	}
 }
 
 static void _handle_hotplug_event_root(struct work_struct *work)
Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -441,18 +441,8 @@ static void acpi_hotplug_notify_cb(acpi_
 					  NULL);
 }
 
-/**
- * acpi_bus_hot_remove_device: hot-remove a device and its children
- * @context: struct acpi_eject_event pointer (freed in this func)
- *
- * Hot-remove a device and its children. This function frees up the
- * memory space passed by arg context, so that the caller may call
- * this function asynchronously through acpi_os_hotplug_execute().
- */
-void acpi_bus_hot_remove_device(void *context)
+void __acpi_bus_hot_remove_device(struct acpi_device *device, u32 ost_src)
 {
-	struct acpi_eject_event *ej_event = context;
-	struct acpi_device *device = ej_event->device;
 	acpi_handle handle = device->handle;
 	int error;
 
@@ -461,13 +451,21 @@ void acpi_bus_hot_remove_device(void *co
 
 	error = acpi_scan_hot_remove(device);
 	if (error && handle)
-		acpi_evaluate_hotplug_ost(handle, ej_event->event,
+		acpi_evaluate_hotplug_ost(handle, ost_src,
 					  ACPI_OST_SC_NON_SPECIFIC_FAILURE,
 					  NULL);
 
 	mutex_unlock(&acpi_scan_lock);
 	unlock_device_hotplug();
-	kfree(context);
+}
+
+/**
+ * acpi_bus_hot_remove_device: Hot-remove a device and its children.
+ * @context: Address of the ACPI device object to hot-remove.
+ */
+void acpi_bus_hot_remove_device(void *context)
+{
+	__acpi_bus_hot_remove_device(context, ACPI_NOTIFY_EJECT_REQUEST);
 }
 EXPORT_SYMBOL(acpi_bus_hot_remove_device);
 
@@ -497,15 +495,18 @@ static ssize_t power_state_show(struct d
 
 static DEVICE_ATTR(power_state, 0444, power_state_show, NULL);
 
+static void acpi_eject_store_work(void *context)
+{
+	__acpi_bus_hot_remove_device(context, ACPI_OST_EC_OSPM_EJECT);
+}
+
 static ssize_t
 acpi_eject_store(struct device *d, struct device_attribute *attr,
 		const char *buf, size_t count)
 {
 	struct acpi_device *acpi_device = to_acpi_device(d);
-	struct acpi_eject_event *ej_event;
 	acpi_object_type not_used;
 	acpi_status status;
-	int ret;
 
 	if (!count || buf[0] != '1')
 		return -EINVAL;
@@ -518,28 +519,17 @@ acpi_eject_store(struct device *d, struc
 	if (ACPI_FAILURE(status) || !acpi_device->flags.ejectable)
 		return -ENODEV;
 
-	ej_event = kmalloc(sizeof(*ej_event), GFP_KERNEL);
-	if (!ej_event) {
-		ret = -ENOMEM;
-		goto err_out;
-	}
 	acpi_evaluate_hotplug_ost(acpi_device->handle, ACPI_OST_EC_OSPM_EJECT,
 				  ACPI_OST_SC_EJECT_IN_PROGRESS, NULL);
-	ej_event->device = acpi_device;
-	ej_event->event = ACPI_OST_EC_OSPM_EJECT;
 	get_device(&acpi_device->dev);
-	status = acpi_os_hotplug_execute(acpi_bus_hot_remove_device, ej_event);
+	status = acpi_os_hotplug_execute(acpi_eject_store_work, acpi_device);
 	if (ACPI_SUCCESS(status))
 		return count;
 
 	put_device(&acpi_device->dev);
-	kfree(ej_event);
-	ret = status == AE_NO_MEMORY ? -ENOMEM : -EAGAIN;
-
- err_out:
 	acpi_evaluate_hotplug_ost(acpi_device->handle, ACPI_OST_EC_OSPM_EJECT,
 				  ACPI_OST_SC_NON_SPECIFIC_FAILURE, NULL);
-	return ret;
+	return -EAGAIN;
 }
 
 static DEVICE_ATTR(eject, 0200, NULL, acpi_eject_store);
Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -339,11 +339,6 @@ struct acpi_bus_event {
 	u32 data;
 };
 
-struct acpi_eject_event {
-	struct acpi_device	*device;
-	u32		event;
-};
-
 struct acpi_hp_work {
 	struct work_struct work;
 	acpi_handle handle;

Re: [Update][PATCH 4/6] ACPI / hotplug: Simplify device ejection routines

[Toshi Kani]

On Mon, 2013-11-04 at 14:36 +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Simplify handle_root_bridge_removal() and acpi_eject_store() by
> getting rid of struct acpi_eject_event and passing device objects
> directly to async routines executed via acpi_os_hotplug_execute().
 :
>  static ssize_t
>  acpi_eject_store(struct device *d, struct device_attribute *attr,
>  		const char *buf, size_t count)
>  {
>  	struct acpi_device *acpi_device = to_acpi_device(d);
> -	struct acpi_eject_event *ej_event;
>  	acpi_object_type not_used;
>  	acpi_status status;
> -	int ret;
>  
>  	if (!count || buf[0] != '1')
>  		return -EINVAL;
> @@ -518,28 +519,17 @@ acpi_eject_store(struct device *d, struc
>  	if (ACPI_FAILURE(status) || !acpi_device->flags.ejectable)
>  		return -ENODEV;
>  
> -	ej_event = kmalloc(sizeof(*ej_event), GFP_KERNEL);
> -	if (!ej_event) {
> -		ret = -ENOMEM;
> -		goto err_out;
> -	}
>  	acpi_evaluate_hotplug_ost(acpi_device->handle, ACPI_OST_EC_OSPM_EJECT,
>  				  ACPI_OST_SC_EJECT_IN_PROGRESS, NULL);
> -	ej_event->device = acpi_device;
> -	ej_event->event = ACPI_OST_EC_OSPM_EJECT;
>  	get_device(&acpi_device->dev);
> -	status = acpi_os_hotplug_execute(acpi_bus_hot_remove_device, ej_event);
> +	status = acpi_os_hotplug_execute(acpi_eject_store_work, acpi_device);
>  	if (ACPI_SUCCESS(status))
>  		return count;
>  
>  	put_device(&acpi_device->dev);
> -	kfree(ej_event);
> -	ret = status == AE_NO_MEMORY ? -ENOMEM : -EAGAIN;

Why are you deleting the above line as part of this change?

Thanks,
-Toshi

Re: [Update][PATCH 4/6] ACPI / hotplug: Simplify device ejection routines

[Rafael J. Wysocki]

On Wednesday, November 06, 2013 04:27:01 PM Toshi Kani wrote:
> On Mon, 2013-11-04 at 14:36 +0100, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > 
> > Simplify handle_root_bridge_removal() and acpi_eject_store() by
> > getting rid of struct acpi_eject_event and passing device objects
> > directly to async routines executed via acpi_os_hotplug_execute().
>  :
> >  static ssize_t
> >  acpi_eject_store(struct device *d, struct device_attribute *attr,
> >  		const char *buf, size_t count)
> >  {
> >  	struct acpi_device *acpi_device = to_acpi_device(d);
> > -	struct acpi_eject_event *ej_event;
> >  	acpi_object_type not_used;
> >  	acpi_status status;
> > -	int ret;
> >  
> >  	if (!count || buf[0] != '1')
> >  		return -EINVAL;
> > @@ -518,28 +519,17 @@ acpi_eject_store(struct device *d, struc
> >  	if (ACPI_FAILURE(status) || !acpi_device->flags.ejectable)
> >  		return -ENODEV;
> >  
> > -	ej_event = kmalloc(sizeof(*ej_event), GFP_KERNEL);
> > -	if (!ej_event) {
> > -		ret = -ENOMEM;
> > -		goto err_out;
> > -	}
> >  	acpi_evaluate_hotplug_ost(acpi_device->handle, ACPI_OST_EC_OSPM_EJECT,
> >  				  ACPI_OST_SC_EJECT_IN_PROGRESS, NULL);
> > -	ej_event->device = acpi_device;
> > -	ej_event->event = ACPI_OST_EC_OSPM_EJECT;
> >  	get_device(&acpi_device->dev);
> > -	status = acpi_os_hotplug_execute(acpi_bus_hot_remove_device, ej_event);
> > +	status = acpi_os_hotplug_execute(acpi_eject_store_work, acpi_device);
> >  	if (ACPI_SUCCESS(status))
> >  		return count;
> >  
> >  	put_device(&acpi_device->dev);
> > -	kfree(ej_event);
> > -	ret = status == AE_NO_MEMORY ? -ENOMEM : -EAGAIN;
> 
> Why are you deleting the above line as part of this change?

Ah, because I forgot that acpi_os_hotplug_execute() can also return AE_NO_MEMORY. :-)

Good catch, updated pach follows.

Thanks,
Rafael

---
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Subject: ACPI / hotplug: Simplify device ejection routines

Simplify handle_root_bridge_removal() and acpi_eject_store() by
getting rid of struct acpi_eject_event and passing device objects
directly to async routines executed via acpi_os_hotplug_execute().

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/pci_root.c |   20 ++------------------
 drivers/acpi/scan.c     |   46 ++++++++++++++++++----------------------------
 include/acpi/acpi_bus.h |    5 -----
 3 files changed, 20 insertions(+), 51 deletions(-)

Index: linux-pm/drivers/acpi/pci_root.c
===================================================================
--- linux-pm.orig/drivers/acpi/pci_root.c
+++ linux-pm/drivers/acpi/pci_root.c
@@ -593,27 +593,11 @@ static void handle_root_bridge_insertion
 static void handle_root_bridge_removal(struct acpi_device *device)
 {
 	acpi_status status;
-	struct acpi_eject_event *ej_event;
-
-	ej_event = kmalloc(sizeof(*ej_event), GFP_KERNEL);
-	if (!ej_event) {
-		/* Inform firmware the hot-remove operation has error */
-		(void) acpi_evaluate_hotplug_ost(device->handle,
-					ACPI_NOTIFY_EJECT_REQUEST,
-					ACPI_OST_SC_NON_SPECIFIC_FAILURE,
-					NULL);
-		return;
-	}
-
-	ej_event->device = device;
-	ej_event->event = ACPI_NOTIFY_EJECT_REQUEST;
 
 	get_device(&device->dev);
-	status = acpi_os_hotplug_execute(acpi_bus_hot_remove_device, ej_event);
-	if (ACPI_FAILURE(status)) {
+	status = acpi_os_hotplug_execute(acpi_bus_hot_remove_device, device);
+	if (ACPI_FAILURE(status))
 		put_device(&device->dev);
-		kfree(ej_event);
-	}
 }
 
 static void _handle_hotplug_event_root(struct work_struct *work)
Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -441,18 +441,8 @@ static void acpi_hotplug_notify_cb(acpi_
 					  NULL);
 }
 
-/**
- * acpi_bus_hot_remove_device: hot-remove a device and its children
- * @context: struct acpi_eject_event pointer (freed in this func)
- *
- * Hot-remove a device and its children. This function frees up the
- * memory space passed by arg context, so that the caller may call
- * this function asynchronously through acpi_os_hotplug_execute().
- */
-void acpi_bus_hot_remove_device(void *context)
+void __acpi_bus_hot_remove_device(struct acpi_device *device, u32 ost_src)
 {
-	struct acpi_eject_event *ej_event = context;
-	struct acpi_device *device = ej_event->device;
 	acpi_handle handle = device->handle;
 	int error;
 
@@ -461,13 +451,21 @@ void acpi_bus_hot_remove_device(void *co
 
 	error = acpi_scan_hot_remove(device);
 	if (error && handle)
-		acpi_evaluate_hotplug_ost(handle, ej_event->event,
+		acpi_evaluate_hotplug_ost(handle, ost_src,
 					  ACPI_OST_SC_NON_SPECIFIC_FAILURE,
 					  NULL);
 
 	mutex_unlock(&acpi_scan_lock);
 	unlock_device_hotplug();
-	kfree(context);
+}
+
+/**
+ * acpi_bus_hot_remove_device: Hot-remove a device and its children.
+ * @context: Address of the ACPI device object to hot-remove.
+ */
+void acpi_bus_hot_remove_device(void *context)
+{
+	__acpi_bus_hot_remove_device(context, ACPI_NOTIFY_EJECT_REQUEST);
 }
 EXPORT_SYMBOL(acpi_bus_hot_remove_device);
 
@@ -497,15 +495,18 @@ static ssize_t power_state_show(struct d
 
 static DEVICE_ATTR(power_state, 0444, power_state_show, NULL);
 
+static void acpi_eject_store_work(void *context)
+{
+	__acpi_bus_hot_remove_device(context, ACPI_OST_EC_OSPM_EJECT);
+}
+
 static ssize_t
 acpi_eject_store(struct device *d, struct device_attribute *attr,
 		const char *buf, size_t count)
 {
 	struct acpi_device *acpi_device = to_acpi_device(d);
-	struct acpi_eject_event *ej_event;
 	acpi_object_type not_used;
 	acpi_status status;
-	int ret;
 
 	if (!count || buf[0] != '1')
 		return -EINVAL;
@@ -518,28 +519,17 @@ acpi_eject_store(struct device *d, struc
 	if (ACPI_FAILURE(status) || !acpi_device->flags.ejectable)
 		return -ENODEV;
 
-	ej_event = kmalloc(sizeof(*ej_event), GFP_KERNEL);
-	if (!ej_event) {
-		ret = -ENOMEM;
-		goto err_out;
-	}
 	acpi_evaluate_hotplug_ost(acpi_device->handle, ACPI_OST_EC_OSPM_EJECT,
 				  ACPI_OST_SC_EJECT_IN_PROGRESS, NULL);
-	ej_event->device = acpi_device;
-	ej_event->event = ACPI_OST_EC_OSPM_EJECT;
 	get_device(&acpi_device->dev);
-	status = acpi_os_hotplug_execute(acpi_bus_hot_remove_device, ej_event);
+	status = acpi_os_hotplug_execute(acpi_eject_store_work, acpi_device);
 	if (ACPI_SUCCESS(status))
 		return count;
 
 	put_device(&acpi_device->dev);
-	kfree(ej_event);
-	ret = status == AE_NO_MEMORY ? -ENOMEM : -EAGAIN;
-
- err_out:
 	acpi_evaluate_hotplug_ost(acpi_device->handle, ACPI_OST_EC_OSPM_EJECT,
 				  ACPI_OST_SC_NON_SPECIFIC_FAILURE, NULL);
-	return ret;
+	return status == AE_NO_MEMORY ? -ENOMEM : -EAGAIN;
 }
 
 static DEVICE_ATTR(eject, 0200, NULL, acpi_eject_store);
Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -339,11 +339,6 @@ struct acpi_bus_event {
 	u32 data;
 };
 
-struct acpi_eject_event {
-	struct acpi_device	*device;
-	u32		event;
-};
-
 struct acpi_hp_work {
 	struct work_struct work;
 	acpi_handle handle;

Re: [Update][PATCH 4/6] ACPI / hotplug: Simplify device ejection routines

[Toshi Kani]

On Thu, 2013-11-07 at 01:14 +0100, Rafael J. Wysocki wrote:
> On Wednesday, November 06, 2013 04:27:01 PM Toshi Kani wrote:
> > On Mon, 2013-11-04 at 14:36 +0100, Rafael J. Wysocki wrote:
> > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > 
> > > Simplify handle_root_bridge_removal() and acpi_eject_store() by
> > > getting rid of struct acpi_eject_event and passing device objects
> > > directly to async routines executed via acpi_os_hotplug_execute().
> >  :
> > >  static ssize_t
> > >  acpi_eject_store(struct device *d, struct device_attribute *attr,
> > >  		const char *buf, size_t count)
> > >  {
> > >  	struct acpi_device *acpi_device = to_acpi_device(d);
> > > -	struct acpi_eject_event *ej_event;
> > >  	acpi_object_type not_used;
> > >  	acpi_status status;
> > > -	int ret;
> > >  
> > >  	if (!count || buf[0] != '1')
> > >  		return -EINVAL;
> > > @@ -518,28 +519,17 @@ acpi_eject_store(struct device *d, struc
> > >  	if (ACPI_FAILURE(status) || !acpi_device->flags.ejectable)
> > >  		return -ENODEV;
> > >  
> > > -	ej_event = kmalloc(sizeof(*ej_event), GFP_KERNEL);
> > > -	if (!ej_event) {
> > > -		ret = -ENOMEM;
> > > -		goto err_out;
> > > -	}
> > >  	acpi_evaluate_hotplug_ost(acpi_device->handle, ACPI_OST_EC_OSPM_EJECT,
> > >  				  ACPI_OST_SC_EJECT_IN_PROGRESS, NULL);
> > > -	ej_event->device = acpi_device;
> > > -	ej_event->event = ACPI_OST_EC_OSPM_EJECT;
> > >  	get_device(&acpi_device->dev);
> > > -	status = acpi_os_hotplug_execute(acpi_bus_hot_remove_device, ej_event);
> > > +	status = acpi_os_hotplug_execute(acpi_eject_store_work, acpi_device);
> > >  	if (ACPI_SUCCESS(status))
> > >  		return count;
> > >  
> > >  	put_device(&acpi_device->dev);
> > > -	kfree(ej_event);
> > > -	ret = status == AE_NO_MEMORY ? -ENOMEM : -EAGAIN;
> > 
> > Why are you deleting the above line as part of this change?
> 
> Ah, because I forgot that acpi_os_hotplug_execute() can also return AE_NO_MEMORY. :-)
> 
> Good catch, updated pach follows.

Looks good.  The rest of the patches also look good.  For patch 4-6:

Acked-by: Toshi Kani <toshi.kani@hp.com>

Thanks,
-Toshi

Re: [Update][PATCH 4/6] ACPI / hotplug: Simplify device ejection routines

[Rafael J. Wysocki]

On Wednesday, November 06, 2013 05:17:05 PM Toshi Kani wrote:
> On Thu, 2013-11-07 at 01:14 +0100, Rafael J. Wysocki wrote:
> > On Wednesday, November 06, 2013 04:27:01 PM Toshi Kani wrote:
> > > On Mon, 2013-11-04 at 14:36 +0100, Rafael J. Wysocki wrote:
> > > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > > 
> > > > Simplify handle_root_bridge_removal() and acpi_eject_store() by
> > > > getting rid of struct acpi_eject_event and passing device objects
> > > > directly to async routines executed via acpi_os_hotplug_execute().
> > >  :
> > > >  static ssize_t
> > > >  acpi_eject_store(struct device *d, struct device_attribute *attr,
> > > >  		const char *buf, size_t count)
> > > >  {
> > > >  	struct acpi_device *acpi_device = to_acpi_device(d);
> > > > -	struct acpi_eject_event *ej_event;
> > > >  	acpi_object_type not_used;
> > > >  	acpi_status status;
> > > > -	int ret;
> > > >  
> > > >  	if (!count || buf[0] != '1')
> > > >  		return -EINVAL;
> > > > @@ -518,28 +519,17 @@ acpi_eject_store(struct device *d, struc
> > > >  	if (ACPI_FAILURE(status) || !acpi_device->flags.ejectable)
> > > >  		return -ENODEV;
> > > >  
> > > > -	ej_event = kmalloc(sizeof(*ej_event), GFP_KERNEL);
> > > > -	if (!ej_event) {
> > > > -		ret = -ENOMEM;
> > > > -		goto err_out;
> > > > -	}
> > > >  	acpi_evaluate_hotplug_ost(acpi_device->handle, ACPI_OST_EC_OSPM_EJECT,
> > > >  				  ACPI_OST_SC_EJECT_IN_PROGRESS, NULL);
> > > > -	ej_event->device = acpi_device;
> > > > -	ej_event->event = ACPI_OST_EC_OSPM_EJECT;
> > > >  	get_device(&acpi_device->dev);
> > > > -	status = acpi_os_hotplug_execute(acpi_bus_hot_remove_device, ej_event);
> > > > +	status = acpi_os_hotplug_execute(acpi_eject_store_work, acpi_device);
> > > >  	if (ACPI_SUCCESS(status))
> > > >  		return count;
> > > >  
> > > >  	put_device(&acpi_device->dev);
> > > > -	kfree(ej_event);
> > > > -	ret = status == AE_NO_MEMORY ? -ENOMEM : -EAGAIN;
> > > 
> > > Why are you deleting the above line as part of this change?
> > 
> > Ah, because I forgot that acpi_os_hotplug_execute() can also return AE_NO_MEMORY. :-)
> > 
> > Good catch, updated pach follows.
> 
> Looks good.  The rest of the patches also look good.  For patch 4-6:
> 
> Acked-by: Toshi Kani <toshi.kani@hp.com>

Awesome, thanks! :-)

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

[Update][PATCH 5/6] ACPI / hotplug: Make acpi_bus_hot_remove_device() internal

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Notice that handle_root_bridge_removal() is the only user of
acpi_bus_hot_remove_device(), so it doesn't have to be exported
any more and can be made internal to the ACPI core.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/internal.h |    1 +
 drivers/acpi/pci_root.c |    2 ++
 drivers/acpi/scan.c     |    1 -
 include/acpi/acpi_bus.h |    1 -
 4 files changed, 3 insertions(+), 2 deletions(-)

Index: linux-pm/drivers/acpi/internal.h
===================================================================
--- linux-pm.orig/drivers/acpi/internal.h
+++ linux-pm/drivers/acpi/internal.h
@@ -92,6 +92,7 @@ void acpi_device_add_finalize(struct acp
 void acpi_free_pnp_ids(struct acpi_device_pnp *pnp);
 int acpi_bind_one(struct device *dev, acpi_handle handle);
 int acpi_unbind_one(struct device *dev);
+void acpi_bus_hot_remove_device(void *context);
 
 /* --------------------------------------------------------------------------
                                   Power Resource
Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -386,7 +386,6 @@ int acpi_scan_add_handler(struct acpi_sc
 int acpi_bus_register_driver(struct acpi_driver *driver);
 void acpi_bus_unregister_driver(struct acpi_driver *driver);
 int acpi_bus_scan(acpi_handle handle);
-void acpi_bus_hot_remove_device(void *context);
 void acpi_bus_trim(struct acpi_device *start);
 acpi_status acpi_bus_get_ejd(acpi_handle handle, acpi_handle * ejd);
 int acpi_match_device_ids(struct acpi_device *device,
Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -467,7 +467,6 @@ void acpi_bus_hot_remove_device(void *co
 {
 	__acpi_bus_hot_remove_device(context, ACPI_NOTIFY_EJECT_REQUEST);
 }
-EXPORT_SYMBOL(acpi_bus_hot_remove_device);
 
 static ssize_t real_power_state_show(struct device *dev,
 				     struct device_attribute *attr, char *buf)
Index: linux-pm/drivers/acpi/pci_root.c
===================================================================
--- linux-pm.orig/drivers/acpi/pci_root.c
+++ linux-pm/drivers/acpi/pci_root.c
@@ -39,6 +39,8 @@
 #include <acpi/acpi_drivers.h>
 #include <acpi/apei.h>
 
+#include "internal.h"
+
 #define PREFIX "ACPI: "
 
 #define _COMPONENT		ACPI_PCI_COMPONENT

[Update][PATCH 6/6] ACPI / hotplug: Merge device hot-removal routines

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

There is no real reasn why acpi_bus_device_eject() and
acpi_bus_hot_remove_device() should work differently, so rework
acpi_bus_device_eject() so that it can be called internally by
both acpi_bus_hot_remove_device() and acpi_eject_store_work().
Accordingly, rework acpi_hotplug_notify_cb() to queue up the
execution of acpi_bus_hot_remove_device() through
acpi_os_hotplug_execute() on eject request notifications.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/scan.c |   84 +++++++++++++++++++++++-----------------------------
 1 file changed, 38 insertions(+), 46 deletions(-)

Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -285,10 +285,9 @@ static int acpi_scan_hot_remove(struct a
 	return 0;
 }
 
-static void acpi_bus_device_eject(void *context)
+static void acpi_bus_device_eject(struct acpi_device *device, u32 ost_src)
 {
-	acpi_handle handle = context;
-	struct acpi_device *device = NULL;
+	acpi_handle handle = device->handle;
 	struct acpi_scan_handler *handler;
 	u32 ost_code = ACPI_OST_SC_NON_SPECIFIC_FAILURE;
 	int error;
@@ -296,20 +295,19 @@ static void acpi_bus_device_eject(void *
 	lock_device_hotplug();
 	mutex_lock(&acpi_scan_lock);
 
-	acpi_bus_get_device(handle, &device);
-	if (!device)
-		goto err_out;
-
 	handler = device->handler;
-	if (!handler || !handler->hotplug.enabled)
+	if (!handler || !handler->hotplug.enabled) {
+		put_device(&device->dev);
 		goto err_support;
+	}
+
+	if (ost_src == ACPI_NOTIFY_EJECT_REQUEST)
+		acpi_evaluate_hotplug_ost(handle, ACPI_NOTIFY_EJECT_REQUEST,
+					  ACPI_OST_SC_EJECT_IN_PROGRESS, NULL);
 
-	acpi_evaluate_hotplug_ost(handle, ACPI_NOTIFY_EJECT_REQUEST,
-				  ACPI_OST_SC_EJECT_IN_PROGRESS, NULL);
 	if (handler->hotplug.mode == AHM_CONTAINER)
 		kobject_uevent(&device->dev.kobj, KOBJ_OFFLINE);
 
-	get_device(&device->dev);
 	error = acpi_scan_hot_remove(device);
 	if (error == -EPERM) {
 		goto err_support;
@@ -325,8 +323,7 @@ static void acpi_bus_device_eject(void *
  err_support:
 	ost_code = ACPI_OST_SC_EJECT_NOT_SUPPORTED;
  err_out:
-	acpi_evaluate_hotplug_ost(handle, ACPI_NOTIFY_EJECT_REQUEST, ost_code,
-				  NULL);
+	acpi_evaluate_hotplug_ost(handle, ost_src, ost_code, NULL);
 	goto out;
 }
 
@@ -408,10 +405,20 @@ static void acpi_hotplug_unsupported(acp
 	acpi_evaluate_hotplug_ost(handle, type, ost_status, NULL);
 }
 
+/**
+ * acpi_bus_hot_remove_device: Hot-remove a device and its children.
+ * @context: Address of the ACPI device object to hot-remove.
+ */
+void acpi_bus_hot_remove_device(void *context)
+{
+	acpi_bus_device_eject(context, ACPI_NOTIFY_EJECT_REQUEST);
+}
+
 static void acpi_hotplug_notify_cb(acpi_handle handle, u32 type, void *data)
 {
 	acpi_osd_exec_callback callback;
 	struct acpi_scan_handler *handler = data;
+	struct acpi_device *adev;
 	acpi_status status;
 
 	if (!handler->hotplug.enabled)
@@ -428,44 +435,29 @@ static void acpi_hotplug_notify_cb(acpi_
 		break;
 	case ACPI_NOTIFY_EJECT_REQUEST:
 		acpi_handle_debug(handle, "ACPI_NOTIFY_EJECT_REQUEST event\n");
-		callback = acpi_bus_device_eject;
-		break;
+		status = acpi_bus_get_device(handle, &adev);
+		if (ACPI_FAILURE(status))
+			goto err_out;
+
+		get_device(&adev->dev);
+		callback = acpi_bus_hot_remove_device;
+		status = acpi_os_hotplug_execute(callback, adev);
+		if (ACPI_SUCCESS(status))
+			return;
+
+		put_device(&adev->dev);
+		goto err_out;
 	default:
 		/* non-hotplug event; possibly handled by other handler */
 		return;
 	}
 	status = acpi_os_hotplug_execute(callback, handle);
-	if (ACPI_FAILURE(status))
-		acpi_evaluate_hotplug_ost(handle, type,
-					  ACPI_OST_SC_NON_SPECIFIC_FAILURE,
-					  NULL);
-}
-
-void __acpi_bus_hot_remove_device(struct acpi_device *device, u32 ost_src)
-{
-	acpi_handle handle = device->handle;
-	int error;
-
-	lock_device_hotplug();
-	mutex_lock(&acpi_scan_lock);
-
-	error = acpi_scan_hot_remove(device);
-	if (error && handle)
-		acpi_evaluate_hotplug_ost(handle, ost_src,
-					  ACPI_OST_SC_NON_SPECIFIC_FAILURE,
-					  NULL);
-
-	mutex_unlock(&acpi_scan_lock);
-	unlock_device_hotplug();
-}
+	if (ACPI_SUCCESS(status))
+		return;
 
-/**
- * acpi_bus_hot_remove_device: Hot-remove a device and its children.
- * @context: Address of the ACPI device object to hot-remove.
- */
-void acpi_bus_hot_remove_device(void *context)
-{
-	__acpi_bus_hot_remove_device(context, ACPI_NOTIFY_EJECT_REQUEST);
+ err_out:
+	acpi_evaluate_hotplug_ost(handle, type,
+				  ACPI_OST_SC_NON_SPECIFIC_FAILURE, NULL);
 }
 
 static ssize_t real_power_state_show(struct device *dev,
@@ -496,7 +488,7 @@ static DEVICE_ATTR(power_state, 0444, po
 
 static void acpi_eject_store_work(void *context)
 {
-	__acpi_bus_hot_remove_device(context, ACPI_OST_EC_OSPM_EJECT);
+	acpi_bus_device_eject(context, ACPI_OST_EC_OSPM_EJECT);
 }
 
 static ssize_t

[PATCH 0/3] More ACPI hotplug updates (was: [Update][PATCH 0/6] ACPI scan and hotplug fixes)

[Rafael J. Wysocki]

On Monday, November 04, 2013 02:29:56 PM Rafael J. Wysocki wrote:
> On Monday, November 04, 2013 01:17:12 AM Rafael J. Wysocki wrote:
> > Hi,
> > 
> > The following three patches fix some issues that we have in the common ACPI
> > hotplug infrastructure.
> > 
> > If anyone sees any problems with them, please let me know.
> 
> Well, the last patch from the last version didn't really do the right thing in
> my opinion, so new series follows.
> 
> I actually would like to squeeze it into 3.13 if there are no objections, since
> at least patch [3/6] is kind of urgent.

Three more patches for 3.13 if people don't mind, on top of the previous series
of six.  [2/3] actually is a fix, the other two are more like cleanups, but
they are worth doing at this point if no one sees any problems with them.

This is the last batch for 3.13, I promise.

Thanks!

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

[PATCH 1/3] ACPI / hotplug: Carry out PCI root eject directly

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Since _handle_hotplug_event_root() is run from the ACPI hotplug
workqueue, it doesn't need to queue up a work item to eject a PCI
host bridge on the same workqueue.  Instead, it can just carry out
the eject by calling acpi_bus_device_eject() directly, so make that
happen.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/internal.h |    2 +-
 drivers/acpi/pci_root.c |    6 +-----
 drivers/acpi/scan.c     |    4 ++--
 3 files changed, 4 insertions(+), 8 deletions(-)

Index: linux-pm/drivers/acpi/internal.h
===================================================================
--- linux-pm.orig/drivers/acpi/internal.h
+++ linux-pm/drivers/acpi/internal.h
@@ -92,7 +92,7 @@ void acpi_device_add_finalize(struct acp
 void acpi_free_pnp_ids(struct acpi_device_pnp *pnp);
 int acpi_bind_one(struct device *dev, acpi_handle handle);
 int acpi_unbind_one(struct device *dev);
-void acpi_bus_hot_remove_device(void *context);
+void acpi_bus_device_eject(struct acpi_device *device, u32 ost_src);
 
 /* --------------------------------------------------------------------------
                                   Power Resource
Index: linux-pm/drivers/acpi/pci_root.c
===================================================================
--- linux-pm.orig/drivers/acpi/pci_root.c
+++ linux-pm/drivers/acpi/pci_root.c
@@ -594,12 +594,8 @@ static void handle_root_bridge_insertion
 
 static void handle_root_bridge_removal(struct acpi_device *device)
 {
-	acpi_status status;
-
 	get_device(&device->dev);
-	status = acpi_os_hotplug_execute(acpi_bus_hot_remove_device, device);
-	if (ACPI_FAILURE(status))
-		put_device(&device->dev);
+	acpi_bus_device_eject(device, ACPI_NOTIFY_EJECT_REQUEST);
 }
 
 static void _handle_hotplug_event_root(struct work_struct *work)
Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -286,7 +286,7 @@ static int acpi_scan_hot_remove(struct a
 	return 0;
 }
 
-static void acpi_bus_device_eject(struct acpi_device *device, u32 ost_src)
+void acpi_bus_device_eject(struct acpi_device *device, u32 ost_src)
 {
 	acpi_handle handle = device->handle;
 	struct acpi_scan_handler *handler;
@@ -410,7 +410,7 @@ static void acpi_hotplug_unsupported(acp
  * acpi_bus_hot_remove_device: Hot-remove a device and its children.
  * @context: Address of the ACPI device object to hot-remove.
  */
-void acpi_bus_hot_remove_device(void *context)
+static void acpi_bus_hot_remove_device(void *context)
 {
 	acpi_bus_device_eject(context, ACPI_NOTIFY_EJECT_REQUEST);
 }

[Update][PATCH 1/3] ACPI / hotplug: Carry out PCI root eject directly

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Subject: ACPI / hotplug: Carry out PCI root eject directly

Since _handle_hotplug_event_root() is run from the ACPI hotplug
workqueue, it doesn't need to queue up a work item to eject a PCI
host bridge on the same workqueue.  Instead, it can just carry out
the eject by calling acpi_bus_device_eject() directly, so make that
happen.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

It looks like I sent this too early. :-/

Obviously, acpi_bus_device_eject() cannot be called under acpi_scan_lock,
so that lock has to be dropped before calling that function.

Patch [3/3] doesn't apply any more after this modification, so I'll send
an update of it too.

Thanks,
Rafael

---
 drivers/acpi/internal.h |    2 +-
 drivers/acpi/pci_root.c |   24 ++++++++++--------------
 drivers/acpi/scan.c     |    4 ++--
 3 files changed, 13 insertions(+), 17 deletions(-)

Index: linux-pm/drivers/acpi/internal.h
===================================================================
--- linux-pm.orig/drivers/acpi/internal.h
+++ linux-pm/drivers/acpi/internal.h
@@ -92,7 +92,7 @@ void acpi_device_add_finalize(struct acp
 void acpi_free_pnp_ids(struct acpi_device_pnp *pnp);
 int acpi_bind_one(struct device *dev, acpi_handle handle);
 int acpi_unbind_one(struct device *dev);
-void acpi_bus_hot_remove_device(void *context);
+void acpi_bus_device_eject(struct acpi_device *device, u32 ost_src);
 
 /* --------------------------------------------------------------------------
                                   Power Resource
Index: linux-pm/drivers/acpi/pci_root.c
===================================================================
--- linux-pm.orig/drivers/acpi/pci_root.c
+++ linux-pm/drivers/acpi/pci_root.c
@@ -592,16 +592,6 @@ static void handle_root_bridge_insertion
 		acpi_handle_err(handle, "cannot add bridge to acpi list\n");
 }
 
-static void handle_root_bridge_removal(struct acpi_device *device)
-{
-	acpi_status status;
-
-	get_device(&device->dev);
-	status = acpi_os_hotplug_execute(acpi_bus_hot_remove_device, device);
-	if (ACPI_FAILURE(status))
-		put_device(&device->dev);
-}
-
 static void _handle_hotplug_event_root(struct work_struct *work)
 {
 	struct acpi_pci_root *root;
@@ -612,6 +602,7 @@ static void _handle_hotplug_event_root(s
 	hp_work = container_of(work, struct acpi_hp_work, work);
 	handle = hp_work->handle;
 	type = hp_work->type;
+	kfree(hp_work); /* allocated in handle_hotplug_event_bridge */
 
 	acpi_scan_lock_acquire();
 
@@ -641,9 +632,15 @@ static void _handle_hotplug_event_root(s
 		/* request device eject */
 		acpi_handle_printk(KERN_DEBUG, handle,
 				   "Device eject notify on %s\n", __func__);
-		if (root)
-			handle_root_bridge_removal(root->device);
-		break;
+		if (!root)
+			break;
+
+		get_device(&root->device->dev);
+
+		acpi_scan_lock_release();
+
+		acpi_bus_device_eject(root->device, ACPI_NOTIFY_EJECT_REQUEST);
+		return;
 	default:
 		acpi_handle_warn(handle,
 				 "notify_handler: unknown event type 0x%x\n",
@@ -652,7 +649,6 @@ static void _handle_hotplug_event_root(s
 	}
 
 	acpi_scan_lock_release();
-	kfree(hp_work); /* allocated in handle_hotplug_event_bridge */
 }
 
 static void handle_hotplug_event_root(acpi_handle handle, u32 type,
Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -285,7 +285,7 @@ static int acpi_scan_hot_remove(struct a
 	return 0;
 }
 
-static void acpi_bus_device_eject(struct acpi_device *device, u32 ost_src)
+void acpi_bus_device_eject(struct acpi_device *device, u32 ost_src)
 {
 	acpi_handle handle = device->handle;
 	struct acpi_scan_handler *handler;
@@ -409,7 +409,7 @@ static void acpi_hotplug_unsupported(acp
  * acpi_bus_hot_remove_device: Hot-remove a device and its children.
  * @context: Address of the ACPI device object to hot-remove.
  */
-void acpi_bus_hot_remove_device(void *context)
+static void acpi_bus_hot_remove_device(void *context)
 {
 	acpi_bus_device_eject(context, ACPI_NOTIFY_EJECT_REQUEST);
 }

[PATCH 2/3] ACPI / hotplug: Do not execute "insert in progress" _OST

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

According to the ACPI spec (5.0, Section 6.3.5), the "Device
insertion in progress (pending)" (0x80) _OST status code is
reserved for the "Insertion Processing" (0x200) source event
which is "a result of an OSPM action".  Specifically, it is not
a notification, so that status code should not be used during
notification processing, which unfortunately is done by
acpi_scan_bus_device_check().

For this reason, drop the ACPI_OST_SC_INSERT_IN_PROGRESS _OST
status evaluation from there (it was a mistake to put it in there
in the first place).

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

Material for stable, in my opinion.

Thanks,
Rafael

---
 drivers/acpi/scan.c |    2 --
 1 file changed, 2 deletions(-)

Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -344,8 +344,6 @@ static void acpi_scan_bus_device_check(a
 			goto out;
 		}
 	}
-	acpi_evaluate_hotplug_ost(handle, ost_source,
-				  ACPI_OST_SC_INSERT_IN_PROGRESS, NULL);
 	error = acpi_bus_scan(handle);
 	if (error) {
 		acpi_handle_warn(handle, "Namespace scan failure\n");

[PATCH 3/3] ACPI / hotplug: Consolidate deferred execution of ACPI hotplug routines

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

There are two different interfaces for queuing up work items on the
ACPI hotplug workqueue, alloc_acpi_hp_work() used by PCI and PCI host
bridge hotplug code and acpi_os_hotplug_execute() used by the common
ACPI hotplug code and docking stations.  They both are somewhat
cumbersome to use and work slightly differently.

The users of alloc_acpi_hp_work() have to submit a work function that
will extract the necessary data items from a struct acpi_hp_work
object allocated by alloc_acpi_hp_work() and then will free that
object, while it would be more straightforward to simply use a work
function with one more argument and let the interface take care of
the execution details.

The users of acpi_os_hotplug_execute() also have to deal with the
fact that it takes only one argument in addition to the work function
pointer, although acpi_os_execute_deferred() actually takes care of
the allocation and freeing of memory, so it would have been able to
pass more arguments to the work function if it hadn't been
constrained by the connection with acpi_os_execute().

Moreover, while alloc_acpi_hp_work() makes GFP_KERNEL memory
allocations, which is correct, because hotplug work items are
always queued up from process context, acpi_os_hotplug_execute()
uses GFP_ATOMIC, as that is needed by acpi_os_execute().  Also,
acpi_os_execute_deferred() queued up by it waits for the ACPI event
workqueues to flush before calling the work function, while
alloc_acpi_hp_work() obviously can't do anything like that.  That
leads to somewhat arbitrary differences in behavior between various
ACPI hotplug code paths and has to be straightened up.

For this reason, replace both alloc_acpi_hp_work() and
acpi_os_hotplug_execute() with a single interface,
acpi_hotplug_execute(), combining their behavior and being more
friendly to its users than any of the two.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/dock.c                |   25 +--------
 drivers/acpi/internal.h            |    2 
 drivers/acpi/osl.c                 |   96 +++++++++++++++++--------------------
 drivers/acpi/pci_root.c            |   14 +----
 drivers/acpi/scan.c                |   43 +++-------------
 drivers/pci/hotplug/acpiphp_glue.c |   18 ++----
 include/acpi/acpi_bus.h            |   12 +---
 include/acpi/platform/aclinux.h    |    3 -
 8 files changed, 71 insertions(+), 142 deletions(-)

Index: linux-pm/drivers/acpi/osl.c
===================================================================
--- linux-pm.orig/drivers/acpi/osl.c
+++ linux-pm/drivers/acpi/osl.c
@@ -61,7 +61,6 @@ struct acpi_os_dpc {
 	acpi_osd_exec_callback function;
 	void *context;
 	struct work_struct work;
-	int wait;
 };
 
 #ifdef CONFIG_ACPI_CUSTOM_DSDT
@@ -1087,9 +1086,6 @@ static void acpi_os_execute_deferred(str
 {
 	struct acpi_os_dpc *dpc = container_of(work, struct acpi_os_dpc, work);
 
-	if (dpc->wait)
-		acpi_os_wait_events_complete();
-
 	dpc->function(dpc->context);
 	kfree(dpc);
 }
@@ -1109,8 +1105,8 @@ static void acpi_os_execute_deferred(str
  *
  ******************************************************************************/
 
-static acpi_status __acpi_os_execute(acpi_execute_type type,
-	acpi_osd_exec_callback function, void *context, int hp)
+acpi_status acpi_os_execute(acpi_execute_type type,
+			    acpi_osd_exec_callback function, void *context)
 {
 	acpi_status status = AE_OK;
 	struct acpi_os_dpc *dpc;
@@ -1137,20 +1133,11 @@ static acpi_status __acpi_os_execute(acp
 	dpc->context = context;
 
 	/*
-	 * We can't run hotplug code in keventd_wq/kacpid_wq/kacpid_notify_wq
-	 * because the hotplug code may call driver .remove() functions,
-	 * which invoke flush_scheduled_work/acpi_os_wait_events_complete
-	 * to flush these workqueues.
-	 *
 	 * To prevent lockdep from complaining unnecessarily, make sure that
 	 * there is a different static lockdep key for each workqueue by using
 	 * INIT_WORK() for each of them separately.
 	 */
-	if (hp) {
-		queue = kacpi_hotplug_wq;
-		dpc->wait = 1;
-		INIT_WORK(&dpc->work, acpi_os_execute_deferred);
-	} else if (type == OSL_NOTIFY_HANDLER) {
+	if (type == OSL_NOTIFY_HANDLER) {
 		queue = kacpi_notify_wq;
 		INIT_WORK(&dpc->work, acpi_os_execute_deferred);
 	} else {
@@ -1175,28 +1162,59 @@ static acpi_status __acpi_os_execute(acp
 	}
 	return status;
 }
+EXPORT_SYMBOL(acpi_os_execute);
 
-acpi_status acpi_os_execute(acpi_execute_type type,
-			    acpi_osd_exec_callback function, void *context)
+void acpi_os_wait_events_complete(void)
 {
-	return __acpi_os_execute(type, function, context, 0);
+	flush_workqueue(kacpid_wq);
+	flush_workqueue(kacpi_notify_wq);
 }
-EXPORT_SYMBOL(acpi_os_execute);
 
-acpi_status acpi_os_hotplug_execute(acpi_osd_exec_callback function,
-	void *context)
+struct acpi_hp_work {
+	struct work_struct work;
+	acpi_hp_callback func;
+	void *data;
+	u32 src;
+};
+
+static void acpi_hotplug_work_fn(struct work_struct *work)
 {
-	return __acpi_os_execute(0, function, context, 1);
+	struct acpi_hp_work *hpw = container_of(work, struct acpi_hp_work, work);
+
+	acpi_os_wait_events_complete();
+	hpw->func(hpw->data, hpw->src);
+	kfree(hpw);
 }
-EXPORT_SYMBOL(acpi_os_hotplug_execute);
 
-void acpi_os_wait_events_complete(void)
+acpi_status acpi_hotplug_execute(acpi_hp_callback func, void *data, u32 src)
 {
-	flush_workqueue(kacpid_wq);
-	flush_workqueue(kacpi_notify_wq);
+	struct acpi_hp_work *hpw;
+
+	ACPI_DEBUG_PRINT((ACPI_DB_EXEC,
+		  "Scheduling function [%p(%p, %u)] for deferred execution.\n",
+		  func, data, src));
+
+	hpw = kmalloc(sizeof(*hpw), GFP_KERNEL);
+	if (!hpw)
+		return AE_NO_MEMORY;
+
+	INIT_WORK(&hpw->work, acpi_hotplug_work_fn);
+	hpw->func = func;
+	hpw->data = data;
+	hpw->src = src;
+	/*
+	 * We can't run hotplug code in kacpid_wq/kacpid_notify_wq etc., because
+	 * the hotplug code may call driver .remove() functions, which may
+	 * invoke flush_scheduled_work()/acpi_os_wait_events_complete() to flush
+	 * these workqueues.
+	 */
+	if (!queue_work(kacpi_hotplug_wq, &hpw->work)) {
+		kfree(hpw);
+		return AE_ERROR;
+	}
+	return AE_OK;
 }
 
-EXPORT_SYMBOL(acpi_os_wait_events_complete);
 
 acpi_status
 acpi_os_create_semaphore(u32 max_units, u32 initial_units, acpi_handle * handle)
@@ -1845,25 +1863,3 @@ void acpi_os_set_prepare_extended_sleep(
 {
 	__acpi_os_prepare_extended_sleep = func;
 }
-
-
-void alloc_acpi_hp_work(acpi_handle handle, u32 type, void *context,
-			void (*func)(struct work_struct *work))
-{
-	struct acpi_hp_work *hp_work;
-	int ret;
-
-	hp_work = kmalloc(sizeof(*hp_work), GFP_KERNEL);
-	if (!hp_work)
-		return;
-
-	hp_work->handle = handle;
-	hp_work->type = type;
-	hp_work->context = context;
-
-	INIT_WORK(&hp_work->work, func);
-	ret = queue_work(kacpi_hotplug_wq, &hp_work->work);
-	if (!ret)
-		kfree(hp_work);
-}
-EXPORT_SYMBOL_GPL(alloc_acpi_hp_work);
Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -339,15 +339,6 @@ struct acpi_bus_event {
 	u32 data;
 };
 
-struct acpi_hp_work {
-	struct work_struct work;
-	acpi_handle handle;
-	u32 type;
-	void *context;
-};
-void alloc_acpi_hp_work(acpi_handle handle, u32 type, void *context,
-			void (*func)(struct work_struct *work));
-
 extern struct kobject *acpi_kobj;
 extern int acpi_bus_generate_netlink_event(const char*, const char*, u8, int);
 void acpi_bus_private_data_handler(acpi_handle, void *);
@@ -393,6 +384,9 @@ int acpi_match_device_ids(struct acpi_de
 int acpi_create_dir(struct acpi_device *);
 void acpi_remove_dir(struct acpi_device *);
 
+typedef void (*acpi_hp_callback)(void *data, u32 src);
+
+acpi_status acpi_hotplug_execute(acpi_hp_callback func, void *data, u32 src);
 
 /**
  * module_acpi_driver(acpi_driver) - Helper macro for registering an ACPI driver
Index: linux-pm/include/acpi/platform/aclinux.h
===================================================================
--- linux-pm.orig/include/acpi/platform/aclinux.h
+++ linux-pm/include/acpi/platform/aclinux.h
@@ -243,9 +243,6 @@ void acpi_os_gpe_count(u32 gpe_number);
 
 void acpi_os_fixed_event_count(u32 fixed_event_number);
 
-acpi_status
-acpi_os_hotplug_execute(acpi_osd_exec_callback function, void *context);
-
 #endif				/* __KERNEL__ */
 
 #endif				/* __ACLINUX_H__ */
Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -286,8 +286,9 @@ static int acpi_scan_hot_remove(struct a
 	return 0;
 }
 
-void acpi_bus_device_eject(struct acpi_device *device, u32 ost_src)
+void acpi_bus_device_eject(void *data, u32 ost_src)
 {
+	struct acpi_device *device = data;
 	acpi_handle handle = device->handle;
 	struct acpi_scan_handler *handler;
 	u32 ost_code = ACPI_OST_SC_NON_SPECIFIC_FAILURE;
@@ -328,8 +329,9 @@ void acpi_bus_device_eject(struct acpi_d
 	goto out;
 }
 
-static void acpi_scan_bus_device_check(acpi_handle handle, u32 ost_source)
+static void acpi_scan_bus_device_check(void *data, u32 ost_source)
 {
+	acpi_handle handle = data;
 	struct acpi_device *device = NULL;
 	u32 ost_code = ACPI_OST_SC_NON_SPECIFIC_FAILURE;
 	int error;
@@ -364,18 +366,6 @@ static void acpi_scan_bus_device_check(a
 	unlock_device_hotplug();
 }
 
-static void acpi_scan_bus_check(void *context)
-{
-	acpi_scan_bus_device_check((acpi_handle)context,
-				   ACPI_NOTIFY_BUS_CHECK);
-}
-
-static void acpi_scan_device_check(void *context)
-{
-	acpi_scan_bus_device_check((acpi_handle)context,
-				   ACPI_NOTIFY_DEVICE_CHECK);
-}
-
 static void acpi_hotplug_unsupported(acpi_handle handle, u32 type)
 {
 	u32 ost_status;
@@ -404,18 +394,8 @@ static void acpi_hotplug_unsupported(acp
 	acpi_evaluate_hotplug_ost(handle, type, ost_status, NULL);
 }
 
-/**
- * acpi_bus_hot_remove_device: Hot-remove a device and its children.
- * @context: Address of the ACPI device object to hot-remove.
- */
-static void acpi_bus_hot_remove_device(void *context)
-{
-	acpi_bus_device_eject(context, ACPI_NOTIFY_EJECT_REQUEST);
-}
-
 static void acpi_hotplug_notify_cb(acpi_handle handle, u32 type, void *data)
 {
-	acpi_osd_exec_callback callback;
 	struct acpi_scan_handler *handler = data;
 	struct acpi_device *adev;
 	acpi_status status;
@@ -426,11 +406,9 @@ static void acpi_hotplug_notify_cb(acpi_
 	switch (type) {
 	case ACPI_NOTIFY_BUS_CHECK:
 		acpi_handle_debug(handle, "ACPI_NOTIFY_BUS_CHECK event\n");
-		callback = acpi_scan_bus_check;
 		break;
 	case ACPI_NOTIFY_DEVICE_CHECK:
 		acpi_handle_debug(handle, "ACPI_NOTIFY_DEVICE_CHECK event\n");
-		callback = acpi_scan_device_check;
 		break;
 	case ACPI_NOTIFY_EJECT_REQUEST:
 		acpi_handle_debug(handle, "ACPI_NOTIFY_EJECT_REQUEST event\n");
@@ -439,8 +417,7 @@ static void acpi_hotplug_notify_cb(acpi_
 			goto err_out;
 
 		get_device(&adev->dev);
-		callback = acpi_bus_hot_remove_device;
-		status = acpi_os_hotplug_execute(callback, adev);
+		status = acpi_hotplug_execute(acpi_bus_device_eject, adev, type);
 		if (ACPI_SUCCESS(status))
 			return;
 
@@ -450,7 +427,7 @@ static void acpi_hotplug_notify_cb(acpi_
 		/* non-hotplug event; possibly handled by other handler */
 		return;
 	}
-	status = acpi_os_hotplug_execute(callback, handle);
+	status = acpi_hotplug_execute(acpi_scan_bus_device_check, handle, type);
 	if (ACPI_SUCCESS(status))
 		return;
 
@@ -485,11 +462,6 @@ static ssize_t power_state_show(struct d
 
 static DEVICE_ATTR(power_state, 0444, power_state_show, NULL);
 
-static void acpi_eject_store_work(void *context)
-{
-	acpi_bus_device_eject(context, ACPI_OST_EC_OSPM_EJECT);
-}
-
 static ssize_t
 acpi_eject_store(struct device *d, struct device_attribute *attr,
 		const char *buf, size_t count)
@@ -512,7 +484,8 @@ acpi_eject_store(struct device *d, struc
 	acpi_evaluate_hotplug_ost(acpi_device->handle, ACPI_OST_EC_OSPM_EJECT,
 				  ACPI_OST_SC_EJECT_IN_PROGRESS, NULL);
 	get_device(&acpi_device->dev);
-	status = acpi_os_hotplug_execute(acpi_eject_store_work, acpi_device);
+	status = acpi_hotplug_execute(acpi_bus_device_eject, acpi_device,
+				      ACPI_OST_EC_OSPM_EJECT);
 	if (ACPI_SUCCESS(status))
 		return count;
 
Index: linux-pm/drivers/acpi/pci_root.c
===================================================================
--- linux-pm.orig/drivers/acpi/pci_root.c
+++ linux-pm/drivers/acpi/pci_root.c
@@ -598,16 +598,10 @@ static void handle_root_bridge_removal(s
 	acpi_bus_device_eject(device, ACPI_NOTIFY_EJECT_REQUEST);
 }
 
-static void _handle_hotplug_event_root(struct work_struct *work)
+static void hotplug_event_root(void *data, u32 type)
 {
+	acpi_handle handle = data;
 	struct acpi_pci_root *root;
-	struct acpi_hp_work *hp_work;
-	acpi_handle handle;
-	u32 type;
-
-	hp_work = container_of(work, struct acpi_hp_work, work);
-	handle = hp_work->handle;
-	type = hp_work->type;
 
 	acpi_scan_lock_acquire();
 
@@ -648,14 +642,12 @@ static void _handle_hotplug_event_root(s
 	}
 
 	acpi_scan_lock_release();
-	kfree(hp_work); /* allocated in handle_hotplug_event_bridge */
 }
 
 static void handle_hotplug_event_root(acpi_handle handle, u32 type,
 					void *context)
 {
-	alloc_acpi_hp_work(handle, type, context,
-				_handle_hotplug_event_root);
+	acpi_hotplug_execute(hotplug_event_root, handle, type);
 }
 
 static acpi_status __init
Index: linux-pm/drivers/acpi/dock.c
===================================================================
--- linux-pm.orig/drivers/acpi/dock.c
+++ linux-pm/drivers/acpi/dock.c
@@ -669,39 +669,20 @@ static void dock_notify(struct dock_stat
 	}
 }
 
-struct dock_data {
-	struct dock_station *ds;
-	u32 event;
-};
-
-static void acpi_dock_deferred_cb(void *context)
+static void acpi_dock_deferred_cb(void *data, u32 event)
 {
-	struct dock_data *data = context;
-
 	acpi_scan_lock_acquire();
-	dock_notify(data->ds, data->event);
+	dock_notify(data, event);
 	acpi_scan_lock_release();
-	kfree(data);
 }
 
 static void dock_notify_handler(acpi_handle handle, u32 event, void *data)
 {
-	struct dock_data *dd;
-
 	if (event != ACPI_NOTIFY_BUS_CHECK && event != ACPI_NOTIFY_DEVICE_CHECK
 	   && event != ACPI_NOTIFY_EJECT_REQUEST)
 		return;
 
-	dd = kmalloc(sizeof(*dd), GFP_KERNEL);
-	if (dd) {
-		acpi_status status;
-
-		dd->ds = data;
-		dd->event = event;
-		status = acpi_os_hotplug_execute(acpi_dock_deferred_cb, dd);
-		if (ACPI_FAILURE(status))
-			kfree(dd);
-	}
+	acpi_hotplug_execute(acpi_dock_deferred_cb, data, event);
 }
 
 /**
Index: linux-pm/drivers/pci/hotplug/acpiphp_glue.c
===================================================================
--- linux-pm.orig/drivers/pci/hotplug/acpiphp_glue.c
+++ linux-pm/drivers/pci/hotplug/acpiphp_glue.c
@@ -871,21 +871,17 @@ static void hotplug_event(acpi_handle ha
 		put_bridge(bridge);
 }
 
-static void hotplug_event_work(struct work_struct *work)
+static void hotplug_event_work(void *data, u32 type)
 {
-	struct acpiphp_context *context;
-	struct acpi_hp_work *hp_work;
+	struct acpiphp_context *context = data;
+	acpi_handle handle = context->handle;
 
-	hp_work = container_of(work, struct acpi_hp_work, work);
-	context = hp_work->context;
 	acpi_scan_lock_acquire();
 
-	hotplug_event(hp_work->handle, hp_work->type, context);
+	hotplug_event(handle, type, context);
 
 	acpi_scan_lock_release();
-	acpi_evaluate_hotplug_ost(hp_work->handle, hp_work->type,
-				  ACPI_OST_SC_SUCCESS, NULL);
-	kfree(hp_work); /* allocated in handle_hotplug_event() */
+	acpi_evaluate_hotplug_ost(handle, type, ACPI_OST_SC_SUCCESS, NULL);
 	put_bridge(context->func.parent);
 }
 
@@ -936,10 +932,10 @@ static void handle_hotplug_event(acpi_ha
 
 	mutex_lock(&acpiphp_context_lock);
 	context = acpiphp_get_context(handle);
-	if (context) {
+	if (context && !WARN_ON(context->handle != handle)) {
 		get_bridge(context->func.parent);
 		acpiphp_put_context(context);
-		alloc_acpi_hp_work(handle, type, context, hotplug_event_work);
+		acpi_hotplug_execute(hotplug_event_work, context, type);
 		mutex_unlock(&acpiphp_context_lock);
 		return;
 	}
Index: linux-pm/drivers/acpi/internal.h
===================================================================
--- linux-pm.orig/drivers/acpi/internal.h
+++ linux-pm/drivers/acpi/internal.h
@@ -92,7 +92,7 @@ void acpi_device_add_finalize(struct acp
 void acpi_free_pnp_ids(struct acpi_device_pnp *pnp);
 int acpi_bind_one(struct device *dev, acpi_handle handle);
 int acpi_unbind_one(struct device *dev);
-void acpi_bus_device_eject(struct acpi_device *device, u32 ost_src);
+void acpi_bus_device_eject(void *data, u32 ost_src);
 
 /* --------------------------------------------------------------------------
                                   Power Resource

[Update][PATCH 3/3] ACPI / hotplug: Consolidate deferred execution of ACPI hotplug routines

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

There are two different interfaces for queuing up work items on the
ACPI hotplug workqueue, alloc_acpi_hp_work() used by PCI and PCI host
bridge hotplug code and acpi_os_hotplug_execute() used by the common
ACPI hotplug code and docking stations.  They both are somewhat
cumbersome to use and work slightly differently.

The users of alloc_acpi_hp_work() have to submit a work function that
will extract the necessary data items from a struct acpi_hp_work
object allocated by alloc_acpi_hp_work() and then will free that
object, while it would be more straightforward to simply use a work
function with one more argument and let the interface take care of
the execution details.

The users of acpi_os_hotplug_execute() also have to deal with the
fact that it takes only one argument in addition to the work function
pointer, although acpi_os_execute_deferred() actually takes care of
the allocation and freeing of memory, so it would have been able to
pass more arguments to the work function if it hadn't been
constrained by the connection with acpi_os_execute().

Moreover, while alloc_acpi_hp_work() makes GFP_KERNEL memory
allocations, which is correct, because hotplug work items are
always queued up from process context, acpi_os_hotplug_execute()
uses GFP_ATOMIC, as that is needed by acpi_os_execute().  Also,
acpi_os_execute_deferred() queued up by it waits for the ACPI event
workqueues to flush before executing the work function, whereas
alloc_acpi_hp_work() can't do anything similar.  That leads to
somewhat arbitrary differences in behavior between various ACPI
hotplug code paths and has to be straightened up.

For this reason, replace both alloc_acpi_hp_work() and
acpi_os_hotplug_execute() with a single interface,
acpi_hotplug_execute(), combining their behavior and being more
friendly to its users than any of the two.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

Updated after the fixup of patch [1/3].

Thanks,
Rafael

---
 drivers/acpi/dock.c                |   25 +--------
 drivers/acpi/internal.h            |    2 
 drivers/acpi/osl.c                 |   96 +++++++++++++++++--------------------
 drivers/acpi/pci_root.c            |   14 +----
 drivers/acpi/scan.c                |   43 +++-------------
 drivers/pci/hotplug/acpiphp_glue.c |   18 ++----
 include/acpi/acpi_bus.h            |   12 +---
 include/acpi/platform/aclinux.h    |    3 -
 8 files changed, 71 insertions(+), 142 deletions(-)

Index: linux-pm/drivers/acpi/osl.c
===================================================================
--- linux-pm.orig/drivers/acpi/osl.c
+++ linux-pm/drivers/acpi/osl.c
@@ -61,7 +61,6 @@ struct acpi_os_dpc {
 	acpi_osd_exec_callback function;
 	void *context;
 	struct work_struct work;
-	int wait;
 };
 
 #ifdef CONFIG_ACPI_CUSTOM_DSDT
@@ -1087,9 +1086,6 @@ static void acpi_os_execute_deferred(str
 {
 	struct acpi_os_dpc *dpc = container_of(work, struct acpi_os_dpc, work);
 
-	if (dpc->wait)
-		acpi_os_wait_events_complete();
-
 	dpc->function(dpc->context);
 	kfree(dpc);
 }
@@ -1109,8 +1105,8 @@ static void acpi_os_execute_deferred(str
  *
  ******************************************************************************/
 
-static acpi_status __acpi_os_execute(acpi_execute_type type,
-	acpi_osd_exec_callback function, void *context, int hp)
+acpi_status acpi_os_execute(acpi_execute_type type,
+			    acpi_osd_exec_callback function, void *context)
 {
 	acpi_status status = AE_OK;
 	struct acpi_os_dpc *dpc;
@@ -1137,20 +1133,11 @@ static acpi_status __acpi_os_execute(acp
 	dpc->context = context;
 
 	/*
-	 * We can't run hotplug code in keventd_wq/kacpid_wq/kacpid_notify_wq
-	 * because the hotplug code may call driver .remove() functions,
-	 * which invoke flush_scheduled_work/acpi_os_wait_events_complete
-	 * to flush these workqueues.
-	 *
 	 * To prevent lockdep from complaining unnecessarily, make sure that
 	 * there is a different static lockdep key for each workqueue by using
 	 * INIT_WORK() for each of them separately.
 	 */
-	if (hp) {
-		queue = kacpi_hotplug_wq;
-		dpc->wait = 1;
-		INIT_WORK(&dpc->work, acpi_os_execute_deferred);
-	} else if (type == OSL_NOTIFY_HANDLER) {
+	if (type == OSL_NOTIFY_HANDLER) {
 		queue = kacpi_notify_wq;
 		INIT_WORK(&dpc->work, acpi_os_execute_deferred);
 	} else {
@@ -1175,28 +1162,59 @@ static acpi_status __acpi_os_execute(acp
 	}
 	return status;
 }
+EXPORT_SYMBOL(acpi_os_execute);
 
-acpi_status acpi_os_execute(acpi_execute_type type,
-			    acpi_osd_exec_callback function, void *context)
+void acpi_os_wait_events_complete(void)
 {
-	return __acpi_os_execute(type, function, context, 0);
+	flush_workqueue(kacpid_wq);
+	flush_workqueue(kacpi_notify_wq);
 }
-EXPORT_SYMBOL(acpi_os_execute);
 
-acpi_status acpi_os_hotplug_execute(acpi_osd_exec_callback function,
-	void *context)
+struct acpi_hp_work {
+	struct work_struct work;
+	acpi_hp_callback func;
+	void *data;
+	u32 src;
+};
+
+static void acpi_hotplug_work_fn(struct work_struct *work)
 {
-	return __acpi_os_execute(0, function, context, 1);
+	struct acpi_hp_work *hpw = container_of(work, struct acpi_hp_work, work);
+
+	acpi_os_wait_events_complete();
+	hpw->func(hpw->data, hpw->src);
+	kfree(hpw);
 }
-EXPORT_SYMBOL(acpi_os_hotplug_execute);
 
-void acpi_os_wait_events_complete(void)
+acpi_status acpi_hotplug_execute(acpi_hp_callback func, void *data, u32 src)
 {
-	flush_workqueue(kacpid_wq);
-	flush_workqueue(kacpi_notify_wq);
+	struct acpi_hp_work *hpw;
+
+	ACPI_DEBUG_PRINT((ACPI_DB_EXEC,
+		  "Scheduling function [%p(%p, %u)] for deferred execution.\n",
+		  func, data, src));
+
+	hpw = kmalloc(sizeof(*hpw), GFP_KERNEL);
+	if (!hpw)
+		return AE_NO_MEMORY;
+
+	INIT_WORK(&hpw->work, acpi_hotplug_work_fn);
+	hpw->func = func;
+	hpw->data = data;
+	hpw->src = src;
+	/*
+	 * We can't run hotplug code in kacpid_wq/kacpid_notify_wq etc., because
+	 * the hotplug code may call driver .remove() functions, which may
+	 * invoke flush_scheduled_work()/acpi_os_wait_events_complete() to flush
+	 * these workqueues.
+	 */
+	if (!queue_work(kacpi_hotplug_wq, &hpw->work)) {
+		kfree(hpw);
+		return AE_ERROR;
+	}
+	return AE_OK;
 }
 
-EXPORT_SYMBOL(acpi_os_wait_events_complete);
 
 acpi_status
 acpi_os_create_semaphore(u32 max_units, u32 initial_units, acpi_handle * handle)
@@ -1845,25 +1863,3 @@ void acpi_os_set_prepare_extended_sleep(
 {
 	__acpi_os_prepare_extended_sleep = func;
 }
-
-
-void alloc_acpi_hp_work(acpi_handle handle, u32 type, void *context,
-			void (*func)(struct work_struct *work))
-{
-	struct acpi_hp_work *hp_work;
-	int ret;
-
-	hp_work = kmalloc(sizeof(*hp_work), GFP_KERNEL);
-	if (!hp_work)
-		return;
-
-	hp_work->handle = handle;
-	hp_work->type = type;
-	hp_work->context = context;
-
-	INIT_WORK(&hp_work->work, func);
-	ret = queue_work(kacpi_hotplug_wq, &hp_work->work);
-	if (!ret)
-		kfree(hp_work);
-}
-EXPORT_SYMBOL_GPL(alloc_acpi_hp_work);
Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -339,15 +339,6 @@ struct acpi_bus_event {
 	u32 data;
 };
 
-struct acpi_hp_work {
-	struct work_struct work;
-	acpi_handle handle;
-	u32 type;
-	void *context;
-};
-void alloc_acpi_hp_work(acpi_handle handle, u32 type, void *context,
-			void (*func)(struct work_struct *work));
-
 extern struct kobject *acpi_kobj;
 extern int acpi_bus_generate_netlink_event(const char*, const char*, u8, int);
 void acpi_bus_private_data_handler(acpi_handle, void *);
@@ -393,6 +384,9 @@ int acpi_match_device_ids(struct acpi_de
 int acpi_create_dir(struct acpi_device *);
 void acpi_remove_dir(struct acpi_device *);
 
+typedef void (*acpi_hp_callback)(void *data, u32 src);
+
+acpi_status acpi_hotplug_execute(acpi_hp_callback func, void *data, u32 src);
 
 /**
  * module_acpi_driver(acpi_driver) - Helper macro for registering an ACPI driver
Index: linux-pm/include/acpi/platform/aclinux.h
===================================================================
--- linux-pm.orig/include/acpi/platform/aclinux.h
+++ linux-pm/include/acpi/platform/aclinux.h
@@ -243,9 +243,6 @@ void acpi_os_gpe_count(u32 gpe_number);
 
 void acpi_os_fixed_event_count(u32 fixed_event_number);
 
-acpi_status
-acpi_os_hotplug_execute(acpi_osd_exec_callback function, void *context);
-
 #endif				/* __KERNEL__ */
 
 #endif				/* __ACLINUX_H__ */
Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -285,8 +285,9 @@ static int acpi_scan_hot_remove(struct a
 	return 0;
 }
 
-void acpi_bus_device_eject(struct acpi_device *device, u32 ost_src)
+void acpi_bus_device_eject(void *data, u32 ost_src)
 {
+	struct acpi_device *device = data;
 	acpi_handle handle = device->handle;
 	struct acpi_scan_handler *handler;
 	u32 ost_code = ACPI_OST_SC_NON_SPECIFIC_FAILURE;
@@ -327,8 +328,9 @@ void acpi_bus_device_eject(struct acpi_d
 	goto out;
 }
 
-static void acpi_scan_bus_device_check(acpi_handle handle, u32 ost_source)
+static void acpi_scan_bus_device_check(void *data, u32 ost_source)
 {
+	acpi_handle handle = data;
 	struct acpi_device *device = NULL;
 	u32 ost_code = ACPI_OST_SC_NON_SPECIFIC_FAILURE;
 	int error;
@@ -363,18 +365,6 @@ static void acpi_scan_bus_device_check(a
 	unlock_device_hotplug();
 }
 
-static void acpi_scan_bus_check(void *context)
-{
-	acpi_scan_bus_device_check((acpi_handle)context,
-				   ACPI_NOTIFY_BUS_CHECK);
-}
-
-static void acpi_scan_device_check(void *context)
-{
-	acpi_scan_bus_device_check((acpi_handle)context,
-				   ACPI_NOTIFY_DEVICE_CHECK);
-}
-
 static void acpi_hotplug_unsupported(acpi_handle handle, u32 type)
 {
 	u32 ost_status;
@@ -403,18 +393,8 @@ static void acpi_hotplug_unsupported(acp
 	acpi_evaluate_hotplug_ost(handle, type, ost_status, NULL);
 }
 
-/**
- * acpi_bus_hot_remove_device: Hot-remove a device and its children.
- * @context: Address of the ACPI device object to hot-remove.
- */
-static void acpi_bus_hot_remove_device(void *context)
-{
-	acpi_bus_device_eject(context, ACPI_NOTIFY_EJECT_REQUEST);
-}
-
 static void acpi_hotplug_notify_cb(acpi_handle handle, u32 type, void *data)
 {
-	acpi_osd_exec_callback callback;
 	struct acpi_scan_handler *handler = data;
 	struct acpi_device *adev;
 	acpi_status status;
@@ -425,11 +405,9 @@ static void acpi_hotplug_notify_cb(acpi_
 	switch (type) {
 	case ACPI_NOTIFY_BUS_CHECK:
 		acpi_handle_debug(handle, "ACPI_NOTIFY_BUS_CHECK event\n");
-		callback = acpi_scan_bus_check;
 		break;
 	case ACPI_NOTIFY_DEVICE_CHECK:
 		acpi_handle_debug(handle, "ACPI_NOTIFY_DEVICE_CHECK event\n");
-		callback = acpi_scan_device_check;
 		break;
 	case ACPI_NOTIFY_EJECT_REQUEST:
 		acpi_handle_debug(handle, "ACPI_NOTIFY_EJECT_REQUEST event\n");
@@ -438,8 +416,7 @@ static void acpi_hotplug_notify_cb(acpi_
 			goto err_out;
 
 		get_device(&adev->dev);
-		callback = acpi_bus_hot_remove_device;
-		status = acpi_os_hotplug_execute(callback, adev);
+		status = acpi_hotplug_execute(acpi_bus_device_eject, adev, type);
 		if (ACPI_SUCCESS(status))
 			return;
 
@@ -449,7 +426,7 @@ static void acpi_hotplug_notify_cb(acpi_
 		/* non-hotplug event; possibly handled by other handler */
 		return;
 	}
-	status = acpi_os_hotplug_execute(callback, handle);
+	status = acpi_hotplug_execute(acpi_scan_bus_device_check, handle, type);
 	if (ACPI_SUCCESS(status))
 		return;
 
@@ -484,11 +461,6 @@ static ssize_t power_state_show(struct d
 
 static DEVICE_ATTR(power_state, 0444, power_state_show, NULL);
 
-static void acpi_eject_store_work(void *context)
-{
-	acpi_bus_device_eject(context, ACPI_OST_EC_OSPM_EJECT);
-}
-
 static ssize_t
 acpi_eject_store(struct device *d, struct device_attribute *attr,
 		const char *buf, size_t count)
@@ -511,7 +483,8 @@ acpi_eject_store(struct device *d, struc
 	acpi_evaluate_hotplug_ost(acpi_device->handle, ACPI_OST_EC_OSPM_EJECT,
 				  ACPI_OST_SC_EJECT_IN_PROGRESS, NULL);
 	get_device(&acpi_device->dev);
-	status = acpi_os_hotplug_execute(acpi_eject_store_work, acpi_device);
+	status = acpi_hotplug_execute(acpi_bus_device_eject, acpi_device,
+				      ACPI_OST_EC_OSPM_EJECT);
 	if (ACPI_SUCCESS(status))
 		return count;
 
Index: linux-pm/drivers/acpi/pci_root.c
===================================================================
--- linux-pm.orig/drivers/acpi/pci_root.c
+++ linux-pm/drivers/acpi/pci_root.c
@@ -592,17 +592,10 @@ static void handle_root_bridge_insertion
 		acpi_handle_err(handle, "cannot add bridge to acpi list\n");
 }
 
-static void _handle_hotplug_event_root(struct work_struct *work)
+static void hotplug_event_root(void *data, u32 type)
 {
+	acpi_handle handle = data;
 	struct acpi_pci_root *root;
-	struct acpi_hp_work *hp_work;
-	acpi_handle handle;
-	u32 type;
-
-	hp_work = container_of(work, struct acpi_hp_work, work);
-	handle = hp_work->handle;
-	type = hp_work->type;
-	kfree(hp_work); /* allocated in handle_hotplug_event_bridge */
 
 	acpi_scan_lock_acquire();
 
@@ -654,8 +647,7 @@ static void _handle_hotplug_event_root(s
 static void handle_hotplug_event_root(acpi_handle handle, u32 type,
 					void *context)
 {
-	alloc_acpi_hp_work(handle, type, context,
-				_handle_hotplug_event_root);
+	acpi_hotplug_execute(hotplug_event_root, handle, type);
 }
 
 static acpi_status __init
Index: linux-pm/drivers/acpi/dock.c
===================================================================
--- linux-pm.orig/drivers/acpi/dock.c
+++ linux-pm/drivers/acpi/dock.c
@@ -669,39 +669,20 @@ static void dock_notify(struct dock_stat
 	}
 }
 
-struct dock_data {
-	struct dock_station *ds;
-	u32 event;
-};
-
-static void acpi_dock_deferred_cb(void *context)
+static void acpi_dock_deferred_cb(void *data, u32 event)
 {
-	struct dock_data *data = context;
-
 	acpi_scan_lock_acquire();
-	dock_notify(data->ds, data->event);
+	dock_notify(data, event);
 	acpi_scan_lock_release();
-	kfree(data);
 }
 
 static void dock_notify_handler(acpi_handle handle, u32 event, void *data)
 {
-	struct dock_data *dd;
-
 	if (event != ACPI_NOTIFY_BUS_CHECK && event != ACPI_NOTIFY_DEVICE_CHECK
 	   && event != ACPI_NOTIFY_EJECT_REQUEST)
 		return;
 
-	dd = kmalloc(sizeof(*dd), GFP_KERNEL);
-	if (dd) {
-		acpi_status status;
-
-		dd->ds = data;
-		dd->event = event;
-		status = acpi_os_hotplug_execute(acpi_dock_deferred_cb, dd);
-		if (ACPI_FAILURE(status))
-			kfree(dd);
-	}
+	acpi_hotplug_execute(acpi_dock_deferred_cb, data, event);
 }
 
 /**
Index: linux-pm/drivers/pci/hotplug/acpiphp_glue.c
===================================================================
--- linux-pm.orig/drivers/pci/hotplug/acpiphp_glue.c
+++ linux-pm/drivers/pci/hotplug/acpiphp_glue.c
@@ -871,21 +871,17 @@ static void hotplug_event(acpi_handle ha
 		put_bridge(bridge);
 }
 
-static void hotplug_event_work(struct work_struct *work)
+static void hotplug_event_work(void *data, u32 type)
 {
-	struct acpiphp_context *context;
-	struct acpi_hp_work *hp_work;
+	struct acpiphp_context *context = data;
+	acpi_handle handle = context->handle;
 
-	hp_work = container_of(work, struct acpi_hp_work, work);
-	context = hp_work->context;
 	acpi_scan_lock_acquire();
 
-	hotplug_event(hp_work->handle, hp_work->type, context);
+	hotplug_event(handle, type, context);
 
 	acpi_scan_lock_release();
-	acpi_evaluate_hotplug_ost(hp_work->handle, hp_work->type,
-				  ACPI_OST_SC_SUCCESS, NULL);
-	kfree(hp_work); /* allocated in handle_hotplug_event() */
+	acpi_evaluate_hotplug_ost(handle, type, ACPI_OST_SC_SUCCESS, NULL);
 	put_bridge(context->func.parent);
 }
 
@@ -936,10 +932,10 @@ static void handle_hotplug_event(acpi_ha
 
 	mutex_lock(&acpiphp_context_lock);
 	context = acpiphp_get_context(handle);
-	if (context) {
+	if (context && !WARN_ON(context->handle != handle)) {
 		get_bridge(context->func.parent);
 		acpiphp_put_context(context);
-		alloc_acpi_hp_work(handle, type, context, hotplug_event_work);
+		acpi_hotplug_execute(hotplug_event_work, context, type);
 		mutex_unlock(&acpiphp_context_lock);
 		return;
 	}
Index: linux-pm/drivers/acpi/internal.h
===================================================================
--- linux-pm.orig/drivers/acpi/internal.h
+++ linux-pm/drivers/acpi/internal.h
@@ -92,7 +92,7 @@ void acpi_device_add_finalize(struct acp
 void acpi_free_pnp_ids(struct acpi_device_pnp *pnp);
 int acpi_bind_one(struct device *dev, acpi_handle handle);
 int acpi_unbind_one(struct device *dev);
-void acpi_bus_device_eject(struct acpi_device *device, u32 ost_src);
+void acpi_bus_device_eject(void *data, u32 ost_src);
 
 /* --------------------------------------------------------------------------
                                   Power Resource