[PATCH 0/10] ACPI: Device objects for all namespace nodes and PCI root hotplug integration

[PATCH 0/10] ACPI: Device objects for all namespace nodes and PCI root hotplug integration

[Rafael J. Wysocki]

Hi,

This is 3.14 material, but I'm posting it today to let you know it is coming.
Of course, if you see any immediate problems with the following patches, please
let me know.

Patch [1/10] is a place holder until we get an equivalent from ACPICA upstream,
which should happen early after 3.13-rc1.  The remaining patches are:

 [2/10] ACPI / scan: Define non-empty removal handler to passed to
        acpi_attach_data() for struct acpi_device objects.

 [3/10] ACPI / scan: Create struct acpi_device objects for all device nodes in
        the namespace regardless of the current result of _STA (the reasons are
        explained in the changelog).

 [4/10] ACPI / hotplug: Do not fail bus check and device check notifications
        if hotplug is not enabled for the given group of devices via sysfs
        (failing those notifications only skips device enumeration, but doesn't
         prevent devices from being physically added to the system).

 [5/10] ACPI / hotplug: Introduce common device hotplug function
        acpi_device_hotplug() and reduce some code duplication.

 [6/10] ACPI / hotplug: Make PCI host bridge hotplug use common code (instead
        of defining its own notify handler).

 [7/10] ACPI / hotplug: Move container-specific code out of the common code.

 [8/10] ACPI / hotplug: Rework the common device hotplug code to cover surprise
        removals of devices.

 [9/10] ACPI / hotplug: Drop unfinished global hotplug notification handling
        routines.

[10/10] ACPI: Clean up some revoltingly ugly stuff.

Thanks!

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

[PATCH 1/10] ACPICA: Delete all attached data objects on node deletion

[Rafael J. Wysocki]

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

Since many data objects may be attached to a single namespace node,
acpi_ns_delete_node() should take that into account and delete all
of those objects along with the namespace node itself, calling
deletion handlers for all of them in the process.  Make that happen.

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

Index: linux-pm/drivers/acpi/acpica/nsalloc.c
===================================================================
--- linux-pm.orig/drivers/acpi/acpica/nsalloc.c
+++ linux-pm/drivers/acpi/acpica/nsalloc.c
@@ -106,6 +106,7 @@ struct acpi_namespace_node *acpi_ns_crea
 void acpi_ns_delete_node(struct acpi_namespace_node *node)
 {
 	union acpi_operand_object *obj_desc;
+	union acpi_operand_object *next_obj_desc;
 
 	ACPI_FUNCTION_NAME(ns_delete_node);
 
@@ -119,7 +120,8 @@ void acpi_ns_delete_node(struct acpi_nam
 	 * detached above, the only possible remaining object is a data object.
 	 */
 	obj_desc = node->object;
-	if (obj_desc && (obj_desc->common.type == ACPI_TYPE_LOCAL_DATA)) {
+	while (obj_desc && obj_desc->common.type == ACPI_TYPE_LOCAL_DATA) {
+		next_obj_desc = obj_desc->common.next_object;
 
 		/* Invoke the attached data deletion handler if present */
 
@@ -128,6 +130,8 @@ void acpi_ns_delete_node(struct acpi_nam
 		}
 
 		acpi_ut_remove_reference(obj_desc);
+
+		obj_desc = next_obj_desc;
 	}
 
 	/* Now we can delete the node */

[PATCH 2/10] ACPI / scan: Define non-empty device removal handler

[Rafael J. Wysocki]

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

If an ACPI namespace node is removed (usually, as a result of a
table unload), and there is a data object attached to that node,
acpi_ns_delete_node() executes the removal handler submitted to
acpi_attach_data() for that object.  That handler is currently empty
for struct acpi_device objects, so it is necessary to detach those
objects from the corresponding ACPI namespace nodes in advance every
time a table unload may happen.  That is cumbersome and inefficient
and leads to some design constraints that turn out to be quite
inconvenient (in particular, struct acpi_device objects cannot be
registered for namespace nodes representing devices that are not
reported as present or functional by _STA).

For this reason, introduce a non-empty removal handler for ACPI
device objects that will unregister them when their ACPI namespace
nodes go away.

This code modification alone should not change functionality except
for the ordering of the ACPI hotplug workqueue which should not
matter (without subsequent code changes).

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

Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -14,6 +14,8 @@
 
 #include <acpi/acpi_drivers.h>
 
+#include <asm/pgtable.h>
+
 #include "internal.h"
 
 #define _COMPONENT		ACPI_BUS_COMPONENT
@@ -27,6 +29,8 @@ extern struct acpi_device *acpi_root;
 
 #define ACPI_IS_ROOT_DEVICE(device)    (!(device)->parent)
 
+#define INVALID_ACPI_HANDLE	((acpi_handle)empty_zero_page)
+
 /*
  * If set, devices will be hot-removed even if they cannot be put offline
  * gracefully (from the kernel's standpoint).
@@ -907,9 +911,91 @@ struct bus_type acpi_bus_type = {
 	.uevent		= acpi_device_uevent,
 };
 
-static void acpi_bus_data_handler(acpi_handle handle, void *context)
+static void acpi_device_del(struct acpi_device *device)
+{
+	mutex_lock(&acpi_device_lock);
+	if (device->parent)
+		list_del(&device->node);
+
+	list_del(&device->wakeup_list);
+	mutex_unlock(&acpi_device_lock);
+
+	acpi_power_add_remove_device(device, false);
+	acpi_device_remove_files(device);
+	if (device->remove)
+		device->remove(device);
+
+	device_del(&device->dev);
+}
+
+static LIST_HEAD(acpi_device_del_list);
+static DEFINE_MUTEX(acpi_device_del_lock);
+
+static void acpi_device_del_work_fn(struct work_struct *work_not_used)
+{
+	for (;;) {
+		struct acpi_device *adev;
+
+		mutex_lock(&acpi_device_del_lock);
+
+		if (list_empty(&acpi_device_del_list)) {
+			mutex_unlock(&acpi_device_del_lock);
+			break;
+		}
+		adev = list_first_entry(&acpi_device_del_list,
+					struct acpi_device, del_list);
+		list_del(&adev->del_list);
+
+		mutex_unlock(&acpi_device_del_lock);
+
+		acpi_device_del(adev);
+		/*
+		 * Drop references to all power resources that might have been
+		 * used by the device.
+		 */
+		acpi_power_transition(adev, ACPI_STATE_D3_COLD);
+		put_device(&adev->dev);
+	}
+}
+
+/**
+ * acpi_scan_drop_device - Drop an ACPI device object.
+ * @handle: Handle of an ACPI namespace node, not used.
+ * @context: Address of the ACPI device object to drop.
+ *
+ * This is invoked by acpi_ns_delete_node() during the removal of the ACPI
+ * namespace node the device object pointed to by @context is attached to.
+ *
+ * The unregistration is carried out asynchronously to avoid running
+ * acpi_device_del() under the ACPICA's namespace mutex and the list is used to
+ * ensure the correct ordering (the device objects must be unregistered in the
+ * same order in which the corresponding namespace nodes are deleted).
+ */
+static void acpi_scan_drop_device(acpi_handle handle, void *context)
 {
-	/* Intentionally empty. */
+	static DECLARE_WORK(work, acpi_device_del_work_fn);
+	struct acpi_device *adev = context;
+
+	mutex_lock(&acpi_device_del_lock);
+
+	/*
+	 * Use the ACPI hotplug workqueue which is ordered, so this work item
+	 * won't run after any hotplug work items submitted subsequently.  That
+	 * prevents attempts to register device objects identical to those being
+	 * deleted from happening concurrently (such attempts result from
+	 * hotplug events handled via the ACPI hotplug workqueue).  It also will
+	 * run after all of the work items submitted previosuly, which helps
+	 * those work items to ensure that they are not accessing stale device
+	 * objects.
+	 */
+	if (list_empty(&acpi_device_del_list))
+		acpi_queue_hotplug_work(&work);
+
+	list_add_tail(&adev->del_list, &acpi_device_del_list);
+	/* Make acpi_ns_validate_handle() return NULL for this handle. */
+	adev->handle = INVALID_ACPI_HANDLE;
+
+	mutex_unlock(&acpi_device_del_lock);
 }
 
 int acpi_bus_get_device(acpi_handle handle, struct acpi_device **device)
@@ -919,7 +1005,7 @@ int acpi_bus_get_device(acpi_handle hand
 	if (!device)
 		return -EINVAL;
 
-	status = acpi_get_data(handle, acpi_bus_data_handler, (void **)device);
+	status = acpi_get_data(handle, acpi_scan_drop_device, (void **)device);
 	if (ACPI_FAILURE(status) || !*device) {
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "No context for object [%p]\n",
 				  handle));
@@ -939,7 +1025,7 @@ int acpi_device_add(struct acpi_device *
 	if (device->handle) {
 		acpi_status status;
 
-		status = acpi_attach_data(device->handle, acpi_bus_data_handler,
+		status = acpi_attach_data(device->handle, acpi_scan_drop_device,
 					  device);
 		if (ACPI_FAILURE(status)) {
 			acpi_handle_err(device->handle,
@@ -957,6 +1043,7 @@ int acpi_device_add(struct acpi_device *
 	INIT_LIST_HEAD(&device->node);
 	INIT_LIST_HEAD(&device->wakeup_list);
 	INIT_LIST_HEAD(&device->physical_node_list);
+	INIT_LIST_HEAD(&device->del_list);
 	mutex_init(&device->physical_node_lock);
 
 	new_bus_id = kzalloc(sizeof(struct acpi_device_bus_id), GFP_KERNEL);
@@ -1020,27 +1107,14 @@ int acpi_device_add(struct acpi_device *
 	mutex_unlock(&acpi_device_lock);
 
  err_detach:
-	acpi_detach_data(device->handle, acpi_bus_data_handler);
+	acpi_detach_data(device->handle, acpi_scan_drop_device);
 	return result;
 }
 
 static void acpi_device_unregister(struct acpi_device *device)
 {
-	mutex_lock(&acpi_device_lock);
-	if (device->parent)
-		list_del(&device->node);
-
-	list_del(&device->wakeup_list);
-	mutex_unlock(&acpi_device_lock);
-
-	acpi_detach_data(device->handle, acpi_bus_data_handler);
-
-	acpi_power_add_remove_device(device, false);
-	acpi_device_remove_files(device);
-	if (device->remove)
-		device->remove(device);
-
-	device_del(&device->dev);
+	acpi_detach_data(device->handle, acpi_scan_drop_device);
+	acpi_device_del(device);
 	/*
 	 * Transition the device to D3cold to drop the reference counts of all
 	 * power resources the device depends on and turn off the ones that have
Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -298,6 +298,7 @@ struct acpi_device {
 	struct list_head children;
 	struct list_head node;
 	struct list_head wakeup_list;
+	struct list_head del_list;
 	struct acpi_device_status status;
 	struct acpi_device_flags flags;
 	struct acpi_device_pnp pnp;
Index: linux-pm/drivers/acpi/osl.c
===================================================================
--- linux-pm.orig/drivers/acpi/osl.c
+++ linux-pm/drivers/acpi/osl.c
@@ -1215,6 +1215,10 @@ acpi_status acpi_hotplug_execute(acpi_hp
 	return AE_OK;
 }
 
+bool acpi_queue_hotplug_work(struct work_struct *work)
+{
+	return queue_work(kacpi_hotplug_wq, work);
+}
 
 acpi_status
 acpi_os_create_semaphore(u32 max_units, u32 initial_units, acpi_handle * handle)
@@ -1794,7 +1798,7 @@ acpi_status __init acpi_os_initialize1(v
 {
 	kacpid_wq = alloc_workqueue("kacpid", 0, 1);
 	kacpi_notify_wq = alloc_workqueue("kacpi_notify", 0, 1);
-	kacpi_hotplug_wq = alloc_workqueue("kacpi_hotplug", 0, 1);
+	kacpi_hotplug_wq = alloc_ordered_workqueue("kacpi_hotplug", 0);
 	BUG_ON(!kacpid_wq);
 	BUG_ON(!kacpi_notify_wq);
 	BUG_ON(!kacpi_hotplug_wq);
Index: linux-pm/drivers/acpi/internal.h
===================================================================
--- linux-pm.orig/drivers/acpi/internal.h
+++ linux-pm/drivers/acpi/internal.h
@@ -73,6 +73,8 @@ void acpi_lpss_init(void);
 static inline void acpi_lpss_init(void) {}
 #endif
 
+bool acpi_queue_hotplug_work(struct work_struct *work);
+
 /* --------------------------------------------------------------------------
                      Device Node Initialization / Removal
    -------------------------------------------------------------------------- */

[PATCH 3/10] ACPI / scan: Add acpi_device objects for all device nodes in the namespace

[Rafael J. Wysocki]

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

Modify the ACPI namespace scanning code to register a struct
acpi_device object for every namespace node representing a device,
processor and so on, even if the device represented by that namespace
node is reported to be not present and not functional by _STA.

There are multiple reasons to do that.  First of all, it avoids
quite a lot of overhead when struct acpi_device objects are
deleted every time acpi_bus_trim() is run and then added again
by a subsequent acpi_bus_scan() for the same scope, although the
namespace objects they correspond to stay in memory all the time
(which always is the case on a vast majority of systems).

Second, it will allow user space to see that there are namespace
nodes representing devices that are not present at the moment and may
be added to the system.  It will also allow user space to evaluate
_SUN for those nodes to check what physical slots the "missing"
devices may be put into and it will make sense to add a sysfs
attribute for _STA evaluation after this change (that will be
useful for thermal management on some systems).

Next, it will help to consolidate the ACPI hotplug handling among
subsystems by making it possible to store hotplug-related information
in struct acpi_device objects in a standard common way.

Finally, it will help to avoid a race condition related to the
deletion of ACPI namespace nodes.  Namely, namespace nodes may be
deleted as a result of a table unload triggered by _EJ0 or _DCK.
If a hotplug notification for one of those nodes is triggered
right before the deletion and it executes a hotplug callback
via acpi_hotplug_execute(), the ACPI handle passed to that
callback may be stale when the callback actually runs.  One way
to work around that is to always pass struct acpi_device pointers
to hotplug callbacks after doing a get_device() on the objects in
question which eliminates the use-after-free possibility (the ACPI
handles in those objects are invalidated by acpi_scan_drop_device(),
so they will trigger ACPICA errors on attempts to use them).

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 Documentation/acpi/namespace.txt   |   10 ---
 drivers/acpi/device_pm.c           |   22 ++++++-
 drivers/acpi/dock.c                |    9 +--
 drivers/acpi/internal.h            |    3 +
 drivers/acpi/pci_root.c            |    5 +
 drivers/acpi/scan.c                |  107 +++++++++++++++++--------------------
 drivers/pci/hotplug/acpiphp_glue.c |    2 
 drivers/xen/xen-acpi-cpuhotplug.c  |    8 +-
 drivers/xen/xen-acpi-memhotplug.c  |    7 +-
 include/acpi/acpi_bus.h            |    9 ++-
 10 files changed, 98 insertions(+), 84 deletions(-)

Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -168,7 +168,9 @@ struct acpi_device_flags {
 	u32 ejectable:1;
 	u32 power_manageable:1;
 	u32 match_driver:1;
-	u32 reserved:27;
+	u32 initialized:1;
+	u32 visited:1;
+	u32 reserved:25;
 };
 
 /* File System */
@@ -385,6 +387,11 @@ int acpi_match_device_ids(struct acpi_de
 int acpi_create_dir(struct acpi_device *);
 void acpi_remove_dir(struct acpi_device *);
 
+static inline bool acpi_device_enumerated(struct acpi_device *adev)
+{
+	return adev && adev->flags.initialized && adev->flags.visited;
+}
+
 typedef void (*acpi_hp_callback)(void *data, u32 src);
 
 acpi_status acpi_hotplug_execute(acpi_hp_callback func, void *data, u32 src);
Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -259,7 +259,6 @@ static int acpi_scan_hot_remove(struct a
 
 	acpi_bus_trim(device);
 
-	/* Device node has been unregistered. */
 	put_device(&device->dev);
 	device = NULL;
 
@@ -328,7 +327,7 @@ void acpi_bus_device_eject(void *data, u
 static void acpi_scan_bus_device_check(void *data, u32 ost_source)
 {
 	acpi_handle handle = data;
-	struct acpi_device *device = NULL;
+	struct acpi_device *device;
 	u32 ost_code = ACPI_OST_SC_NON_SPECIFIC_FAILURE;
 	int error;
 
@@ -336,8 +335,9 @@ static void acpi_scan_bus_device_check(v
 	mutex_lock(&acpi_scan_lock);
 
 	if (ost_source != ACPI_NOTIFY_BUS_CHECK) {
+		device = NULL;
 		acpi_bus_get_device(handle, &device);
-		if (device) {
+		if (acpi_device_enumerated(device)) {
 			dev_warn(&device->dev, "Attempt to re-insert\n");
 			goto out;
 		}
@@ -347,9 +347,10 @@ static void acpi_scan_bus_device_check(v
 		acpi_handle_warn(handle, "Namespace scan failure\n");
 		goto out;
 	}
-	error = acpi_bus_get_device(handle, &device);
-	if (error) {
-		acpi_handle_warn(handle, "Missing device node object\n");
+	device = NULL;
+	acpi_bus_get_device(handle, &device);
+	if (!acpi_device_enumerated(device)) {
+		acpi_handle_warn(handle, "Device not enumerated\n");
 		goto out;
 	}
 	ost_code = ACPI_OST_SC_SUCCESS;
@@ -1111,20 +1112,6 @@ int acpi_device_add(struct acpi_device *
 	return result;
 }
 
-static void acpi_device_unregister(struct acpi_device *device)
-{
-	acpi_detach_data(device->handle, acpi_scan_drop_device);
-	acpi_device_del(device);
-	/*
-	 * Transition the device to D3cold to drop the reference counts of all
-	 * power resources the device depends on and turn off the ones that have
-	 * no more references.
-	 */
-	acpi_device_set_power(device, ACPI_STATE_D3_COLD);
-	device->handle = NULL;
-	put_device(&device->dev);
-}
-
 /* --------------------------------------------------------------------------
                                  Driver Management
    -------------------------------------------------------------------------- */
@@ -1703,6 +1690,8 @@ void acpi_init_device_object(struct acpi
 	acpi_set_pnp_ids(handle, &device->pnp, type);
 	acpi_bus_get_flags(device);
 	device->flags.match_driver = false;
+	device->flags.initialized = true;
+	device->flags.visited = false;
 	device_initialize(&device->dev);
 	dev_set_uevent_suppress(&device->dev, true);
 }
@@ -1787,6 +1776,15 @@ static int acpi_bus_type_and_status(acpi
 	return 0;
 }
 
+bool acpi_device_is_present(struct acpi_device *adev)
+{
+	if (adev->status.present || adev->status.functional)
+		return true;
+
+	adev->flags.initialized = false;
+	return false;
+}
+
 static bool acpi_scan_handler_matching(struct acpi_scan_handler *handler,
 				       char *idstr,
 				       const struct acpi_device_id **matchid)
@@ -1880,18 +1878,6 @@ static acpi_status acpi_bus_check_add(ac
 
 	acpi_scan_init_hotplug(handle, type);
 
-	if (!(sta & ACPI_STA_DEVICE_PRESENT) &&
-	    !(sta & ACPI_STA_DEVICE_FUNCTIONING)) {
-		struct acpi_device_wakeup wakeup;
-
-		if (acpi_has_method(handle, "_PRW")) {
-			acpi_bus_extract_wakeup_device_power_package(handle,
-								     &wakeup);
-			acpi_power_resources_list_free(&wakeup.resources);
-		}
-		return AE_CTRL_DEPTH;
-	}
-
 	acpi_add_single_object(&device, handle, type, sta);
 	if (!device)
 		return AE_CTRL_DEPTH;
@@ -1930,32 +1916,50 @@ static acpi_status acpi_bus_device_attac
 					  void *not_used, void **ret_not_used)
 {
 	struct acpi_device *device;
-	unsigned long long sta_not_used;
+	unsigned long long sta;
 	int ret;
 
 	/*
 	 * Ignore errors ignored by acpi_bus_check_add() to avoid terminating
 	 * namespace walks prematurely.
 	 */
-	if (acpi_bus_type_and_status(handle, &ret, &sta_not_used))
+	if (acpi_bus_type_and_status(handle, &ret, &sta))
 		return AE_OK;
 
 	if (acpi_bus_get_device(handle, &device))
 		return AE_CTRL_DEPTH;
 
+	STRUCT_TO_INT(device->status) = sta;
+	/* Skip devices that are not present. */
+	if (!acpi_device_is_present(device))
+		goto err;
+
 	if (device->handler)
 		return AE_OK;
 
+	if (!device->flags.initialized) {
+		acpi_bus_update_power(device, NULL);
+		device->flags.initialized = true;
+	}
 	ret = acpi_scan_attach_handler(device);
 	if (ret < 0)
-		return AE_CTRL_DEPTH;
+		goto err;
 
 	device->flags.match_driver = true;
 	if (ret > 0)
-		return AE_OK;
+		goto ok;
 
 	ret = device_attach(&device->dev);
-	return ret >= 0 ? AE_OK : AE_CTRL_DEPTH;
+	if (ret < 0)
+		goto err;
+
+ ok:
+	device->flags.visited = true;
+	return AE_OK;
+
+ err:
+	device->flags.visited = false;
+	return AE_CTRL_DEPTH;
 }
 
 /**
@@ -2007,21 +2011,17 @@ static acpi_status acpi_bus_device_detac
 		} else {
 			device_release_driver(&device->dev);
 		}
+		/*
+		 * Most likely, the device is going away, so put it into D3cold
+		 * before that.
+		 */
+		acpi_device_set_power(device, ACPI_STATE_D3_COLD);
+		device->flags.initialized = false;
+		device->flags.visited = false;
 	}
 	return AE_OK;
 }
 
-static acpi_status acpi_bus_remove(acpi_handle handle, u32 lvl_not_used,
-				   void *not_used, void **ret_not_used)
-{
-	struct acpi_device *device = NULL;
-
-	if (!acpi_bus_get_device(handle, &device))
-		acpi_device_unregister(device);
-
-	return AE_OK;
-}
-
 /**
  * acpi_bus_trim - Remove ACPI device node and all of its descendants
  * @start: Root of the ACPI device nodes subtree to remove.
@@ -2037,13 +2037,6 @@ void acpi_bus_trim(struct acpi_device *s
 	acpi_walk_namespace(ACPI_TYPE_ANY, start->handle, ACPI_UINT32_MAX, NULL,
 			    acpi_bus_device_detach, NULL, NULL);
 	acpi_bus_device_detach(start->handle, 0, NULL, NULL);
-	/*
-	 * Execute acpi_bus_remove() as a post-order callback to remove device
-	 * nodes in the given namespace scope.
-	 */
-	acpi_walk_namespace(ACPI_TYPE_ANY, start->handle, ACPI_UINT32_MAX, NULL,
-			    acpi_bus_remove, NULL, NULL);
-	acpi_bus_remove(start->handle, 0, NULL, NULL);
 }
 EXPORT_SYMBOL_GPL(acpi_bus_trim);
 
@@ -2119,7 +2112,9 @@ int __init acpi_scan_init(void)
 
 	result = acpi_bus_scan_fixed();
 	if (result) {
-		acpi_device_unregister(acpi_root);
+		acpi_detach_data(acpi_root->handle, acpi_scan_drop_device);
+		acpi_device_del(acpi_root);
+		put_device(&acpi_root->dev);
 		goto out;
 	}
 
Index: linux-pm/drivers/acpi/device_pm.c
===================================================================
--- linux-pm.orig/drivers/acpi/device_pm.c
+++ linux-pm/drivers/acpi/device_pm.c
@@ -256,6 +256,8 @@ int acpi_bus_init_power(struct acpi_devi
 		return -EINVAL;
 
 	device->power.state = ACPI_STATE_UNKNOWN;
+	if (!acpi_device_is_present(device))
+		return 0;
 
 	result = acpi_device_get_power(device, &state);
 	if (result)
@@ -302,15 +304,18 @@ int acpi_device_fix_up_power(struct acpi
 	return ret;
 }
 
-int acpi_bus_update_power(acpi_handle handle, int *state_p)
+int acpi_device_update_power(struct acpi_device *device, int *state_p)
 {
-	struct acpi_device *device;
 	int state;
 	int result;
 
-	result = acpi_bus_get_device(handle, &device);
-	if (result)
+	if (device->power.state == ACPI_STATE_UNKNOWN) {
+		result = acpi_bus_init_power(device);
+		if (!result && state_p)
+			*state_p = device->power.state;
+
 		return result;
+	}
 
 	result = acpi_device_get_power(device, &state);
 	if (result)
@@ -338,6 +343,15 @@ int acpi_bus_update_power(acpi_handle ha
 
 	return 0;
 }
+
+int acpi_bus_update_power(acpi_handle handle, int *state_p)
+{
+	struct acpi_device *device;
+	int result;
+
+	result = acpi_bus_get_device(handle, &device);
+	return result ? result : acpi_device_update_power(device, state_p);
+}
 EXPORT_SYMBOL_GPL(acpi_bus_update_power);
 
 bool acpi_bus_power_manageable(acpi_handle handle)
Index: linux-pm/drivers/acpi/internal.h
===================================================================
--- linux-pm.orig/drivers/acpi/internal.h
+++ linux-pm/drivers/acpi/internal.h
@@ -90,6 +90,7 @@ void acpi_free_pnp_ids(struct acpi_devic
 int acpi_bind_one(struct device *dev, acpi_handle handle);
 int acpi_unbind_one(struct device *dev);
 void acpi_bus_device_eject(void *data, u32 ost_src);
+bool acpi_device_is_present(struct acpi_device *adev);
 
 /* --------------------------------------------------------------------------
                                   Power Resource
@@ -107,6 +108,8 @@ int acpi_power_get_inferred_state(struct
 int acpi_power_on_resources(struct acpi_device *device, int state);
 int acpi_power_transition(struct acpi_device *device, int state);
 
+int acpi_device_update_power(struct acpi_device *device, int *state_p);
+
 int acpi_wakeup_device_init(void);
 void acpi_early_processor_set_pdc(void);
 
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
@@ -489,7 +489,7 @@ static void acpiphp_bus_add(acpi_handle
 
 	acpi_bus_scan(handle);
 	acpi_bus_get_device(handle, &adev);
-	if (adev)
+	if (acpi_device_enumerated(adev))
 		acpi_device_set_power(adev, ACPI_STATE_D0);
 }
 
Index: linux-pm/drivers/acpi/dock.c
===================================================================
--- linux-pm.orig/drivers/acpi/dock.c
+++ linux-pm/drivers/acpi/dock.c
@@ -323,14 +323,11 @@ static int dock_present(struct dock_stat
  */
 static void dock_create_acpi_device(acpi_handle handle)
 {
-	struct acpi_device *device;
+	struct acpi_device *device = NULL;
 	int ret;
 
-	if (acpi_bus_get_device(handle, &device)) {
-		/*
-		 * no device created for this object,
-		 * so we should create one.
-		 */
+	acpi_bus_get_device(handle, &device);
+	if (!acpi_device_enumerated(device)) {
 		ret = acpi_bus_scan(handle);
 		if (ret)
 			pr_debug("error adding bus, %x\n", -ret);
Index: linux-pm/drivers/acpi/pci_root.c
===================================================================
--- linux-pm.orig/drivers/acpi/pci_root.c
+++ linux-pm/drivers/acpi/pci_root.c
@@ -631,9 +631,10 @@ void __init acpi_pci_root_init(void)
 
 static void handle_root_bridge_insertion(acpi_handle handle)
 {
-	struct acpi_device *device;
+	struct acpi_device *device = NULL;
 
-	if (!acpi_bus_get_device(handle, &device)) {
+	acpi_bus_get_device(handle, &device);
+	if (acpi_device_enumerated(device)) {
 		dev_printk(KERN_DEBUG, &device->dev,
 			   "acpi device already exists; ignoring notify\n");
 		return;
Index: linux-pm/drivers/xen/xen-acpi-cpuhotplug.c
===================================================================
--- linux-pm.orig/drivers/xen/xen-acpi-cpuhotplug.c
+++ linux-pm/drivers/xen/xen-acpi-cpuhotplug.c
@@ -269,7 +269,8 @@ static void acpi_processor_hotplug_notif
 		if (!is_processor_present(handle))
 			break;
 
-		if (!acpi_bus_get_device(handle, &device))
+		acpi_bus_get_device(handle, &device);
+		if (acpi_device_enumerated(device))
 			break;
 
 		result = acpi_bus_scan(handle);
@@ -277,8 +278,9 @@ static void acpi_processor_hotplug_notif
 			pr_err(PREFIX "Unable to add the device\n");
 			break;
 		}
-		result = acpi_bus_get_device(handle, &device);
-		if (result) {
+		device = NULL;
+		acpi_bus_get_device(handle, &device);
+		if (!acpi_device_enumerated(device)) {
 			pr_err(PREFIX "Missing device object\n");
 			break;
 		}
Index: linux-pm/drivers/xen/xen-acpi-memhotplug.c
===================================================================
--- linux-pm.orig/drivers/xen/xen-acpi-memhotplug.c
+++ linux-pm/drivers/xen/xen-acpi-memhotplug.c
@@ -169,7 +169,7 @@ static int acpi_memory_get_device(acpi_h
 	acpi_scan_lock_acquire();
 
 	acpi_bus_get_device(handle, &device);
-	if (device)
+	if (acpi_device_enumerated(device))
 		goto end;
 
 	/*
@@ -182,8 +182,9 @@ static int acpi_memory_get_device(acpi_h
 		result = -EINVAL;
 		goto out;
 	}
-	result = acpi_bus_get_device(handle, &device);
-	if (result) {
+	device = NULL;
+	acpi_bus_get_device(handle, &device);
+	if (!acpi_device_enumerated(device)) {
 		pr_warn(PREFIX "Missing device object\n");
 		result = -EINVAL;
 		goto out;
Index: linux-pm/Documentation/acpi/namespace.txt
===================================================================
--- linux-pm.orig/Documentation/acpi/namespace.txt
+++ linux-pm/Documentation/acpi/namespace.txt
@@ -235,10 +235,6 @@ Wysocki <rafael.j.wysocki@intel.com>.
       named object's type in the second column).  In that case the object's
       directory in sysfs will contain the 'path' attribute whose value is
       the full path to the node from the namespace root.
-      struct acpi_device objects are created for the ACPI namespace nodes
-      whose _STA control methods return PRESENT or FUNCTIONING.  The power
-      resource nodes or nodes without _STA are assumed to be both PRESENT
-      and FUNCTIONING.
    F:
       The struct acpi_device object is created for a fixed hardware
       feature (as indicated by the fixed feature flag's name in the second
@@ -340,7 +336,7 @@ Wysocki <rafael.j.wysocki@intel.com>.
      | +-------------+-------+----------------+
      |   |
      |   | +- - - - - - - +- - - - - - +- - - - - - - -+
-     |   +-| * PNP0C0D:00 | \_SB_.LID0 | acpi:PNP0C0D: |
+     |   +-| PNP0C0D:00 | \_SB_.LID0 | acpi:PNP0C0D: |
      |   | +- - - - - - - +- - - - - - +- - - - - - - -+
      |   |
      |   | +------------+------------+-----------------------+
@@ -390,6 +386,4 @@ Wysocki <rafael.j.wysocki@intel.com>.
             attribute (as described earlier in this document).
    NOTE: N/A indicates the device object does not have the 'path' or the
          'modalias' attribute.
-   NOTE: The PNP0C0D device listed above is highlighted (marked by "*")
-         to indicate it will be created only when its _STA methods return
-         PRESENT or FUNCTIONING.
+

[PATCH 4/10] ACPI / hotplug: Do not fail bus and device checks for disabled hotplug

[Rafael J. Wysocki]

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

If the scan handler for the given device has hotplug.enabled
unset, it doesn't really make sense to fail bus check and device
check notifications.

First, bus check may not have anything to do with the device it is
signaled for, but it may concern another device on the bus below
this one.  For this reason, bus check notifications should not be
failed if hotplug is disabled for the target device.

Second, device check notifications are signaled only after a device
has already appeared (or disappeared), so failing it can only prevent
scan handlers and drivers from attaching to that (already existing)
device, which is not very useful.

Consequently, if device hotplug is disabled through the device's
scan handler, fail eject request notifications only.

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

Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -363,43 +363,13 @@ static void acpi_scan_bus_device_check(v
 	unlock_device_hotplug();
 }
 
-static void acpi_hotplug_unsupported(acpi_handle handle, u32 type)
-{
-	u32 ost_status;
-
-	switch (type) {
-	case ACPI_NOTIFY_BUS_CHECK:
-		acpi_handle_debug(handle,
-			"ACPI_NOTIFY_BUS_CHECK event: unsupported\n");
-		ost_status = ACPI_OST_SC_INSERT_NOT_SUPPORTED;
-		break;
-	case ACPI_NOTIFY_DEVICE_CHECK:
-		acpi_handle_debug(handle,
-			"ACPI_NOTIFY_DEVICE_CHECK event: unsupported\n");
-		ost_status = ACPI_OST_SC_INSERT_NOT_SUPPORTED;
-		break;
-	case ACPI_NOTIFY_EJECT_REQUEST:
-		acpi_handle_debug(handle,
-			"ACPI_NOTIFY_EJECT_REQUEST event: unsupported\n");
-		ost_status = ACPI_OST_SC_EJECT_NOT_SUPPORTED;
-		break;
-	default:
-		/* non-hotplug event; possibly handled by other handler */
-		return;
-	}
-
-	acpi_evaluate_hotplug_ost(handle, type, ost_status, NULL);
-}
-
 static void acpi_hotplug_notify_cb(acpi_handle handle, u32 type, void *data)
 {
+	u32 ost_code = ACPI_OST_SC_NON_SPECIFIC_FAILURE;
 	struct acpi_scan_handler *handler = data;
 	struct acpi_device *adev;
 	acpi_status status;
 
-	if (!handler->hotplug.enabled)
-		return acpi_hotplug_unsupported(handle, type);
-
 	switch (type) {
 	case ACPI_NOTIFY_BUS_CHECK:
 		acpi_handle_debug(handle, "ACPI_NOTIFY_BUS_CHECK event\n");
@@ -409,6 +379,11 @@ static void acpi_hotplug_notify_cb(acpi_
 		break;
 	case ACPI_NOTIFY_EJECT_REQUEST:
 		acpi_handle_debug(handle, "ACPI_NOTIFY_EJECT_REQUEST event\n");
+		if (!handler->hotplug.enabled) {
+			acpi_handle_err(handle, "Eject disabled\n");
+			ost_code = ACPI_OST_SC_EJECT_NOT_SUPPORTED;
+			goto err_out;
+		}
 		if (acpi_bus_get_device(handle, &adev))
 			goto err_out;
 
@@ -428,8 +403,7 @@ static void acpi_hotplug_notify_cb(acpi_
 		return;
 
  err_out:
-	acpi_evaluate_hotplug_ost(handle, type,
-				  ACPI_OST_SC_NON_SPECIFIC_FAILURE, NULL);
+	acpi_evaluate_hotplug_ost(handle, type, ost_code, NULL);
 }
 
 static ssize_t real_power_state_show(struct device *dev,

[PATCH 5/10] ACPI / hotplug: Introduce common hotplug function acpi_device_hotplug()

[Rafael J. Wysocki]

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

Modify the common ACPI device hotplug code to always queue up the
same function, acpi_device_hotplug(), using acpi_hotplug_execute()
and make the PCI host bridge hotplug code use that function too for
device hot removal.

This allows some code duplication to be reduced and a race condition
where the relevant ACPI handle may become invalid between the
notification handler and the function queued up by it via
acpi_hotplug_execute() to be avoided.

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

Index: linux-pm/drivers/acpi/internal.h
===================================================================
--- linux-pm.orig/drivers/acpi/internal.h
+++ linux-pm/drivers/acpi/internal.h
@@ -89,7 +89,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(void *data, u32 ost_src);
+void acpi_device_hotplug(void *data, u32 ost_src);
 bool acpi_device_is_present(struct acpi_device *adev);
 
 /* --------------------------------------------------------------------------
Index: linux-pm/drivers/acpi/pci_root.c
===================================================================
--- linux-pm.orig/drivers/acpi/pci_root.c
+++ linux-pm/drivers/acpi/pci_root.c
@@ -680,11 +680,13 @@ static void hotplug_event_root(void *dat
 		if (!root)
 			break;
 
+		acpi_evaluate_hotplug_ost(handle, ACPI_NOTIFY_EJECT_REQUEST,
+					  ACPI_OST_SC_EJECT_IN_PROGRESS, NULL);
 		get_device(&root->device->dev);
 
 		acpi_scan_lock_release();
 
-		acpi_bus_device_eject(root->device, ACPI_NOTIFY_EJECT_REQUEST);
+		acpi_device_hotplug(root->device, ACPI_NOTIFY_EJECT_REQUEST);
 		return;
 	default:
 		acpi_handle_warn(handle,
Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -206,12 +206,8 @@ static int acpi_scan_hot_remove(struct a
 	acpi_status status;
 	unsigned long long sta;
 
-	/* 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;
-	}
+	if (device->handler && device->handler->hotplug.mode == AHM_CONTAINER)
+		kobject_uevent(&device->dev.kobj, KOBJ_OFFLINE);
 
 	/*
 	 * Carry out two passes here and ignore errors in the first pass,
@@ -230,7 +226,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);
@@ -249,7 +244,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;
 		}
 	}
@@ -259,9 +253,6 @@ static int acpi_scan_hot_remove(struct a
 
 	acpi_bus_trim(device);
 
-	put_device(&device->dev);
-	device = NULL;
-
 	acpi_evaluate_lck(handle, 0);
 	/*
 	 * TBD: _EJD support.
@@ -288,77 +279,74 @@ static int acpi_scan_hot_remove(struct a
 	return 0;
 }
 
-void acpi_bus_device_eject(void *data, u32 ost_src)
+static int acpi_scan_device_check(struct acpi_device *adev)
 {
-	struct acpi_device *device = data;
-	acpi_handle handle = device->handle;
-	u32 ost_code = ACPI_OST_SC_NON_SPECIFIC_FAILURE;
 	int error;
 
-	lock_device_hotplug();
-	mutex_lock(&acpi_scan_lock);
-
-	if (ost_src == ACPI_NOTIFY_EJECT_REQUEST)
-		acpi_evaluate_hotplug_ost(handle, ACPI_NOTIFY_EJECT_REQUEST,
-					  ACPI_OST_SC_EJECT_IN_PROGRESS, NULL);
-
-	if (device->handler && device->handler->hotplug.mode == AHM_CONTAINER)
-		kobject_uevent(&device->dev.kobj, KOBJ_OFFLINE);
-
-	error = acpi_scan_hot_remove(device);
-	if (error == -EPERM) {
-		goto err_support;
-	} else if (error) {
-		goto err_out;
+	/*
+	 * This function is only called for device objects for which matching
+	 * scan handlers exist.  The only situation in which the scan handler is
+	 * not attached to this device object yet is when the device has just
+	 * appeared (either it wasn't present at all before or it was removed
+	 * and then added again).
+	 */
+	if (adev->handler) {
+		dev_warn(&adev->dev, "Already enumerated\n");
+		return -EBUSY;
 	}
-
- 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, ost_src, ost_code, NULL);
-	goto out;
+	error = acpi_bus_scan(adev->handle);
+	if (error) {
+		dev_warn(&adev->dev, "Namespace scan failure\n");
+		return error;
+	}
+	if (adev->handler) {
+		if (adev->handler->hotplug.mode == AHM_CONTAINER)
+			kobject_uevent(&adev->dev.kobj, KOBJ_ONLINE);
+	} else {
+		dev_warn(&adev->dev, "Enumeration failure\n");
+		return -ENODEV;
+	}
+	return 0;
 }
 
-static void acpi_scan_bus_device_check(void *data, u32 ost_source)
+void acpi_device_hotplug(void *data, u32 src)
 {
-	acpi_handle handle = data;
-	struct acpi_device *device;
 	u32 ost_code = ACPI_OST_SC_NON_SPECIFIC_FAILURE;
+	struct acpi_device *adev = data;
 	int error;
 
 	lock_device_hotplug();
 	mutex_lock(&acpi_scan_lock);
 
-	if (ost_source != ACPI_NOTIFY_BUS_CHECK) {
-		device = NULL;
-		acpi_bus_get_device(handle, &device);
-		if (acpi_device_enumerated(device)) {
-			dev_warn(&device->dev, "Attempt to re-insert\n");
-			goto out;
-		}
-	}
-	error = acpi_bus_scan(handle);
-	if (error) {
-		acpi_handle_warn(handle, "Namespace scan failure\n");
-		goto out;
-	}
-	device = NULL;
-	acpi_bus_get_device(handle, &device);
-	if (!acpi_device_enumerated(device)) {
-		acpi_handle_warn(handle, "Device not enumerated\n");
+	/*
+	 * The device object's ACPI handle cannot become invalid as long as we
+	 * are holding acpi_scan_lock, but it may have become invalid before
+	 * that lock was acquired.
+	 */
+	if (adev->handle == INVALID_ACPI_HANDLE)
 		goto out;
+
+	switch (src) {
+	case ACPI_NOTIFY_BUS_CHECK:
+		error = acpi_bus_scan(adev->handle);
+		break;
+	case ACPI_NOTIFY_DEVICE_CHECK:
+		error = acpi_scan_device_check(adev);
+		break;
+	case ACPI_NOTIFY_EJECT_REQUEST:
+	case ACPI_OST_EC_OSPM_EJECT:
+		error = acpi_scan_hot_remove(adev);
+		break;
+	default:
+		error = -EINVAL;
+		break;
 	}
-	ost_code = ACPI_OST_SC_SUCCESS;
-	if (device->handler && device->handler->hotplug.mode == AHM_CONTAINER)
-		kobject_uevent(&device->dev.kobj, KOBJ_ONLINE);
+	if (!error)
+		ost_code = ACPI_OST_SC_SUCCESS;
 
  out:
-	acpi_evaluate_hotplug_ost(handle, ost_source, ost_code, NULL);
+	acpi_evaluate_hotplug_ost(adev->handle, src, ost_code, NULL);
+	put_device(&adev->dev);
 	mutex_unlock(&acpi_scan_lock);
 	unlock_device_hotplug();
 }
@@ -370,6 +358,9 @@ static void acpi_hotplug_notify_cb(acpi_
 	struct acpi_device *adev;
 	acpi_status status;
 
+	if (acpi_bus_get_device(handle, &adev))
+		goto err_out;
+
 	switch (type) {
 	case ACPI_NOTIFY_BUS_CHECK:
 		acpi_handle_debug(handle, "ACPI_NOTIFY_BUS_CHECK event\n");
@@ -384,24 +375,20 @@ static void acpi_hotplug_notify_cb(acpi_
 			ost_code = ACPI_OST_SC_EJECT_NOT_SUPPORTED;
 			goto err_out;
 		}
-		if (acpi_bus_get_device(handle, &adev))
-			goto err_out;
-
-		get_device(&adev->dev);
-		status = acpi_hotplug_execute(acpi_bus_device_eject, adev, type);
-		if (ACPI_SUCCESS(status))
-			return;
-
-		put_device(&adev->dev);
-		goto err_out;
+		acpi_evaluate_hotplug_ost(handle, ACPI_NOTIFY_EJECT_REQUEST,
+					  ACPI_OST_SC_EJECT_IN_PROGRESS, NULL);
+		break;
 	default:
 		/* non-hotplug event; possibly handled by other handler */
 		return;
 	}
-	status = acpi_hotplug_execute(acpi_scan_bus_device_check, handle, type);
+	get_device(&adev->dev);
+	status = acpi_hotplug_execute(acpi_device_hotplug, adev, type);
 	if (ACPI_SUCCESS(status))
 		return;
 
+	put_device(&adev->dev);
+
  err_out:
 	acpi_evaluate_hotplug_ost(handle, type, ost_code, NULL);
 }
@@ -454,7 +441,7 @@ 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_hotplug_execute(acpi_bus_device_eject, acpi_device,
+	status = acpi_hotplug_execute(acpi_device_hotplug, acpi_device,
 				      ACPI_OST_EC_OSPM_EJECT);
 	if (ACPI_SUCCESS(status))
 		return count;

[PATCH 6/10] ACPI / hotplug: Make ACPI PCI root hotplug use common hotplug code

[Rafael J. Wysocki]

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

Rework the common ACPI device hotplug code so that it is suitable
for PCI host bridge hotplug and switch the PCI host bridge scan
handler to using the common hotplug code.

This allows quite a few lines of code that are not necessary any more
to be dropped from the PCI host bridge scan handler and removes
arbitrary differences in behavior between PCI host bridge hotplug
and ACPI-based hotplug of other components, like CPUs and memory.

Also acpi_device_hotplug() can be static now.

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

Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -101,6 +101,7 @@ struct acpi_hotplug_profile {
 	struct kobject kobj;
 	bool enabled:1;
 	enum acpi_hotplug_mode mode;
+	int (*scan_dependent)(struct acpi_device *adev);
 };
 
 static inline struct acpi_hotplug_profile *to_acpi_hotplug_profile(
Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -283,17 +283,6 @@ static int acpi_scan_device_check(struct
 {
 	int error;
 
-	/*
-	 * This function is only called for device objects for which matching
-	 * scan handlers exist.  The only situation in which the scan handler is
-	 * not attached to this device object yet is when the device has just
-	 * appeared (either it wasn't present at all before or it was removed
-	 * and then added again).
-	 */
-	if (adev->handler) {
-		dev_warn(&adev->dev, "Already enumerated\n");
-		return -EBUSY;
-	}
 	error = acpi_bus_scan(adev->handle);
 	if (error) {
 		dev_warn(&adev->dev, "Namespace scan failure\n");
@@ -309,10 +298,11 @@ static int acpi_scan_device_check(struct
 	return 0;
 }
 
-void acpi_device_hotplug(void *data, u32 src)
+static void acpi_device_hotplug(void *data, u32 src)
 {
 	u32 ost_code = ACPI_OST_SC_NON_SPECIFIC_FAILURE;
 	struct acpi_device *adev = data;
+	struct acpi_scan_handler *handler;
 	int error;
 
 	lock_device_hotplug();
@@ -326,12 +316,32 @@ void acpi_device_hotplug(void *data, u32
 	if (adev->handle == INVALID_ACPI_HANDLE)
 		goto out;
 
+	handler = adev->handler;
+
 	switch (src) {
 	case ACPI_NOTIFY_BUS_CHECK:
-		error = acpi_bus_scan(adev->handle);
+		if (handler) {
+			error = handler->hotplug.scan_dependent ?
+					handler->hotplug.scan_dependent(adev) :
+					acpi_bus_scan(adev->handle);
+		} else {
+			error = acpi_scan_device_check(adev);
+		}
 		break;
 	case ACPI_NOTIFY_DEVICE_CHECK:
-		error = acpi_scan_device_check(adev);
+		/*
+		 * This code is only run for device objects for which matching
+		 * scan handlers exist.  The only situation in which the scan
+		 * handler is not attached to this device object yet is when the
+		 * device has just appeared (either it wasn't present at all
+		 * before or it was removed and then added again).
+		 */
+		if (adev->handler) {
+			dev_warn(&adev->dev, "Already enumerated\n");
+			error = -EBUSY;
+		} else {
+			error = acpi_scan_device_check(adev);
+		}
 		break;
 	case ACPI_NOTIFY_EJECT_REQUEST:
 	case ACPI_OST_EC_OSPM_EJECT:
@@ -2081,8 +2091,6 @@ int __init acpi_scan_init(void)
 
 	acpi_update_all_gpes();
 
-	acpi_pci_root_hp_init();
-
  out:
 	mutex_unlock(&acpi_scan_lock);
 	return result;
Index: linux-pm/drivers/acpi/pci_root.c
===================================================================
--- linux-pm.orig/drivers/acpi/pci_root.c
+++ linux-pm/drivers/acpi/pci_root.c
@@ -51,6 +51,12 @@ static int acpi_pci_root_add(struct acpi
 			     const struct acpi_device_id *not_used);
 static void acpi_pci_root_remove(struct acpi_device *device);
 
+static int acpi_pci_root_scan_dependent(struct acpi_device *adev)
+{
+	acpiphp_check_host_bridge(adev->handle);
+	return 0;
+}
+
 #define ACPI_PCIE_REQ_SUPPORT (OSC_PCI_EXT_CONFIG_SUPPORT \
 				| OSC_PCI_ASPM_SUPPORT \
 				| OSC_PCI_CLOCK_PM_SUPPORT \
@@ -65,6 +71,10 @@ static struct acpi_scan_handler pci_root
 	.ids = root_device_ids,
 	.attach = acpi_pci_root_add,
 	.detach = acpi_pci_root_remove,
+	.hotplug = {
+		.enabled = true,
+		.scan_dependent = acpi_pci_root_scan_dependent,
+	},
 };
 
 static DEFINE_MUTEX(osc_lock);
@@ -621,119 +631,9 @@ static void acpi_pci_root_remove(struct
 void __init acpi_pci_root_init(void)
 {
 	acpi_hest_init();
-
-	if (!acpi_pci_disabled) {
-		pci_acpi_crs_quirks();
-		acpi_scan_add_handler(&pci_root_handler);
-	}
-}
-/* Support root bridge hotplug */
-
-static void handle_root_bridge_insertion(acpi_handle handle)
-{
-	struct acpi_device *device = NULL;
-
-	acpi_bus_get_device(handle, &device);
-	if (acpi_device_enumerated(device)) {
-		dev_printk(KERN_DEBUG, &device->dev,
-			   "acpi device already exists; ignoring notify\n");
-		return;
-	}
-
-	if (acpi_bus_scan(handle))
-		acpi_handle_err(handle, "cannot add bridge to acpi list\n");
-}
-
-static void hotplug_event_root(void *data, u32 type)
-{
-	acpi_handle handle = data;
-	struct acpi_pci_root *root;
-
-	acpi_scan_lock_acquire();
-
-	root = acpi_pci_find_root(handle);
-
-	switch (type) {
-	case ACPI_NOTIFY_BUS_CHECK:
-		/* bus enumerate */
-		acpi_handle_printk(KERN_DEBUG, handle,
-				   "Bus check notify on %s\n", __func__);
-		if (root)
-			acpiphp_check_host_bridge(handle);
-		else
-			handle_root_bridge_insertion(handle);
-
-		break;
-
-	case ACPI_NOTIFY_DEVICE_CHECK:
-		/* device check */
-		acpi_handle_printk(KERN_DEBUG, handle,
-				   "Device check notify on %s\n", __func__);
-		if (!root)
-			handle_root_bridge_insertion(handle);
-		break;
-
-	case ACPI_NOTIFY_EJECT_REQUEST:
-		/* request device eject */
-		acpi_handle_printk(KERN_DEBUG, handle,
-				   "Device eject notify on %s\n", __func__);
-		if (!root)
-			break;
-
-		acpi_evaluate_hotplug_ost(handle, ACPI_NOTIFY_EJECT_REQUEST,
-					  ACPI_OST_SC_EJECT_IN_PROGRESS, NULL);
-		get_device(&root->device->dev);
-
-		acpi_scan_lock_release();
-
-		acpi_device_hotplug(root->device, ACPI_NOTIFY_EJECT_REQUEST);
+	if (acpi_pci_disabled)
 		return;
-	default:
-		acpi_handle_warn(handle,
-				 "notify_handler: unknown event type 0x%x\n",
-				 type);
-		break;
-	}
-
-	acpi_scan_lock_release();
-}
-
-static void handle_hotplug_event_root(acpi_handle handle, u32 type,
-					void *context)
-{
-	acpi_hotplug_execute(hotplug_event_root, handle, type);
-}
-
-static acpi_status __init
-find_root_bridges(acpi_handle handle, u32 lvl, void *context, void **rv)
-{
-	acpi_status status;
-	int *count = (int *)context;
-
-	if (!acpi_is_root_bridge(handle))
-		return AE_OK;
-
-	(*count)++;
-
-	status = acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
-					handle_hotplug_event_root, NULL);
-	if (ACPI_FAILURE(status))
-		acpi_handle_printk(KERN_DEBUG, handle,
-			"notify handler is not installed, exit status: %u\n",
-			 (unsigned int)status);
-	else
-		acpi_handle_printk(KERN_DEBUG, handle,
-				   "notify handler is installed\n");
-
-	return AE_OK;
-}
-
-void __init acpi_pci_root_hp_init(void)
-{
-	int num = 0;
-
-	acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
-		ACPI_UINT32_MAX, find_root_bridges, NULL, &num, NULL);
 
-	printk(KERN_DEBUG "Found %d acpi root devices\n", num);
+	pci_acpi_crs_quirks();
+	acpi_scan_add_handler_with_hotplug(&pci_root_handler, "pci_root");
 }
Index: linux-pm/drivers/acpi/internal.h
===================================================================
--- linux-pm.orig/drivers/acpi/internal.h
+++ linux-pm/drivers/acpi/internal.h
@@ -28,7 +28,6 @@ int init_acpi_device_notify(void);
 int acpi_scan_init(void);
 void acpi_pci_root_init(void);
 void acpi_pci_link_init(void);
-void acpi_pci_root_hp_init(void);
 void acpi_processor_init(void);
 void acpi_platform_init(void);
 int acpi_sysfs_init(void);
@@ -89,7 +88,6 @@ 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_device_hotplug(void *data, u32 ost_src);
 bool acpi_device_is_present(struct acpi_device *adev);
 
 /* --------------------------------------------------------------------------

[PATCH 7/10] ACPI / hotplug: Move container-specific code out of the core

[Rafael J. Wysocki]

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

Move container-specific uevents from the core hotplug code to the
container scan handler's .attach() and .detach() callbacks.

This way the core will not have to special-case containers and
the uevents will be guaranteed to happen every time a container
is either scanned or trimmed as appropriate.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/container.c |   11 ++++++++---
 drivers/acpi/scan.c      |    8 +-------
 include/acpi/acpi_bus.h  |    7 -------
 3 files changed, 9 insertions(+), 17 deletions(-)

Index: linux-pm/drivers/acpi/container.c
===================================================================
--- linux-pm.orig/drivers/acpi/container.c
+++ linux-pm/drivers/acpi/container.c
@@ -44,19 +44,24 @@ static const struct acpi_device_id conta
 	{"", 0},
 };
 
-static int container_device_attach(struct acpi_device *device,
+static int container_device_attach(struct acpi_device *adev,
 				   const struct acpi_device_id *not_used)
 {
-	/* This is necessary for container hotplug to work. */
+	kobject_uevent(&adev->dev.kobj, KOBJ_ONLINE);
 	return 1;
 }
 
+static void container_device_detach(struct acpi_device *adev)
+{
+	kobject_uevent(&adev->dev.kobj, KOBJ_OFFLINE);
+}
+
 static struct acpi_scan_handler container_handler = {
 	.ids = container_device_ids,
 	.attach = container_device_attach,
+	.detach = container_device_detach,
 	.hotplug = {
 		.enabled = true,
-		.mode = AHM_CONTAINER,
 	},
 };
 
Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -206,9 +206,6 @@ static int acpi_scan_hot_remove(struct a
 	acpi_status status;
 	unsigned long long sta;
 
-	if (device->handler && device->handler->hotplug.mode == AHM_CONTAINER)
-		kobject_uevent(&device->dev.kobj, KOBJ_OFFLINE);
-
 	/*
 	 * Carry out two passes here and ignore errors in the first pass,
 	 * because if the devices in question are memory blocks and
@@ -288,10 +285,7 @@ static int acpi_scan_device_check(struct
 		dev_warn(&adev->dev, "Namespace scan failure\n");
 		return error;
 	}
-	if (adev->handler) {
-		if (adev->handler->hotplug.mode == AHM_CONTAINER)
-			kobject_uevent(&adev->dev.kobj, KOBJ_ONLINE);
-	} else {
+	if (!adev->handler) {
 		dev_warn(&adev->dev, "Enumeration failure\n");
 		return -ENODEV;
 	}
Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -91,16 +91,9 @@ struct acpi_device;
  * -----------------
  */
 
-enum acpi_hotplug_mode {
-	AHM_GENERIC = 0,
-	AHM_CONTAINER,
-	AHM_COUNT
-};
-
 struct acpi_hotplug_profile {
 	struct kobject kobj;
 	bool enabled:1;
-	enum acpi_hotplug_mode mode;
 	int (*scan_dependent)(struct acpi_device *adev);
 };
 

Re: [PATCH 7/10] ACPI / hotplug: Move container-specific code out of the core

[Yasuaki Ishimatsu]

Hi Rafael,

Replying to this mail may be wrong.

Do you remember following your patch?
http://lkml.org/lkml/2013/2/23/97

I want to add autoeject variable in acpi_hotplug_profile structure and
set autoecjet of container device "false".

Currently, I have a problem on ejecting container device. Since linux-3.12,
container device is removed by acpi_scan_hot_remove.

I think this has two problems.

   1. easily fail
      My container device has CPU device and Memory device, and maximum size of
      memory is 3Tbyte. In my environment, hot removing container device fails
      on offlining memory if memory is used by application.
      I think if offlininig memory, we must retly to offline memory several
      times.
   2. cannot work with userland's application
      Hot removing CPU and memory on container device, we need take care of
      userland application. Before linux-3.12, container device just notifies
      KOBJ_OFFLINE to udev. So by using udev, if application binds to removed
      CPU or node, applications can change them before hot removing container
      device.
      Currently, KOBJ_OFFLINE is notified to udev. But acpi_scan_hot_remove
      also runs simultaneously for hot removing container device. So when
      applications runs for corresponding to the deletion of the devices,
      the devices may have been deleted.

I don't know what devices are on hotpluggable conatainer device of other
vendors. At least, my container device cannot be hot removed correctly.
Then I want to add autoeject variable in acpi_hotplug_profile so that user
can change the parameter to "true" or "false".

Thanks,
Yasuaki Ishimatsu

(2013/11/18 1:36), Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Move container-specific uevents from the core hotplug code to the
> container scan handler's .attach() and .detach() callbacks.
>
> This way the core will not have to special-case containers and
> the uevents will be guaranteed to happen every time a container
> is either scanned or trimmed as appropriate.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>   drivers/acpi/container.c |   11 ++++++++---
>   drivers/acpi/scan.c      |    8 +-------
>   include/acpi/acpi_bus.h  |    7 -------
>   3 files changed, 9 insertions(+), 17 deletions(-)
>
> Index: linux-pm/drivers/acpi/container.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/container.c
> +++ linux-pm/drivers/acpi/container.c
> @@ -44,19 +44,24 @@ static const struct acpi_device_id conta
>   	{"", 0},
>   };
>
> -static int container_device_attach(struct acpi_device *device,
> +static int container_device_attach(struct acpi_device *adev,
>   				   const struct acpi_device_id *not_used)
>   {
> -	/* This is necessary for container hotplug to work. */
> +	kobject_uevent(&adev->dev.kobj, KOBJ_ONLINE);
>   	return 1;
>   }
>
> +static void container_device_detach(struct acpi_device *adev)
> +{
> +	kobject_uevent(&adev->dev.kobj, KOBJ_OFFLINE);
> +}
> +
>   static struct acpi_scan_handler container_handler = {
>   	.ids = container_device_ids,
>   	.attach = container_device_attach,
> +	.detach = container_device_detach,
>   	.hotplug = {
>   		.enabled = true,
> -		.mode = AHM_CONTAINER,
>   	},
>   };
>
> Index: linux-pm/drivers/acpi/scan.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/scan.c
> +++ linux-pm/drivers/acpi/scan.c
> @@ -206,9 +206,6 @@ static int acpi_scan_hot_remove(struct a
>   	acpi_status status;
>   	unsigned long long sta;
>
> -	if (device->handler && device->handler->hotplug.mode == AHM_CONTAINER)
> -		kobject_uevent(&device->dev.kobj, KOBJ_OFFLINE);
> -
>   	/*
>   	 * Carry out two passes here and ignore errors in the first pass,
>   	 * because if the devices in question are memory blocks and
> @@ -288,10 +285,7 @@ static int acpi_scan_device_check(struct
>   		dev_warn(&adev->dev, "Namespace scan failure\n");
>   		return error;
>   	}
> -	if (adev->handler) {
> -		if (adev->handler->hotplug.mode == AHM_CONTAINER)
> -			kobject_uevent(&adev->dev.kobj, KOBJ_ONLINE);
> -	} else {
> +	if (!adev->handler) {
>   		dev_warn(&adev->dev, "Enumeration failure\n");
>   		return -ENODEV;
>   	}
> Index: linux-pm/include/acpi/acpi_bus.h
> ===================================================================
> --- linux-pm.orig/include/acpi/acpi_bus.h
> +++ linux-pm/include/acpi/acpi_bus.h
> @@ -91,16 +91,9 @@ struct acpi_device;
>    * -----------------
>    */
>
> -enum acpi_hotplug_mode {
> -	AHM_GENERIC = 0,
> -	AHM_CONTAINER,
> -	AHM_COUNT
> -};
> -
>   struct acpi_hotplug_profile {
>   	struct kobject kobj;
>   	bool enabled:1;
> -	enum acpi_hotplug_mode mode;
>   	int (*scan_dependent)(struct acpi_device *adev);
>   };
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH 7/10] ACPI / hotplug: Move container-specific code out of the core

[Rafael J. Wysocki]

On Friday, November 29, 2013 11:36:55 AM Yasuaki Ishimatsu wrote:
> Hi Rafael,

Hi,

> Replying to this mail may be wrong.

OK, so this particular patch doesn't break things any more?

> Do you remember following your patch?
> http://lkml.org/lkml/2013/2/23/97
> 
> I want to add autoeject variable in acpi_hotplug_profile structure and
> set autoecjet of container device "false".

Then after the series the $subject patch belongs to it will work almost the
same way as /sys/firmware/acpi/container/enabled (hot add will still work after
patch [4/10] if "enabled" is 0), but only for containers.

> Currently, I have a problem on ejecting container device. Since linux-3.12,
> container device is removed by acpi_scan_hot_remove.
> 
> I think this has two problems.
> 
>    1. easily fail
>       My container device has CPU device and Memory device, and maximum size of
>       memory is 3Tbyte. In my environment, hot removing container device fails
>       on offlining memory if memory is used by application.
>       I think if offlininig memory, we must retly to offline memory several
>       times.

Yes, that's correct.  But then you can try to offline the memory upfront
and only remove the container after that has been successful.

>    2. cannot work with userland's application
>       Hot removing CPU and memory on container device, we need take care of
>       userland application. Before linux-3.12, container device just notifies
>       KOBJ_OFFLINE to udev. So by using udev, if application binds to removed
>       CPU or node, applications can change them before hot removing container
>       device.
>       Currently, KOBJ_OFFLINE is notified to udev. But acpi_scan_hot_remove
>       also runs simultaneously for hot removing container device. So when
>       applications runs for corresponding to the deletion of the devices,
>       the devices may have been deleted.

So the expectation is that the container will refuse to offline, but instead
it will emit KOBJ_OFFLINE so that user space can do some cleanup and offline
it through the "eject" attribute, right?

> I don't know what devices are on hotpluggable conatainer device of other
> vendors. At least, my container device cannot be hot removed correctly.
> Then I want to add autoeject variable in acpi_hotplug_profile so that user
> can change the parameter to "true" or "false".

I have a different idea.

Why don't we create a bus type for containers in analogy with CPUs and memory
and make it support offline.  Then, the container scan handler will create a
"physical" container device under that bus type and the new bus type code will
implement the logic you need (that is, it will have a sysfs flag that will
cause the offline to fail emitting a uevent of some sort if set and will allow
the offline to happen when unset).  That "physical" container device will go
away (again, via the container scan handler) during container removal.

The eject work flow can be:
  (1) an eject event occurs,
  (2) the container "physical" device fails offline in acpi_scan_hot_remove()
      emmitting, say, KOBJ_CHANGE for the "physical" device,
  (3) user space notices the KOBJ_CHANGE and does the cleanup as needed,
  (4) user space changes the "physical" container device flag controlling
      offline to 0,
  (5) user space uses the sysfs "eject" attribute of the ACPI container object
      to finally eject the container,
  (6) the offline in acpi_scan_hot_remove() is now successful, because the
      flag controlling it has been set to 0 in step (4),
  (7) the "physical" container device goes away before executing _EJ0,
  (8) the container is ejected.

Of course, if the flag controlling container offline is 0 to start with, step
(6) will now occur directly after (1), so whoever wants containers to be
hot-removed automatically may just clear that flag for all of them on boot.

How does that sound?

Rafael

Re: [PATCH 7/10] ACPI / hotplug: Move container-specific code out of the core

[Yasuaki Ishimatsu]

(2013/11/29 22:08), Rafael J. Wysocki wrote:
> On Friday, November 29, 2013 11:36:55 AM Yasuaki Ishimatsu wrote:
>> Hi Rafael,
>
> Hi,
>
>> Replying to this mail may be wrong.
>
> OK, so this particular patch doesn't break things any more?

Yes.

>
>> Do you remember following your patch?
>> http://lkml.org/lkml/2013/2/23/97
>>
>> I want to add autoeject variable in acpi_hotplug_profile structure and
>> set autoecjet of container device "false".
>
> Then after the series the $subject patch belongs to it will work almost the
> same way as /sys/firmware/acpi/container/enabled (hot add will still work after
> patch [4/10] if "enabled" is 0), but only for containers.
>
>> Currently, I have a problem on ejecting container device. Since linux-3.12,
>> container device is removed by acpi_scan_hot_remove.
>>
>> I think this has two problems.
>>
>>     1. easily fail
>>        My container device has CPU device and Memory device, and maximum size of
>>        memory is 3Tbyte. In my environment, hot removing container device fails
>>        on offlining memory if memory is used by application.
>>        I think if offlininig memory, we must retly to offline memory several
>>        times.
>
> Yes, that's correct.  But then you can try to offline the memory upfront
> and only remove the container after that has been successful.
>
>>     2. cannot work with userland's application
>>        Hot removing CPU and memory on container device, we need take care of
>>        userland application. Before linux-3.12, container device just notifies
>>        KOBJ_OFFLINE to udev. So by using udev, if application binds to removed
>>        CPU or node, applications can change them before hot removing container
>>        device.
>>        Currently, KOBJ_OFFLINE is notified to udev. But acpi_scan_hot_remove
>>        also runs simultaneously for hot removing container device. So when
>>        applications runs for corresponding to the deletion of the devices,
>>        the devices may have been deleted.
>

> So the expectation is that the container will refuse to offline, but instead
> it will emit KOBJ_OFFLINE so that user space can do some cleanup and offline
> it through the "eject" attribute, right?

Yes, that's right.

>
>> I don't know what devices are on hotpluggable conatainer device of other
>> vendors. At least, my container device cannot be hot removed correctly.
>> Then I want to add autoeject variable in acpi_hotplug_profile so that user
>> can change the parameter to "true" or "false".
>
> I have a different idea.
>
> Why don't we create a bus type for containers in analogy with CPUs and memory
> and make it support offline.  Then, the container scan handler will create a
> "physical" container device under that bus type and the new bus type code will
> implement the logic you need (that is, it will have a sysfs flag that will
> cause the offline to fail emitting a uevent of some sort if set and will allow
> the offline to happen when unset).  That "physical" container device will go
> away (again, via the container scan handler) during container removal.
>

> The eject work flow can be:
>    (1) an eject event occurs,
>    (2) the container "physical" device fails offline in acpi_scan_hot_remove()
>        emmitting, say, KOBJ_CHANGE for the "physical" device,
>    (3) user space notices the KOBJ_CHANGE and does the cleanup as needed,
>    (4) user space changes the "physical" container device flag controlling
>        offline to 0,
>    (5) user space uses the sysfs "eject" attribute of the ACPI container object
>        to finally eject the container,
>    (6) the offline in acpi_scan_hot_remove() is now successful, because the
>        flag controlling it has been set to 0 in step (4),
>    (7) the "physical" container device goes away before executing _EJ0,
>    (8) the container is ejected.
>
> Of course, if the flag controlling container offline is 0 to start with, step
> (6) will now occur directly after (1), so whoever wants containers to be
> hot-removed automatically may just clear that flag for all of them on boot.
>
> How does that sound?


The above ideas are almost O.K. I want kernel to notify user space of KOBJ_OFFLINE.
Even if user space catches "KOBJ_CHANGE", user doesn't know whether the notification
is offline or not.

Thanks,
Yasuaki Ishimatsu

> Rafael
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH 7/10] ACPI / hotplug: Move container-specific code out of the core

[Rafael J. Wysocki]

On Tuesday, December 03, 2013 11:46:24 AM Yasuaki Ishimatsu wrote:
> (2013/11/29 22:08), Rafael J. Wysocki wrote:
> > On Friday, November 29, 2013 11:36:55 AM Yasuaki Ishimatsu wrote:
> >> Hi Rafael,
> >
> > Hi,
> >
> >> Replying to this mail may be wrong.
> >
> > OK, so this particular patch doesn't break things any more?
> 
> Yes.
> 
> >
> >> Do you remember following your patch?
> >> http://lkml.org/lkml/2013/2/23/97
> >>
> >> I want to add autoeject variable in acpi_hotplug_profile structure and
> >> set autoecjet of container device "false".
> >
> > Then after the series the $subject patch belongs to it will work almost the
> > same way as /sys/firmware/acpi/container/enabled (hot add will still work after
> > patch [4/10] if "enabled" is 0), but only for containers.
> >
> >> Currently, I have a problem on ejecting container device. Since linux-3.12,
> >> container device is removed by acpi_scan_hot_remove.
> >>
> >> I think this has two problems.
> >>
> >>     1. easily fail
> >>        My container device has CPU device and Memory device, and maximum size of
> >>        memory is 3Tbyte. In my environment, hot removing container device fails
> >>        on offlining memory if memory is used by application.
> >>        I think if offlininig memory, we must retly to offline memory several
> >>        times.
> >
> > Yes, that's correct.  But then you can try to offline the memory upfront
> > and only remove the container after that has been successful.
> >
> >>     2. cannot work with userland's application
> >>        Hot removing CPU and memory on container device, we need take care of
> >>        userland application. Before linux-3.12, container device just notifies
> >>        KOBJ_OFFLINE to udev. So by using udev, if application binds to removed
> >>        CPU or node, applications can change them before hot removing container
> >>        device.
> >>        Currently, KOBJ_OFFLINE is notified to udev. But acpi_scan_hot_remove
> >>        also runs simultaneously for hot removing container device. So when
> >>        applications runs for corresponding to the deletion of the devices,
> >>        the devices may have been deleted.
> >
> 
> > So the expectation is that the container will refuse to offline, but instead
> > it will emit KOBJ_OFFLINE so that user space can do some cleanup and offline
> > it through the "eject" attribute, right?
> 
> Yes, that's right.
> 
> >
> >> I don't know what devices are on hotpluggable conatainer device of other
> >> vendors. At least, my container device cannot be hot removed correctly.
> >> Then I want to add autoeject variable in acpi_hotplug_profile so that user
> >> can change the parameter to "true" or "false".
> >
> > I have a different idea.
> >
> > Why don't we create a bus type for containers in analogy with CPUs and memory
> > and make it support offline.  Then, the container scan handler will create a
> > "physical" container device under that bus type and the new bus type code will
> > implement the logic you need (that is, it will have a sysfs flag that will
> > cause the offline to fail emitting a uevent of some sort if set and will allow
> > the offline to happen when unset).  That "physical" container device will go
> > away (again, via the container scan handler) during container removal.
> >
> 
> > The eject work flow can be:
> >    (1) an eject event occurs,
> >    (2) the container "physical" device fails offline in acpi_scan_hot_remove()
> >        emmitting, say, KOBJ_CHANGE for the "physical" device,
> >    (3) user space notices the KOBJ_CHANGE and does the cleanup as needed,
> >    (4) user space changes the "physical" container device flag controlling
> >        offline to 0,
> >    (5) user space uses the sysfs "eject" attribute of the ACPI container object
> >        to finally eject the container,
> >    (6) the offline in acpi_scan_hot_remove() is now successful, because the
> >        flag controlling it has been set to 0 in step (4),
> >    (7) the "physical" container device goes away before executing _EJ0,
> >    (8) the container is ejected.
> >
> > Of course, if the flag controlling container offline is 0 to start with, step
> > (6) will now occur directly after (1), so whoever wants containers to be
> > hot-removed automatically may just clear that flag for all of them on boot.
> >
> > How does that sound?
> 
> 
> The above ideas are almost O.K. I want kernel to notify user space of KOBJ_OFFLINE.
> Even if user space catches "KOBJ_CHANGE", user doesn't know whether the notification
> is offline or not.

It is easy to figure out, though.  Since the KOBJ_CHANGE will be emitted for
container devices only in that situation, user space can see that (1) it is
from a container and (2) it is KOBJ_CHANGE, so it must mean "container offline
has been attempted".

My concern with using KOBJ_OFFLINE for that is that device_offline() emits it
too on success and it may be easily confused with the one emitted on failure
for containers.

Thanks,
Rafael

Re: [PATCH 7/10] ACPI / hotplug: Move container-specific code out of the core

[Yasuaki Ishimatsu]

(2013/12/03 22:15), Rafael J. Wysocki wrote:
> On Tuesday, December 03, 2013 11:46:24 AM Yasuaki Ishimatsu wrote:
>> (2013/11/29 22:08), Rafael J. Wysocki wrote:
>>> On Friday, November 29, 2013 11:36:55 AM Yasuaki Ishimatsu wrote:
>>>> Hi Rafael,
>>>
>>> Hi,
>>>
>>>> Replying to this mail may be wrong.
>>>
>>> OK, so this particular patch doesn't break things any more?
>>
>> Yes.
>>
>>>
>>>> Do you remember following your patch?
>>>> http://lkml.org/lkml/2013/2/23/97
>>>>
>>>> I want to add autoeject variable in acpi_hotplug_profile structure and
>>>> set autoecjet of container device "false".
>>>
>>> Then after the series the $subject patch belongs to it will work almost the
>>> same way as /sys/firmware/acpi/container/enabled (hot add will still work after
>>> patch [4/10] if "enabled" is 0), but only for containers.
>>>
>>>> Currently, I have a problem on ejecting container device. Since linux-3.12,
>>>> container device is removed by acpi_scan_hot_remove.
>>>>
>>>> I think this has two problems.
>>>>
>>>>      1. easily fail
>>>>         My container device has CPU device and Memory device, and maximum size of
>>>>         memory is 3Tbyte. In my environment, hot removing container device fails
>>>>         on offlining memory if memory is used by application.
>>>>         I think if offlininig memory, we must retly to offline memory several
>>>>         times.
>>>
>>> Yes, that's correct.  But then you can try to offline the memory upfront
>>> and only remove the container after that has been successful.
>>>
>>>>      2. cannot work with userland's application
>>>>         Hot removing CPU and memory on container device, we need take care of
>>>>         userland application. Before linux-3.12, container device just notifies
>>>>         KOBJ_OFFLINE to udev. So by using udev, if application binds to removed
>>>>         CPU or node, applications can change them before hot removing container
>>>>         device.
>>>>         Currently, KOBJ_OFFLINE is notified to udev. But acpi_scan_hot_remove
>>>>         also runs simultaneously for hot removing container device. So when
>>>>         applications runs for corresponding to the deletion of the devices,
>>>>         the devices may have been deleted.
>>>
>>
>>> So the expectation is that the container will refuse to offline, but instead
>>> it will emit KOBJ_OFFLINE so that user space can do some cleanup and offline
>>> it through the "eject" attribute, right?
>>
>> Yes, that's right.
>>
>>>
>>>> I don't know what devices are on hotpluggable conatainer device of other
>>>> vendors. At least, my container device cannot be hot removed correctly.
>>>> Then I want to add autoeject variable in acpi_hotplug_profile so that user
>>>> can change the parameter to "true" or "false".
>>>
>>> I have a different idea.
>>>
>>> Why don't we create a bus type for containers in analogy with CPUs and memory
>>> and make it support offline.  Then, the container scan handler will create a
>>> "physical" container device under that bus type and the new bus type code will
>>> implement the logic you need (that is, it will have a sysfs flag that will
>>> cause the offline to fail emitting a uevent of some sort if set and will allow
>>> the offline to happen when unset).  That "physical" container device will go
>>> away (again, via the container scan handler) during container removal.
>>>
>>
>>> The eject work flow can be:
>>>     (1) an eject event occurs,
>>>     (2) the container "physical" device fails offline in acpi_scan_hot_remove()
>>>         emmitting, say, KOBJ_CHANGE for the "physical" device,
>>>     (3) user space notices the KOBJ_CHANGE and does the cleanup as needed,
>>>     (4) user space changes the "physical" container device flag controlling
>>>         offline to 0,
>>>     (5) user space uses the sysfs "eject" attribute of the ACPI container object
>>>         to finally eject the container,
>>>     (6) the offline in acpi_scan_hot_remove() is now successful, because the
>>>         flag controlling it has been set to 0 in step (4),
>>>     (7) the "physical" container device goes away before executing _EJ0,
>>>     (8) the container is ejected.
>>>
>>> Of course, if the flag controlling container offline is 0 to start with, step
>>> (6) will now occur directly after (1), so whoever wants containers to be
>>> hot-removed automatically may just clear that flag for all of them on boot.
>>>
>>> How does that sound?
>>
>>
>> The above ideas are almost O.K. I want kernel to notify user space of KOBJ_OFFLINE.
>> Even if user space catches "KOBJ_CHANGE", user doesn't know whether the notification
>> is offline or not.
>

> It is easy to figure out, though.  Since the KOBJ_CHANGE will be emitted for
> container devices only in that situation, user space can see that (1) it is
> from a container and (2) it is KOBJ_CHANGE, so it must mean "container offline
> has been attempted".
>
> My concern with using KOBJ_OFFLINE for that is that device_offline() emits it
> too on success and it may be easily confused with the one emitted on failure
> for containers.

I have no objection.

Thanks,
Yasuaki Ishimatsu

>
> Thanks,
> Rafael
>

Re: [PATCH 7/10] ACPI / hotplug: Move container-specific code out of the core

[Yasuaki Ishimatsu]

Hi Rafael,

Please share your more detailed idea. I started to implement the following
idea. But the idea has one problem.

>>> The eject work flow can be:
>>>     (1) an eject event occurs,
>>>     (2) the container "physical" device fails offline in acpi_scan_hot_remove()
>>>         emmitting, say, KOBJ_CHANGE for the "physical" device,
>>>     (3) user space notices the KOBJ_CHANGE and does the cleanup as needed,
>>>     (4) user space changes the "physical" container device flag controlling
>>>         offline to 0,
>>>     (5) user space uses the sysfs "eject" attribute of the ACPI container object
>>>         to finally eject the container,
>>>     (6) the offline in acpi_scan_hot_remove() is now successful, because the
>>>         flag controlling it has been set to 0 in step (4),
>>>     (7) the "physical" container device goes away before executing _EJ0,
>>>     (8) the container is ejected.

I want to emit KOBJ_CHANGE before offlining devices on container device at (2).
But acpi_scan_hot_remove() offlines devices on container device at first.
So when offline container device, devices on container has been offlined.

Thus the idea cannot fulfill my necessary feature.

Thanks,
Yasauaki Ishimatsu

Thanks,
Yasauaki Ishimatsu

>>>
>>> Of course, if the flag controlling container offline is 0 to start with, step
>>> (6) will now occur directly after (1), so whoever wants containers to be
>>> hot-removed automatically may just clear that flag for all of them on boot.
>>>
>>> How does that sound?
>>
>>
>> The above ideas are almost O.K. I want kernel to notify user space of KOBJ_OFFLINE.
>> Even if user space catches "KOBJ_CHANGE", user doesn't know whether the notification
>> is offline or not.
>
> It is easy to figure out, though.  Since the KOBJ_CHANGE will be emitted for
> container devices only in that situation, user space can see that (1) it is
> from a container and (2) it is KOBJ_CHANGE, so it must mean "container offline
> has been attempted".
>
> My concern with using KOBJ_OFFLINE for that is that device_offline() emits it
> too on success and it may be easily confused with the one emitted on failure
> for containers.
>
> Thanks,
> Rafael
>

Re: [PATCH 7/10] ACPI / hotplug: Move container-specific code out of the core

[Rafael J. Wysocki]

On Friday, December 13, 2013 11:56:32 AM Yasuaki Ishimatsu wrote:
> Hi Rafael,

Hi,

> Please share your more detailed idea. I started to implement the following
> idea. But the idea has one problem.
> 
> >>> The eject work flow can be:
> >>>     (1) an eject event occurs,
> >>>     (2) the container "physical" device fails offline in acpi_scan_hot_remove()
> >>>         emmitting, say, KOBJ_CHANGE for the "physical" device,
> >>>     (3) user space notices the KOBJ_CHANGE and does the cleanup as needed,
> >>>     (4) user space changes the "physical" container device flag controlling
> >>>         offline to 0,
> >>>     (5) user space uses the sysfs "eject" attribute of the ACPI container object
> >>>         to finally eject the container,
> >>>     (6) the offline in acpi_scan_hot_remove() is now successful, because the
> >>>         flag controlling it has been set to 0 in step (4),
> >>>     (7) the "physical" container device goes away before executing _EJ0,
> >>>     (8) the container is ejected.
> 
> I want to emit KOBJ_CHANGE before offlining devices on container device at (2).
> But acpi_scan_hot_remove() offlines devices on container device at first.
> So when offline container device, devices on container has been offlined.
> 
> Thus the idea cannot fulfill my necessary feature.

Well, in that case we need to treat containers in a special way at the ACPI
level.  Which is a bit unfortunate so to speak.

To that end I'd try to add a new flag to struct acpi_hotplug_profile, say
.verify_offline, such that if set, it would cause acpi_scan_hot_remove() to
check if all of the "physical" companions of the top-level device are offline
to start with, and if not, it would just emit KOBJ_CHANGE for the companions
that are not offline and bail out.

So the above algorithm would become:

(1) an eject event occurs,
(2) acpi_scan_hot_remove() checks the verify_offline flag in the target device's
    scan_handler structure,
(3) if set (it would always be set for containers), acpi_scan_hot_remove()
    checks the status of the target device's "physical" companions; if at least
    one of them is offline, KOBJ_CHANGE is emitted for that "physical" device,
    and acpi_scan_hot_remove() returns, [I guess we can just emit KOBJ_CHANGE
    for the first companion that is not offline at this point.]
(4) user space notices the KOBJ_CHANGE and does the cleanup as needed; in the
    process it carries out the offline operation for the container's "physical"
    companion (there's only one such companion for each container), [That
    operation for the container itself is trivial, but to succeed it requires
    all devices below the container to be taken offline in advance.]
(5) user space uses the sysfs "eject" attribute of the ACPI container object
    to finally eject the container,
(6) acpi_scan_hot_remove() is now successful, because the container's "physical"
    companion is now offline,
(7) the "physical" container device goes away before executing _EJ0,
(8) the container is ejected.

I think that should work for you.

Thanks,
Rafael

Re: [PATCH 7/10] ACPI / hotplug: Move container-specific code out of the core

[Yasuaki Ishimatsu]

(2013/12/13 13:56), Rafael J. Wysocki wrote:
> On Friday, December 13, 2013 11:56:32 AM Yasuaki Ishimatsu wrote:
>> Hi Rafael,
>
> Hi,
>
>> Please share your more detailed idea. I started to implement the following
>> idea. But the idea has one problem.
>>
>>>>> The eject work flow can be:
>>>>>      (1) an eject event occurs,
>>>>>      (2) the container "physical" device fails offline in acpi_scan_hot_remove()
>>>>>          emmitting, say, KOBJ_CHANGE for the "physical" device,
>>>>>      (3) user space notices the KOBJ_CHANGE and does the cleanup as needed,
>>>>>      (4) user space changes the "physical" container device flag controlling
>>>>>          offline to 0,
>>>>>      (5) user space uses the sysfs "eject" attribute of the ACPI container object
>>>>>          to finally eject the container,
>>>>>      (6) the offline in acpi_scan_hot_remove() is now successful, because the
>>>>>          flag controlling it has been set to 0 in step (4),
>>>>>      (7) the "physical" container device goes away before executing _EJ0,
>>>>>      (8) the container is ejected.
>>
>> I want to emit KOBJ_CHANGE before offlining devices on container device at (2).
>> But acpi_scan_hot_remove() offlines devices on container device at first.
>> So when offline container device, devices on container has been offlined.
>>
>> Thus the idea cannot fulfill my necessary feature.
>
> Well, in that case we need to treat containers in a special way at the ACPI
> level.  Which is a bit unfortunate so to speak.
>
> To that end I'd try to add a new flag to struct acpi_hotplug_profile, say
> .verify_offline, such that if set, it would cause acpi_scan_hot_remove() to
> check if all of the "physical" companions of the top-level device are offline
> to start with, and if not, it would just emit KOBJ_CHANGE for the companions
> that are not offline and bail out.
>
> So the above algorithm would become:
>
> (1) an eject event occurs,
> (2) acpi_scan_hot_remove() checks the verify_offline flag in the target device's
>      scan_handler structure,
> (3) if set (it would always be set for containers), acpi_scan_hot_remove()
>      checks the status of the target device's "physical" companions; if at least
>      one of them is offline, KOBJ_CHANGE is emitted for that "physical" device,
>      and acpi_scan_hot_remove() returns, [I guess we can just emit KOBJ_CHANGE
>      for the first companion that is not offline at this point.]
> (4) user space notices the KOBJ_CHANGE and does the cleanup as needed; in the
>      process it carries out the offline operation for the container's "physical"
>      companion (there's only one such companion for each container), [That
>      operation for the container itself is trivial, but to succeed it requires
>      all devices below the container to be taken offline in advance.]
> (5) user space uses the sysfs "eject" attribute of the ACPI container object
>      to finally eject the container,
> (6) acpi_scan_hot_remove() is now successful, because the container's "physical"
>      companion is now offline,
> (7) the "physical" container device goes away before executing _EJ0,
> (8) the container is ejected.
>
> I think that should work for you.

This idea seems to same as your previous work.
http://lkml.org/lkml/2013/2/23/97

How about add autoremove flag into acpi_hotplug_profile and check it as follow:

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

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 5383c81..c43d110 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -409,6 +409,11 @@ static void acpi_hotplug_notify_cb(acpi_handle handle, u32 type, void *data)
  			ost_code = ACPI_OST_SC_EJECT_NOT_SUPPORTED;
  			goto err_out;
  		}
+		if (!handler->hotplug.autoremove) {
+			kobject_uevent(&device->dev.kobj, KOBJ_CHANGE);
+			ost_code = ACPI_OST_SC_EJECT_NON_SPECIFIC_FAILURE;
+			goto err_out;
+		}
  		acpi_evaluate_hotplug_ost(handle, ACPI_NOTIFY_EJECT_REQUEST,
  					  ACPI_OST_SC_EJECT_IN_PROGRESS, NULL);
  		break;

Adding the check into "acpi_hotplug_notify_cb()", user need not change the
flag for removing container device by "sysfs eject".

Thanks,
Yasuaki Ishimatsu

>
> Thanks,
> Rafael
>

Re: [PATCH 7/10] ACPI / hotplug: Move container-specific code out of the core

[Rafael J. Wysocki]

On Friday, December 13, 2013 02:17:32 PM Yasuaki Ishimatsu wrote:
> (2013/12/13 13:56), Rafael J. Wysocki wrote:
> > On Friday, December 13, 2013 11:56:32 AM Yasuaki Ishimatsu wrote:
> >> Hi Rafael,
> >
> > Hi,
> >
> >> Please share your more detailed idea. I started to implement the following
> >> idea. But the idea has one problem.
> >>
> >>>>> The eject work flow can be:
> >>>>>      (1) an eject event occurs,
> >>>>>      (2) the container "physical" device fails offline in acpi_scan_hot_remove()
> >>>>>          emmitting, say, KOBJ_CHANGE for the "physical" device,
> >>>>>      (3) user space notices the KOBJ_CHANGE and does the cleanup as needed,
> >>>>>      (4) user space changes the "physical" container device flag controlling
> >>>>>          offline to 0,
> >>>>>      (5) user space uses the sysfs "eject" attribute of the ACPI container object
> >>>>>          to finally eject the container,
> >>>>>      (6) the offline in acpi_scan_hot_remove() is now successful, because the
> >>>>>          flag controlling it has been set to 0 in step (4),
> >>>>>      (7) the "physical" container device goes away before executing _EJ0,
> >>>>>      (8) the container is ejected.
> >>
> >> I want to emit KOBJ_CHANGE before offlining devices on container device at (2).
> >> But acpi_scan_hot_remove() offlines devices on container device at first.
> >> So when offline container device, devices on container has been offlined.
> >>
> >> Thus the idea cannot fulfill my necessary feature.
> >
> > Well, in that case we need to treat containers in a special way at the ACPI
> > level.  Which is a bit unfortunate so to speak.
> >
> > To that end I'd try to add a new flag to struct acpi_hotplug_profile, say
> > .verify_offline, such that if set, it would cause acpi_scan_hot_remove() to
> > check if all of the "physical" companions of the top-level device are offline
> > to start with, and if not, it would just emit KOBJ_CHANGE for the companions
> > that are not offline and bail out.
> >
> > So the above algorithm would become:
> >
> > (1) an eject event occurs,
> > (2) acpi_scan_hot_remove() checks the verify_offline flag in the target device's
> >      scan_handler structure,
> > (3) if set (it would always be set for containers), acpi_scan_hot_remove()
> >      checks the status of the target device's "physical" companions; if at least
> >      one of them is offline, KOBJ_CHANGE is emitted for that "physical" device,
> >      and acpi_scan_hot_remove() returns, [I guess we can just emit KOBJ_CHANGE
> >      for the first companion that is not offline at this point.]
> > (4) user space notices the KOBJ_CHANGE and does the cleanup as needed; in the
> >      process it carries out the offline operation for the container's "physical"
> >      companion (there's only one such companion for each container), [That
> >      operation for the container itself is trivial, but to succeed it requires
> >      all devices below the container to be taken offline in advance.]
> > (5) user space uses the sysfs "eject" attribute of the ACPI container object
> >      to finally eject the container,
> > (6) acpi_scan_hot_remove() is now successful, because the container's "physical"
> >      companion is now offline,
> > (7) the "physical" container device goes away before executing _EJ0,
> > (8) the container is ejected.
> >
> > I think that should work for you.
> 
> This idea seems to same as your previous work.
> http://lkml.org/lkml/2013/2/23/97

No, it is not.  That one didn't involve physical device representations.

> How about add autoremove flag into acpi_hotplug_profile and check it as follow:

This is very similar to "enable" except that it generates the uevent and
"enable" doesn't.  You might as well modify "enable" to trigger a uevent if
eject is not enabled (note that with the latest patches in linux-next "enable"
only applies to eject).

That said I don't think we should generate any uevents for struct acpi_device
objects, because they are not devices.

> ---
>   drivers/acpi/scan.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 5383c81..c43d110 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -409,6 +409,11 @@ static void acpi_hotplug_notify_cb(acpi_handle handle, u32 type, void *data)
>   			ost_code = ACPI_OST_SC_EJECT_NOT_SUPPORTED;
>   			goto err_out;
>   		}
> +		if (!handler->hotplug.autoremove) {
> +			kobject_uevent(&device->dev.kobj, KOBJ_CHANGE);
> +			ost_code = ACPI_OST_SC_EJECT_NON_SPECIFIC_FAILURE;
> +			goto err_out;
> +		}
>   		acpi_evaluate_hotplug_ost(handle, ACPI_NOTIFY_EJECT_REQUEST,
>   					  ACPI_OST_SC_EJECT_IN_PROGRESS, NULL);
>   		break;
> 
> Adding the check into "acpi_hotplug_notify_cb()", user need not change the
> flag for removing container device by "sysfs eject".

Which is utterly confusing.  There is no reason whatsoever why the sysfs eject
attribute should work differently from the event-triggered eject.  Quite the
opposite is the case: it should work in the same way in my opinion so that it
is possible to test the eject code path using that attribute.

I'm traveling now, but when I get back home (next week), I'll try to implement
the thing I was talking about above.

Thanks,
Rafael

[PATCH 8/10] ACPI / hotplug: Rework generic code to handle suprise removals

[Rafael J. Wysocki]

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

The generic ACPI hotplug code used for several types of device
doesn't handle surprise removals, mostly because those devices
currently cannot be removed by surprise in the majority of systems.
However, surprise removals should be handled by that code as well
as surprise additions of devices, so make it do that.

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

Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -276,18 +276,72 @@ static int acpi_scan_hot_remove(struct a
 	return 0;
 }
 
+static int acpi_scan_device_not_present(struct acpi_device *adev)
+{
+	if (!acpi_device_enumerated(adev)) {
+		dev_warn(&adev->dev, "Still not present\n");
+		return -EALREADY;
+	}
+	acpi_bus_trim(adev);
+	return 0;
+}
+
 static int acpi_scan_device_check(struct acpi_device *adev)
 {
 	int error;
 
+	acpi_bus_get_status(adev);
+	if (adev->status.present || adev->status.functional) {
+		/*
+		 * This function is only called for device objects for which
+		 * matching scan handlers exist.  The only situation in which
+		 * the scan handler is not attached to this device object yet
+		 * is when the device has just appeared (either it wasn't
+		 * present at all before or it was removed and then added
+		 * again).
+		 */
+		if (adev->handler) {
+			dev_warn(&adev->dev, "Already enumerated\n");
+			return -EALREADY;
+		}
+		error = acpi_bus_scan(adev->handle);
+		if (error) {
+			dev_warn(&adev->dev, "Namespace scan failure\n");
+			return error;
+		}
+		if (!adev->handler) {
+			dev_warn(&adev->dev, "Enumeration failure\n");
+			error = -ENODEV;
+		}
+	} else {
+		error = acpi_scan_device_not_present(adev);
+	}
+	return error;
+}
+
+static int acpi_scan_bus_check(struct acpi_device *adev)
+{
+	struct acpi_scan_handler *handler = adev->handler;
+	struct acpi_device *child;
+	int error;
+
+	acpi_bus_get_status(adev);
+	if (!(adev->status.present || adev->status.functional)) {
+		acpi_scan_device_not_present(adev);
+		return 0;
+	}
+	if (handler && handler->hotplug.scan_dependent)
+		return handler->hotplug.scan_dependent(adev);
+
 	error = acpi_bus_scan(adev->handle);
 	if (error) {
 		dev_warn(&adev->dev, "Namespace scan failure\n");
 		return error;
 	}
-	if (!adev->handler) {
-		dev_warn(&adev->dev, "Enumeration failure\n");
-		return -ENODEV;
+	list_for_each_entry(child, &adev->children, node) {
+		error = acpi_scan_bus_check(child);
+		if (error)
+			return error;
 	}
 	return 0;
 }
@@ -296,7 +350,6 @@ static void acpi_device_hotplug(void *da
 {
 	u32 ost_code = ACPI_OST_SC_NON_SPECIFIC_FAILURE;
 	struct acpi_device *adev = data;
-	struct acpi_scan_handler *handler;
 	int error;
 
 	lock_device_hotplug();
@@ -310,32 +363,12 @@ static void acpi_device_hotplug(void *da
 	if (adev->handle == INVALID_ACPI_HANDLE)
 		goto out;
 
-	handler = adev->handler;
-
 	switch (src) {
 	case ACPI_NOTIFY_BUS_CHECK:
-		if (handler) {
-			error = handler->hotplug.scan_dependent ?
-					handler->hotplug.scan_dependent(adev) :
-					acpi_bus_scan(adev->handle);
-		} else {
-			error = acpi_scan_device_check(adev);
-		}
+		error = acpi_scan_bus_check(adev);
 		break;
 	case ACPI_NOTIFY_DEVICE_CHECK:
-		/*
-		 * This code is only run for device objects for which matching
-		 * scan handlers exist.  The only situation in which the scan
-		 * handler is not attached to this device object yet is when the
-		 * device has just appeared (either it wasn't present at all
-		 * before or it was removed and then added again).
-		 */
-		if (adev->handler) {
-			dev_warn(&adev->dev, "Already enumerated\n");
-			error = -EBUSY;
-		} else {
-			error = acpi_scan_device_check(adev);
-		}
+		error = acpi_scan_device_check(adev);
 		break;
 	case ACPI_NOTIFY_EJECT_REQUEST:
 	case ACPI_OST_EC_OSPM_EJECT:

[PATCH 9/10] ACPI / hotplug: Drop unfinished global notification handling routines

[Rafael J. Wysocki]

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

There are two global hotplug notification handling routines in bus.c,
acpi_bus_check_device() and acpi_bus_check_scope(), that have never
been finished and don't do anything useful, so drop them.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/bus.c |   64 +----------------------------------------------------
 1 file changed, 2 insertions(+), 62 deletions(-)

Index: linux-pm/drivers/acpi/bus.c
===================================================================
--- linux-pm.orig/drivers/acpi/bus.c
+++ linux-pm/drivers/acpi/bus.c
@@ -329,58 +329,6 @@ static void acpi_bus_osc_support(void)
                              Notification Handling
    -------------------------------------------------------------------------- */
 
-static void acpi_bus_check_device(acpi_handle handle)
-{
-	struct acpi_device *device;
-	acpi_status status;
-	struct acpi_device_status old_status;
-
-	if (acpi_bus_get_device(handle, &device))
-		return;
-	if (!device)
-		return;
-
-	old_status = device->status;
-
-	/*
-	 * Make sure this device's parent is present before we go about
-	 * messing with the device.
-	 */
-	if (device->parent && !device->parent->status.present) {
-		device->status = device->parent->status;
-		return;
-	}
-
-	status = acpi_bus_get_status(device);
-	if (ACPI_FAILURE(status))
-		return;
-
-	if (STRUCT_TO_INT(old_status) == STRUCT_TO_INT(device->status))
-		return;
-
-	/*
-	 * Device Insertion/Removal
-	 */
-	if ((device->status.present) && !(old_status.present)) {
-		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Device insertion detected\n"));
-		/* TBD: Handle device insertion */
-	} else if (!(device->status.present) && (old_status.present)) {
-		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Device removal detected\n"));
-		/* TBD: Handle device removal */
-	}
-}
-
-static void acpi_bus_check_scope(acpi_handle handle)
-{
-	/* Status Change? */
-	acpi_bus_check_device(handle);
-
-	/*
-	 * TBD: Enumerate child devices within this device's scope and
-	 *       run acpi_bus_check_device()'s on them.
-	 */
-}
-
 /**
  * acpi_bus_notify
  * ---------------
@@ -397,19 +345,11 @@ static void acpi_bus_notify(acpi_handle
 	switch (type) {
 
 	case ACPI_NOTIFY_BUS_CHECK:
-		acpi_bus_check_scope(handle);
-		/*
-		 * TBD: We'll need to outsource certain events to non-ACPI
-		 *      drivers via the device manager (device.c).
-		 */
+		/* TBD */
 		break;
 
 	case ACPI_NOTIFY_DEVICE_CHECK:
-		acpi_bus_check_device(handle);
-		/*
-		 * TBD: We'll need to outsource certain events to non-ACPI
-		 *      drivers via the device manager (device.c).
-		 */
+		/* TBD */
 		break;
 
 	case ACPI_NOTIFY_DEVICE_WAKE:

[PATCH 10/10] ACPI: Introduce acpi_set_device_status()

[Rafael J. Wysocki]

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

Introduce a static inline function for setting the status field
of struct acpi_device on the basis of a supplied u32 number,
acpi_set_device_status(), and use it instead of the horrible
horrible STRUCT_TO_INT() macro wherever applicable.  Having done
that, drop STRUCT_TO_INT() (and pretend that it has never existed).

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

Index: linux-pm/drivers/acpi/bus.c
===================================================================
--- linux-pm.orig/drivers/acpi/bus.c
+++ linux-pm/drivers/acpi/bus.c
@@ -52,9 +52,6 @@ struct acpi_device *acpi_root;
 struct proc_dir_entry *acpi_root_dir;
 EXPORT_SYMBOL(acpi_root_dir);
 
-#define STRUCT_TO_INT(s)	(*((int*)&s))
-
-
 #ifdef CONFIG_X86
 static int set_copy_dsdt(const struct dmi_system_id *id)
 {
@@ -115,18 +112,16 @@ int acpi_bus_get_status(struct acpi_devi
 	if (ACPI_FAILURE(status))
 		return -ENODEV;
 
-	STRUCT_TO_INT(device->status) = (int) sta;
+	acpi_set_device_status(device, sta);
 
 	if (device->status.functional && !device->status.present) {
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Device [%s] status [%08x]: "
 		       "functional but not present;\n",
-			device->pnp.bus_id,
-			(u32) STRUCT_TO_INT(device->status)));
+			device->pnp.bus_id, (u32)sta));
 	}
 
 	ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Device [%s] status [%08x]\n",
-			  device->pnp.bus_id,
-			  (u32) STRUCT_TO_INT(device->status)));
+			  device->pnp.bus_id, (u32)sta));
 	return 0;
 }
 EXPORT_SYMBOL(acpi_bus_get_status);
Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -20,7 +20,6 @@
 
 #define _COMPONENT		ACPI_BUS_COMPONENT
 ACPI_MODULE_NAME("scan");
-#define STRUCT_TO_INT(s)	(*((int*)&s))
 extern struct acpi_device *acpi_root;
 
 #define ACPI_BUS_CLASS			"system_bus"
@@ -1683,7 +1682,7 @@ void acpi_init_device_object(struct acpi
 	device->device_type = type;
 	device->handle = handle;
 	device->parent = acpi_bus_get_parent(handle);
-	STRUCT_TO_INT(device->status) = sta;
+	acpi_set_device_status(device, sta);
 	acpi_device_get_busid(device);
 	acpi_set_pnp_ids(handle, &device->pnp, type);
 	acpi_bus_get_flags(device);
@@ -1927,7 +1926,7 @@ static acpi_status acpi_bus_device_attac
 	if (acpi_bus_get_device(handle, &device))
 		return AE_CTRL_DEPTH;
 
-	STRUCT_TO_INT(device->status) = sta;
+	acpi_set_device_status(device, sta);
 	/* Skip devices that are not present. */
 	if (!acpi_device_is_present(device))
 		goto err;
Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -320,6 +320,11 @@ static inline void *acpi_driver_data(str
 #define to_acpi_device(d)	container_of(d, struct acpi_device, dev)
 #define to_acpi_driver(d)	container_of(d, struct acpi_driver, drv)
 
+static inline void acpi_set_device_status(struct acpi_device *adev, u32 sta)
+{
+	*((u32 *)&adev->status) = sta;
+}
+
 /* acpi_device.dev.bus == &acpi_bus_type */
 extern struct bus_type acpi_bus_type;
 

Re: [PATCH 0/10] ACPI: Device objects for all namespace nodes and PCI root hotplug integration

[Mika Westerberg]

On Sun, Nov 17, 2013 at 05:29:41PM +0100, Rafael J. Wysocki wrote:
> This is 3.14 material, but I'm posting it today to let you know it is coming.
> Of course, if you see any immediate problems with the following patches, please
> let me know.

For what it's worth, I have tested this series on three different systems
that have Thunderbolt and it still works fine after the series has been
applied :)

Re: [PATCH 0/10] ACPI: Device objects for all namespace nodes and PCI root hotplug integration

[Rafael J. Wysocki]

On Tuesday, November 19, 2013 04:30:46 PM Mika Westerberg wrote:
> On Sun, Nov 17, 2013 at 05:29:41PM +0100, Rafael J. Wysocki wrote:
> > This is 3.14 material, but I'm posting it today to let you know it is coming.
> > Of course, if you see any immediate problems with the following patches, please
> > let me know.
> 
> For what it's worth, I have tested this series on three different systems
> that have Thunderbolt and it still works fine after the series has been
> applied :)

Awesome, thanks!

I hope you won't mind if I add your Tested-by to the series. :-)

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

Re: [PATCH 0/10] ACPI: Device objects for all namespace nodes and PCI root hotplug integration

[Mika Westerberg]

On Tue, Nov 19, 2013 at 09:51:19PM +0100, Rafael J. Wysocki wrote:
> On Tuesday, November 19, 2013 04:30:46 PM Mika Westerberg wrote:
> > On Sun, Nov 17, 2013 at 05:29:41PM +0100, Rafael J. Wysocki wrote:
> > > This is 3.14 material, but I'm posting it today to let you know it is coming.
> > > Of course, if you see any immediate problems with the following patches, please
> > > let me know.
> > 
> > For what it's worth, I have tested this series on three different systems
> > that have Thunderbolt and it still works fine after the series has been
> > applied :)
> 
> Awesome, thanks!
> 
> I hope you won't mind if I add your Tested-by to the series. :-)

Not at all,

Tested-by: Mika Westerberg <mika.westerberg@linux.intel.com>