[RFC PATCH 0/3] ACPI: Flexible eject options to remove devices cautiously

[RFC PATCH 0/3] ACPI: Flexible eject options to remove devices cautiously

[Chester Lin]

* This proposal is derived from https://lkml.org/lkml/2020/1/2/1401

Currently there are two ways to handle ACPI device ejection. When an eject
event happens on an ACPI container, the kernel sends a KOBJ_CHANGE event to
userland and userland has to deal with offline operation. For other device
types, acpi_scan_try_to_offline() is called and it tries to put target
device(s) offline and then removes all nodes once they are all offline.

However we found that sometimes applications could intensively access
resources on ejectable devices therefore they could have risk if ejection
suddenly happens and removes devices without any notification. In stead
of executing the offline callbakcs directly, I want to introduce an
additional approach, which uses 'request_offline' attributes to remind
the kernel to notify userland that some devices have been planning to be
removed, so the applications running on those targets can be prepared and
determine when offline tasks should be done based on current workload.

This approach is disabled by default so it will not affect systems which
still use the original way. Once it's enabled, the kernel will only
send change events as offline requests of target nodes to userland, so
firmware or userspace callers who raise eject requests will still have to
re-trigger eject events if necessary because the kernel will not track all
targets' offline status actively. However, I still add an 'auto_eject'
attribute for userland if it's required to periodically schedule an eject
event until the target has been removed.

To ensure that eject function can work properly since normal users might
not have their own offline/online handling, I would propose a generic udev
rule to systemd upstream as default in order to deal with change events
and take offline/online actions accordingly:

*Example: 80-acpi-hotplug-eject.rules
------------------------------------------------
# Generic rules for handling ACPI hotplug eject.

SUBSYSTEM=="*", ACTION=="change", ENV{EVENT}=="offline", ATTR{online}=="1", \
DEVPATH=="*", ATTR{online}="0"

SUBSYSTEM=="*", ACTION=="change", ENV{EVENT}=="online", ATTR{online}=="0", \
DEVPATH=="*", ATTR{online}="1"
------------------------------------------------

Chester Lin (3):
  ACPI: scan: add userland notification while handling eject events
  ACPI: scan: add cancel_eject and auto_eject attributes
  ACPI: scan: add a request_offline_recursive attribute

 Documentation/ABI/testing/sysfs-bus-acpi |  43 +++++++
 drivers/acpi/device_sysfs.c              | 154 ++++++++++++++++++++++-
 drivers/acpi/internal.h                  |   4 +-
 drivers/acpi/osl.c                       |  37 ++++--
 drivers/acpi/scan.c                      | 108 +++++++++++++++-
 include/acpi/acpi_bus.h                  |   8 ++
 6 files changed, 339 insertions(+), 15 deletions(-)

-- 
2.24.0

[RFC PATCH 1/3] ACPI: scan: add userland notification while handling eject events

[Chester Lin]

Add a request_offline attribute in order to tell the kernel if it's
required to send notifications to userland first while handling an eject
event. Userland will have to put the target device offline when this
attribute is set.

Signed-off-by: Chester Lin <clin@suse.com>
---
 Documentation/ABI/testing/sysfs-bus-acpi | 16 ++++++++++
 drivers/acpi/device_sysfs.c              | 40 +++++++++++++++++++++++-
 drivers/acpi/scan.c                      | 39 +++++++++++++++++++----
 include/acpi/acpi_bus.h                  |  1 +
 4 files changed, 89 insertions(+), 7 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-acpi b/Documentation/ABI/testing/sysfs-bus-acpi
index e7898cfe5fb1..b9c467704889 100644
--- a/Documentation/ABI/testing/sysfs-bus-acpi
+++ b/Documentation/ABI/testing/sysfs-bus-acpi
@@ -93,3 +93,19 @@ Description:
 		hardware, if the _HRV control method is present.  It is mostly
 		useful for non-PCI devices because lspci can list the hardware
 		version for PCI devices.
+
+What:		/sys/bus/acpi/devices/.../request_offline
+Date:		Mar, 2020
+Contact:	Chester Lin <clin@suse.com>
+Description:
+		(RW) Allows the userland to receive offline requests when
+		devices are planning to be ejected.
+
+		If bit [0] is clear, the kernel will automatically try putting
+		the target offline before the target can be ejected.
+
+		If bit [0] is set, a uevent will be sent to userland as an
+		offline request and userland is responsible for handling offline
+		operations before the target can be ejected. This approach
+		provides flexibility while some applications could need more
+		time to release resources.
diff --git a/drivers/acpi/device_sysfs.c b/drivers/acpi/device_sysfs.c
index 96869f1538b9..453bd1b9edf5 100644
--- a/drivers/acpi/device_sysfs.c
+++ b/drivers/acpi/device_sysfs.c
@@ -506,6 +506,37 @@ static ssize_t status_show(struct device *dev, struct device_attribute *attr,
 }
 static DEVICE_ATTR_RO(status);
 
+static ssize_t request_offline_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct acpi_device *acpi_dev = to_acpi_device(dev);
+
+	return sprintf(buf, "%u\n", acpi_dev->request_offline?1:0);
+}
+
+static ssize_t request_offline_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct acpi_device *acpi_dev = to_acpi_device(dev);
+
+	if (!count)
+		return -EINVAL;
+
+	switch (buf[0]) {
+	case '0':
+		acpi_dev->request_offline = false;
+		break;
+	case '1':
+		acpi_dev->request_offline = true;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return count;
+}
+static DEVICE_ATTR_RW(request_offline);
+
 /**
  * acpi_device_setup_files - Create sysfs attributes of an ACPI device.
  * @dev: ACPI device object.
@@ -580,6 +611,11 @@ int acpi_device_setup_files(struct acpi_device *dev)
 		result = device_create_file(&dev->dev, &dev_attr_eject);
 		if (result)
 			return result;
+
+		result = device_create_file(&dev->dev,
+					    &dev_attr_request_offline);
+		if (result)
+			return result;
 	}
 
 	if (dev->flags.power_manageable) {
@@ -623,8 +659,10 @@ void acpi_device_remove_files(struct acpi_device *dev)
 	/*
 	 * If device has _EJ0, remove 'eject' file.
 	 */
-	if (acpi_has_method(dev->handle, "_EJ0"))
+	if (acpi_has_method(dev->handle, "_EJ0")) {
 		device_remove_file(&dev->dev, &dev_attr_eject);
+		device_remove_file(&dev->dev, &dev_attr_request_offline);
+	}
 
 	if (acpi_has_method(dev->handle, "_SUN"))
 		device_remove_file(&dev->dev, &dev_attr_sun);
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 6d3448895382..1cb39c5360cf 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -145,6 +145,7 @@ static acpi_status acpi_bus_offline(acpi_handle handle, u32 lvl, void *data,
 	struct acpi_device_physical_node *pn;
 	bool second_pass = (bool)data;
 	acpi_status status = AE_OK;
+	char *envp[] = { "EVENT=offline", NULL };
 
 	if (acpi_bus_get_device(handle, &device))
 		return AE_OK;
@@ -166,7 +167,18 @@ static acpi_status acpi_bus_offline(acpi_handle handle, u32 lvl, void *data,
 		} else {
 			pn->put_online = false;
 		}
-		ret = device_offline(pn->dev);
+
+		/* Don't offline directly but need to notify userland first */
+		if (device->request_offline) {
+			if (pn->dev->offline)
+				ret = 0;
+			else
+				ret = kobject_uevent_env(&pn->dev->kobj,
+							KOBJ_CHANGE, envp);
+		} else {
+			ret = device_offline(pn->dev);
+		}
+
 		if (ret >= 0) {
 			pn->put_online = !ret;
 		} else {
@@ -188,6 +200,7 @@ static acpi_status acpi_bus_online(acpi_handle handle, u32 lvl, void *data,
 {
 	struct acpi_device *device = NULL;
 	struct acpi_device_physical_node *pn;
+	char *envp[] = { "EVENT=online", NULL };
 
 	if (acpi_bus_get_device(handle, &device))
 		return AE_OK;
@@ -196,7 +209,12 @@ static acpi_status acpi_bus_online(acpi_handle handle, u32 lvl, void *data,
 
 	list_for_each_entry(pn, &device->physical_node_list, node)
 		if (pn->put_online) {
-			device_online(pn->dev);
+			if (device->request_offline)
+				kobject_uevent_env(&pn->dev->kobj,
+						   KOBJ_CHANGE, envp);
+			else
+				device_online(pn->dev);
+
 			pn->put_online = false;
 		}
 
@@ -256,14 +274,23 @@ static int acpi_scan_hot_remove(struct acpi_device *device)
 	acpi_handle handle = device->handle;
 	unsigned long long sta;
 	acpi_status status;
+	bool notify_single = false;
+	int error;
 
-	if (device->handler && device->handler->hotplug.demand_offline) {
-		if (!acpi_scan_is_offline(device, true))
+	if (device->handler && device->handler->hotplug.demand_offline)
+		if (!device->request_offline)
+			notify_single = true;
+
+	if (!acpi_scan_is_offline(device, notify_single)) {
+		if (notify_single)
 			return -EBUSY;
-	} else {
-		int error = acpi_scan_try_to_offline(device);
+
+		error = acpi_scan_try_to_offline(device);
 		if (error)
 			return error;
+
+		if (device->request_offline)
+			return -EBUSY;
 	}
 
 	ACPI_DEBUG_PRINT((ACPI_DB_INFO,
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 0c23fd0548d1..7a29ea0a7d0e 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -375,6 +375,7 @@ struct acpi_device {
 	struct list_head physical_node_list;
 	struct mutex physical_node_lock;
 	void (*remove)(struct acpi_device *);
+	bool request_offline;
 };
 
 /* Non-device subnode */
-- 
2.24.0

[RFC PATCH 2/3] ACPI: scan: add cancel_eject and auto_eject attributes

[Chester Lin]

Add two attributes 'cancel_eject' and 'auto_eject' as auxiliary features of
request_offline, which are only effective when the request_offline is set.
Writing 1 to cancel_eject will remove pending the eject event and then put
the target back to its original online state if it has been changed.
Writing a time interval to auto_eject will periodically schedule an eject
event and will trigger real hot-remove once the target is offline. You can
still keep auto_eject to 0 if the firmware or userpsace caller who raises
the eject request can re-trigger an eject event by itself.

Signed-off-by: Chester Lin <clin@suse.com>
---
 Documentation/ABI/testing/sysfs-bus-acpi | 20 ++++++
 drivers/acpi/device_sysfs.c              | 87 +++++++++++++++++++++++-
 drivers/acpi/internal.h                  |  2 +
 drivers/acpi/osl.c                       | 37 ++++++++--
 drivers/acpi/scan.c                      | 53 +++++++++++++--
 include/acpi/acpi_bus.h                  |  9 ++-
 6 files changed, 193 insertions(+), 15 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-acpi b/Documentation/ABI/testing/sysfs-bus-acpi
index b9c467704889..be00749f00e6 100644
--- a/Documentation/ABI/testing/sysfs-bus-acpi
+++ b/Documentation/ABI/testing/sysfs-bus-acpi
@@ -109,3 +109,23 @@ Description:
 		operations before the target can be ejected. This approach
 		provides flexibility while some applications could need more
 		time to release resources.
+
+What:		/sys/bus/acpi/devices/.../cancel_eject
+Date:		Mar, 2020
+Contact:	Chester Lin <clin@suse.com>
+Description:
+		(WO) Writing 1 to this attribute will cancel the pending
+		ejection when userland is working on a target's offline
+		procedure [e.g. req_offline is set]. Then it will try putting
+		the target device back to its original online state.
+
+What:		/sys/bus/acpi/devices/.../auto_eject
+Date:		Mar, 2020
+Contact:	Chester Lin <clin@suse.com>
+Description:
+		(RW) Allows the userland to periodically schedule an eject
+		event on a target until it can be successfully removed.
+		Userland can write a time interval [unit: second] to this
+		attribute, and write 0 to disable it. This feature is disabled
+		when the request_offline is 0 or no initial eject event
+		is triggered by firmware or an eject attribute in /sys.
diff --git a/drivers/acpi/device_sysfs.c b/drivers/acpi/device_sysfs.c
index 453bd1b9edf5..e40daafa3f85 100644
--- a/drivers/acpi/device_sysfs.c
+++ b/drivers/acpi/device_sysfs.c
@@ -511,7 +511,7 @@ static ssize_t request_offline_show(struct device *dev,
 {
 	struct acpi_device *acpi_dev = to_acpi_device(dev);
 
-	return sprintf(buf, "%u\n", acpi_dev->request_offline?1:0);
+	return sprintf(buf, "%u\n", acpi_dev->eject.request_offline?1:0);
 }
 
 static ssize_t request_offline_store(struct device *dev,
@@ -524,10 +524,10 @@ static ssize_t request_offline_store(struct device *dev,
 
 	switch (buf[0]) {
 	case '0':
-		acpi_dev->request_offline = false;
+		acpi_dev->eject.request_offline = false;
 		break;
 	case '1':
-		acpi_dev->request_offline = true;
+		acpi_dev->eject.request_offline = true;
 		break;
 	default:
 		return -EINVAL;
@@ -537,6 +537,74 @@ static ssize_t request_offline_store(struct device *dev,
 }
 static DEVICE_ATTR_RW(request_offline);
 
+static ssize_t auto_eject_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct acpi_device *acpi_dev = to_acpi_device(dev);
+
+	return sprintf(buf, "%u\n", acpi_dev->eject.poll_time);
+}
+
+static ssize_t auto_eject_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct acpi_device *acpi_dev = to_acpi_device(dev);
+	unsigned int time_interval;
+
+	if (!count)
+		return -EINVAL;
+
+	if (sscanf(buf, "%u\n", &time_interval) == 1)
+		acpi_dev->eject.poll_time = time_interval;
+
+	return count;
+}
+static DEVICE_ATTR_RW(auto_eject);
+
+static ssize_t
+cancel_eject_store(struct device *d, struct device_attribute *attr,
+		const char *buf, size_t count)
+{
+	struct acpi_device *acpi_device = to_acpi_device(d);
+	acpi_object_type not_used;
+	acpi_status status;
+
+	if (!count || buf[0] != '1')
+		return -EINVAL;
+
+	if ((!acpi_device->handler || !acpi_device->handler->hotplug.enabled)
+	    && !acpi_device->driver)
+		return -ENODEV;
+
+	status = acpi_get_type(acpi_device->handle, &not_used);
+	if (ACPI_FAILURE(status) || !acpi_device->flags.ejectable)
+		return -ENODEV;
+
+	if (!acpi_device->eject.in_progress)
+		return count;
+
+	acpi_device->eject.cancel = true;
+
+	if (!acpi_device->eject.poll_time) {
+		get_device(&acpi_device->dev);
+
+		status = acpi_hotplug_schedule(acpi_device,
+					       ACPI_OST_EC_OSPM_EJECT);
+		if (ACPI_SUCCESS(status))
+			return count;
+
+		put_device(&acpi_device->dev);
+
+		acpi_evaluate_ost(acpi_device->handle, ACPI_OST_EC_OSPM_EJECT,
+				  ACPI_OST_SC_NON_SPECIFIC_FAILURE, NULL);
+
+		return status == AE_NO_MEMORY ? -ENOMEM : -EAGAIN;
+	}
+
+	return count;
+}
+static DEVICE_ATTR_WO(cancel_eject);
+
 /**
  * acpi_device_setup_files - Create sysfs attributes of an ACPI device.
  * @dev: ACPI device object.
@@ -616,6 +684,17 @@ int acpi_device_setup_files(struct acpi_device *dev)
 					    &dev_attr_request_offline);
 		if (result)
 			return result;
+
+		result = device_create_file(&dev->dev,
+					    &dev_attr_auto_eject);
+		if (result)
+			return result;
+
+		result = device_create_file(&dev->dev,
+					    &dev_attr_cancel_eject);
+		if (result)
+			return result;
+
 	}
 
 	if (dev->flags.power_manageable) {
@@ -662,6 +741,8 @@ void acpi_device_remove_files(struct acpi_device *dev)
 	if (acpi_has_method(dev->handle, "_EJ0")) {
 		device_remove_file(&dev->dev, &dev_attr_eject);
 		device_remove_file(&dev->dev, &dev_attr_request_offline);
+		device_remove_file(&dev->dev, &dev_attr_auto_eject);
+		device_remove_file(&dev->dev, &dev_attr_cancel_eject);
 	}
 
 	if (acpi_has_method(dev->handle, "_SUN"))
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index e387517d3354..45f4ce42a044 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -81,6 +81,8 @@ static inline void acpi_lpss_init(void) {}
 void acpi_apd_init(void);
 
 acpi_status acpi_hotplug_schedule(struct acpi_device *adev, u32 src);
+acpi_status acpi_hotplug_delayed_schedule(struct acpi_device *adev,
+					  u32 src, unsigned long delay);
 bool acpi_queue_hotplug_work(struct work_struct *work);
 void acpi_device_hotplug(struct acpi_device *adev, u32 src);
 bool acpi_scan_is_offline(struct acpi_device *adev, bool uevent);
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 762c5d50b8fe..0716c5bbff12 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -1143,33 +1143,44 @@ void acpi_os_wait_events_complete(void)
 EXPORT_SYMBOL(acpi_os_wait_events_complete);
 
 struct acpi_hp_work {
-	struct work_struct work;
+	struct delayed_work work;
 	struct acpi_device *adev;
 	u32 src;
 };
 
 static void acpi_hotplug_work_fn(struct work_struct *work)
 {
-	struct acpi_hp_work *hpw = container_of(work, struct acpi_hp_work, work);
+	struct delayed_work *delay_work;
+	struct acpi_hp_work *hpw;
+
+	delay_work = container_of(work, struct delayed_work, work);
+	hpw = container_of(delay_work, struct acpi_hp_work, work);
+
+	if (!hpw) {
+		pr_debug("Null object of ACPI hotplug work.\n");
+		return;
+	}
 
 	acpi_os_wait_events_complete();
 	acpi_device_hotplug(hpw->adev, hpw->src);
 	kfree(hpw);
 }
 
-acpi_status acpi_hotplug_schedule(struct acpi_device *adev, u32 src)
+static acpi_status acpi_hotplug_schedule_work(struct acpi_device *adev,
+	u32 src, unsigned long delay)
 {
 	struct acpi_hp_work *hpw;
 
 	ACPI_DEBUG_PRINT((ACPI_DB_EXEC,
-		  "Scheduling hotplug event (%p, %u) for deferred execution.\n",
-		  adev, src));
+	  "Scheduling hotplug event (%p, %u, %lu) for deferred execution.\n",
+	  adev, src, delay));
 
 	hpw = kmalloc(sizeof(*hpw), GFP_KERNEL);
 	if (!hpw)
 		return AE_NO_MEMORY;
 
-	INIT_WORK(&hpw->work, acpi_hotplug_work_fn);
+	INIT_DELAYED_WORK(&hpw->work, acpi_hotplug_work_fn);
+
 	hpw->adev = adev;
 	hpw->src = src;
 	/*
@@ -1178,13 +1189,25 @@ acpi_status acpi_hotplug_schedule(struct acpi_device *adev, u32 src)
 	 * invoke flush_scheduled_work()/acpi_os_wait_events_complete() to flush
 	 * these workqueues.
 	 */
-	if (!queue_work(kacpi_hotplug_wq, &hpw->work)) {
+	if (!queue_delayed_work(kacpi_hotplug_wq, &hpw->work, delay)) {
 		kfree(hpw);
 		return AE_ERROR;
 	}
+
 	return AE_OK;
 }
 
+acpi_status acpi_hotplug_schedule(struct acpi_device *adev, u32 src)
+{
+	return acpi_hotplug_schedule_work(adev, src, 0);
+}
+
+acpi_status acpi_hotplug_delayed_schedule(struct acpi_device *adev,
+					  u32 src, unsigned long delay)
+{
+	return acpi_hotplug_schedule_work(adev, src, delay);
+}
+
 bool acpi_queue_hotplug_work(struct work_struct *work)
 {
 	return queue_work(kacpi_hotplug_wq, work);
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 1cb39c5360cf..b4678ed14eed 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -169,7 +169,7 @@ static acpi_status acpi_bus_offline(acpi_handle handle, u32 lvl, void *data,
 		}
 
 		/* Don't offline directly but need to notify userland first */
-		if (device->request_offline) {
+		if (device->eject.request_offline) {
 			if (pn->dev->offline)
 				ret = 0;
 			else
@@ -209,7 +209,7 @@ static acpi_status acpi_bus_online(acpi_handle handle, u32 lvl, void *data,
 
 	list_for_each_entry(pn, &device->physical_node_list, node)
 		if (pn->put_online) {
-			if (device->request_offline)
+			if (device->eject.request_offline)
 				kobject_uevent_env(&pn->dev->kobj,
 						   KOBJ_CHANGE, envp);
 			else
@@ -269,6 +269,41 @@ static int acpi_scan_try_to_offline(struct acpi_device *device)
 	return 0;
 }
 
+static void acpi_scan_cancel_eject(struct acpi_device *device)
+{
+	acpi_handle handle = device->handle;
+
+	/* Get all nodes online again if necessary */
+	acpi_bus_online(handle, 0, NULL, NULL);
+	acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX,
+			    acpi_bus_online, NULL, NULL, NULL);
+
+	device->eject.in_progress = false;
+	device->eject.cancel = false;
+}
+
+static inline void acpi_set_eject_status(struct acpi_device *device)
+{
+	unsigned long delay;
+	acpi_status status;
+
+	device->eject.in_progress = true;
+
+	if (!device->eject.poll_time)
+		return;
+
+	delay = device->eject.poll_time * HZ;
+
+	get_device(&device->dev);
+	status = acpi_hotplug_delayed_schedule(device,
+				ACPI_OST_EC_OSPM_EJECT, delay);
+
+	if (ACPI_FAILURE(status)) {
+		pr_err("Failed to schedule a delayed work\n");
+		put_device(&device->dev);
+	}
+}
+
 static int acpi_scan_hot_remove(struct acpi_device *device)
 {
 	acpi_handle handle = device->handle;
@@ -277,8 +312,13 @@ static int acpi_scan_hot_remove(struct acpi_device *device)
 	bool notify_single = false;
 	int error;
 
+	if (device->eject.request_offline && device->eject.cancel) {
+		acpi_scan_cancel_eject(device);
+		return -EBUSY;
+	}
+
 	if (device->handler && device->handler->hotplug.demand_offline)
-		if (!device->request_offline)
+		if (!device->eject.request_offline)
 			notify_single = true;
 
 	if (!acpi_scan_is_offline(device, notify_single)) {
@@ -289,10 +329,15 @@ static int acpi_scan_hot_remove(struct acpi_device *device)
 		if (error)
 			return error;
 
-		if (device->request_offline)
+		if (device->eject.request_offline) {
+			acpi_set_eject_status(device);
 			return -EBUSY;
+		}
 	}
 
+	device->eject.in_progress = false;
+	device->eject.cancel = false;
+
 	ACPI_DEBUG_PRINT((ACPI_DB_INFO,
 		"Hot-removing device %s...\n", dev_name(&device->dev)));
 
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 7a29ea0a7d0e..1fb72e399e0d 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -346,6 +346,13 @@ struct acpi_device_data {
 
 struct acpi_gpio_mapping;
 
+struct acpi_eject_status {
+	bool request_offline;
+	bool in_progress;
+	bool cancel;
+	unsigned int poll_time; /* unit: second */
+};
+
 /* Device */
 struct acpi_device {
 	int device_type;
@@ -375,7 +382,7 @@ struct acpi_device {
 	struct list_head physical_node_list;
 	struct mutex physical_node_lock;
 	void (*remove)(struct acpi_device *);
-	bool request_offline;
+	struct acpi_eject_status eject;
 };
 
 /* Non-device subnode */
-- 
2.24.0

[RFC PATCH 3/3] ACPI: scan: add a request_offline_recursive attribute

[Chester Lin]

Add a request_offline_recursive attribute for userland to change
request_offline settings of the target and its child nodes at once.

Signed-off-by: Chester Lin <clin@suse.com>
---
 Documentation/ABI/testing/sysfs-bus-acpi |  7 +++++
 drivers/acpi/device_sysfs.c              | 33 ++++++++++++++++++++++++
 drivers/acpi/internal.h                  |  2 +-
 drivers/acpi/scan.c                      | 24 +++++++++++++++++
 4 files changed, 65 insertions(+), 1 deletion(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-acpi b/Documentation/ABI/testing/sysfs-bus-acpi
index be00749f00e6..eb2fd9752b66 100644
--- a/Documentation/ABI/testing/sysfs-bus-acpi
+++ b/Documentation/ABI/testing/sysfs-bus-acpi
@@ -110,6 +110,13 @@ Description:
 		provides flexibility while some applications could need more
 		time to release resources.
 
+What:		/sys/bus/acpi/devices/.../request_offline_recursive
+Date:		Mar, 2020
+Contact:	Chester Lin <clin@suse.com>
+Description:
+		(RW) Same as request_offline but the writing will also apply
+		to all child ndoes under the target.
+
 What:		/sys/bus/acpi/devices/.../cancel_eject
 Date:		Mar, 2020
 Contact:	Chester Lin <clin@suse.com>
diff --git a/drivers/acpi/device_sysfs.c b/drivers/acpi/device_sysfs.c
index e40daafa3f85..d4fdb6846c78 100644
--- a/drivers/acpi/device_sysfs.c
+++ b/drivers/acpi/device_sysfs.c
@@ -537,6 +537,32 @@ static ssize_t request_offline_store(struct device *dev,
 }
 static DEVICE_ATTR_RW(request_offline);
 
+static ssize_t request_offline_recursive_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct acpi_device *acpi_dev = to_acpi_device(dev);
+	bool value;
+
+	if (!count)
+		return -EINVAL;
+
+	switch (buf[0]) {
+	case '0':
+		value = false;
+		break;
+	case '1':
+		value = true;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	acpi_request_offline_recursive(acpi_dev, value);
+
+	return count;
+}
+static DEVICE_ATTR_WO(request_offline_recursive);
+
 static ssize_t auto_eject_show(struct device *dev,
 		struct device_attribute *attr, char *buf)
 {
@@ -685,6 +711,11 @@ int acpi_device_setup_files(struct acpi_device *dev)
 		if (result)
 			return result;
 
+		result = device_create_file(&dev->dev,
+					&dev_attr_request_offline_recursive);
+		if (result)
+			return result;
+
 		result = device_create_file(&dev->dev,
 					    &dev_attr_auto_eject);
 		if (result)
@@ -741,6 +772,8 @@ void acpi_device_remove_files(struct acpi_device *dev)
 	if (acpi_has_method(dev->handle, "_EJ0")) {
 		device_remove_file(&dev->dev, &dev_attr_eject);
 		device_remove_file(&dev->dev, &dev_attr_request_offline);
+		device_remove_file(&dev->dev,
+				   &dev_attr_request_offline_recursive);
 		device_remove_file(&dev->dev, &dev_attr_auto_eject);
 		device_remove_file(&dev->dev, &dev_attr_cancel_eject);
 	}
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index 45f4ce42a044..3a2f66eac639 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -89,7 +89,7 @@ bool acpi_scan_is_offline(struct acpi_device *adev, bool uevent);
 
 acpi_status acpi_sysfs_table_handler(u32 event, void *table, void *context);
 void acpi_scan_table_handler(u32 event, void *table, void *context);
-
+void acpi_request_offline_recursive(struct acpi_device *device, bool value);
 /* --------------------------------------------------------------------------
                      Device Node Initialization / Removal
    -------------------------------------------------------------------------- */
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index b4678ed14eed..db6f0551ca94 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -269,6 +269,30 @@ static int acpi_scan_try_to_offline(struct acpi_device *device)
 	return 0;
 }
 
+static acpi_status acpi_bus_request_offline(acpi_handle handle, u32 lvl,
+					    void *data, void **ret_p)
+{
+	struct acpi_device *device = NULL;
+
+	if (!acpi_bus_get_device(handle, &device))
+		device->eject.request_offline = (bool)data;
+
+	return AE_OK;
+}
+
+void acpi_request_offline_recursive(struct acpi_device *device, bool value)
+{
+	acpi_handle handle = device->handle;
+	acpi_status status;
+
+	mutex_lock(&acpi_scan_lock);
+	device->eject.request_offline = value;
+	status = acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX,
+				     acpi_bus_request_offline, NULL,
+				     (void *)value, NULL);
+	mutex_unlock(&acpi_scan_lock);
+}
+
 static void acpi_scan_cancel_eject(struct acpi_device *device)
 {
 	acpi_handle handle = device->handle;
-- 
2.24.0

Re: [RFC PATCH 1/3] ACPI: scan: add userland notification while handling eject events

[Greg KH]

On Fri, Mar 27, 2020 at 07:22:45PM +0800, Chester Lin wrote:
> Add a request_offline attribute in order to tell the kernel if it's
> required to send notifications to userland first while handling an eject
> event. Userland will have to put the target device offline when this
> attribute is set.
> 
> Signed-off-by: Chester Lin <clin@suse.com>
> ---
>  Documentation/ABI/testing/sysfs-bus-acpi | 16 ++++++++++
>  drivers/acpi/device_sysfs.c              | 40 +++++++++++++++++++++++-
>  drivers/acpi/scan.c                      | 39 +++++++++++++++++++----
>  include/acpi/acpi_bus.h                  |  1 +
>  4 files changed, 89 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-acpi b/Documentation/ABI/testing/sysfs-bus-acpi
> index e7898cfe5fb1..b9c467704889 100644
> --- a/Documentation/ABI/testing/sysfs-bus-acpi
> +++ b/Documentation/ABI/testing/sysfs-bus-acpi
> @@ -93,3 +93,19 @@ Description:
>  		hardware, if the _HRV control method is present.  It is mostly
>  		useful for non-PCI devices because lspci can list the hardware
>  		version for PCI devices.
> +
> +What:		/sys/bus/acpi/devices/.../request_offline
> +Date:		Mar, 2020
> +Contact:	Chester Lin <clin@suse.com>
> +Description:
> +		(RW) Allows the userland to receive offline requests when
> +		devices are planning to be ejected.
> +
> +		If bit [0] is clear, the kernel will automatically try putting
> +		the target offline before the target can be ejected.
> +
> +		If bit [0] is set, a uevent will be sent to userland as an
> +		offline request and userland is responsible for handling offline
> +		operations before the target can be ejected. This approach
> +		provides flexibility while some applications could need more
> +		time to release resources.

Don't use "bit", use 1/0/y/n/Y/N as the kernel will parse all of that
for you with the kstrtobool() which was created just for this type of
sysfs file.

> diff --git a/drivers/acpi/device_sysfs.c b/drivers/acpi/device_sysfs.c
> index 96869f1538b9..453bd1b9edf5 100644
> --- a/drivers/acpi/device_sysfs.c
> +++ b/drivers/acpi/device_sysfs.c
> @@ -506,6 +506,37 @@ static ssize_t status_show(struct device *dev, struct device_attribute *attr,
>  }
>  static DEVICE_ATTR_RO(status);
>  
> +static ssize_t request_offline_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct acpi_device *acpi_dev = to_acpi_device(dev);
> +
> +	return sprintf(buf, "%u\n", acpi_dev->request_offline?1:0);
> +}
> +
> +static ssize_t request_offline_store(struct device *dev,
> +		struct device_attribute *attr, const char *buf, size_t count)
> +{
> +	struct acpi_device *acpi_dev = to_acpi_device(dev);
> +
> +	if (!count)
> +		return -EINVAL;
> +
> +	switch (buf[0]) {
> +	case '0':
> +		acpi_dev->request_offline = false;
> +		break;
> +	case '1':
> +		acpi_dev->request_offline = true;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return count;
> +}
> +static DEVICE_ATTR_RW(request_offline);
> +
>  /**
>   * acpi_device_setup_files - Create sysfs attributes of an ACPI device.
>   * @dev: ACPI device object.
> @@ -580,6 +611,11 @@ int acpi_device_setup_files(struct acpi_device *dev)
>  		result = device_create_file(&dev->dev, &dev_attr_eject);
>  		if (result)
>  			return result;
> +
> +		result = device_create_file(&dev->dev,
> +					    &dev_attr_request_offline);
> +		if (result)
> +			return result;
>  	}
>  
>  	if (dev->flags.power_manageable) {
> @@ -623,8 +659,10 @@ void acpi_device_remove_files(struct acpi_device *dev)
>  	/*
>  	 * If device has _EJ0, remove 'eject' file.
>  	 */
> -	if (acpi_has_method(dev->handle, "_EJ0"))
> +	if (acpi_has_method(dev->handle, "_EJ0")) {
>  		device_remove_file(&dev->dev, &dev_attr_eject);
> +		device_remove_file(&dev->dev, &dev_attr_request_offline);

You all really should be using an attribute group and the is_visible()
callback to handle all of this for you automatically.

But that's a separate issue than this specific patch.

> +	}
>  
>  	if (acpi_has_method(dev->handle, "_SUN"))
>  		device_remove_file(&dev->dev, &dev_attr_sun);
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 6d3448895382..1cb39c5360cf 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -145,6 +145,7 @@ static acpi_status acpi_bus_offline(acpi_handle handle, u32 lvl, void *data,
>  	struct acpi_device_physical_node *pn;
>  	bool second_pass = (bool)data;
>  	acpi_status status = AE_OK;
> +	char *envp[] = { "EVENT=offline", NULL };
>  
>  	if (acpi_bus_get_device(handle, &device))
>  		return AE_OK;
> @@ -166,7 +167,18 @@ static acpi_status acpi_bus_offline(acpi_handle handle, u32 lvl, void *data,
>  		} else {
>  			pn->put_online = false;
>  		}
> -		ret = device_offline(pn->dev);
> +
> +		/* Don't offline directly but need to notify userland first */
> +		if (device->request_offline) {
> +			if (pn->dev->offline)
> +				ret = 0;
> +			else
> +				ret = kobject_uevent_env(&pn->dev->kobj,
> +							KOBJ_CHANGE, envp);

So this is a userspace visable change with regards to kobject events?

Are you sure that is ok?

Re: [RFC PATCH 1/3] ACPI: scan: add userland notification while handling eject events

[Chester Lin]

Hi Greg,

On Fri, Mar 27, 2020 at 12:38:42PM +0100, Greg KH wrote:
> On Fri, Mar 27, 2020 at 07:22:45PM +0800, Chester Lin wrote:
> > Add a request_offline attribute in order to tell the kernel if it's
> > required to send notifications to userland first while handling an eject
> > event. Userland will have to put the target device offline when this
> > attribute is set.
> > 
> > Signed-off-by: Chester Lin <clin@suse.com>
> > ---
> >  Documentation/ABI/testing/sysfs-bus-acpi | 16 ++++++++++
> >  drivers/acpi/device_sysfs.c              | 40 +++++++++++++++++++++++-
> >  drivers/acpi/scan.c                      | 39 +++++++++++++++++++----
> >  include/acpi/acpi_bus.h                  |  1 +
> >  4 files changed, 89 insertions(+), 7 deletions(-)
> > 
> > diff --git a/Documentation/ABI/testing/sysfs-bus-acpi b/Documentation/ABI/testing/sysfs-bus-acpi
> > index e7898cfe5fb1..b9c467704889 100644
> > --- a/Documentation/ABI/testing/sysfs-bus-acpi
> > +++ b/Documentation/ABI/testing/sysfs-bus-acpi
> > @@ -93,3 +93,19 @@ Description:
> >  		hardware, if the _HRV control method is present.  It is mostly
> >  		useful for non-PCI devices because lspci can list the hardware
> >  		version for PCI devices.
> > +
> > +What:		/sys/bus/acpi/devices/.../request_offline
> > +Date:		Mar, 2020
> > +Contact:	Chester Lin <clin@suse.com>
> > +Description:
> > +		(RW) Allows the userland to receive offline requests when
> > +		devices are planning to be ejected.
> > +
> > +		If bit [0] is clear, the kernel will automatically try putting
> > +		the target offline before the target can be ejected.
> > +
> > +		If bit [0] is set, a uevent will be sent to userland as an
> > +		offline request and userland is responsible for handling offline
> > +		operations before the target can be ejected. This approach
> > +		provides flexibility while some applications could need more
> > +		time to release resources.
> 
> Don't use "bit", use 1/0/y/n/Y/N as the kernel will parse all of that
> for you with the kstrtobool() which was created just for this type of
> sysfs file.
> 

I'm sorry for this mistake. Based on my code they should be ASCII char '1' and
'0' but not bitwise ops. I will fix this description.

> > diff --git a/drivers/acpi/device_sysfs.c b/drivers/acpi/device_sysfs.c
> > index 96869f1538b9..453bd1b9edf5 100644
> > --- a/drivers/acpi/device_sysfs.c
> > +++ b/drivers/acpi/device_sysfs.c
> > @@ -506,6 +506,37 @@ static ssize_t status_show(struct device *dev, struct device_attribute *attr,
> >  }
> >  static DEVICE_ATTR_RO(status);
> >  
> > +static ssize_t request_offline_show(struct device *dev,
> > +		struct device_attribute *attr, char *buf)
> > +{
> > +	struct acpi_device *acpi_dev = to_acpi_device(dev);
> > +
> > +	return sprintf(buf, "%u\n", acpi_dev->request_offline?1:0);
> > +}
> > +
> > +static ssize_t request_offline_store(struct device *dev,
> > +		struct device_attribute *attr, const char *buf, size_t count)
> > +{
> > +	struct acpi_device *acpi_dev = to_acpi_device(dev);
> > +
> > +	if (!count)
> > +		return -EINVAL;
> > +
> > +	switch (buf[0]) {
> > +	case '0':
> > +		acpi_dev->request_offline = false;
> > +		break;
> > +	case '1':
> > +		acpi_dev->request_offline = true;
> > +		break;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +
> > +	return count;
> > +}
> > +static DEVICE_ATTR_RW(request_offline);
> > +
> >  /**
> >   * acpi_device_setup_files - Create sysfs attributes of an ACPI device.
> >   * @dev: ACPI device object.
> > @@ -580,6 +611,11 @@ int acpi_device_setup_files(struct acpi_device *dev)
> >  		result = device_create_file(&dev->dev, &dev_attr_eject);
> >  		if (result)
> >  			return result;
> > +
> > +		result = device_create_file(&dev->dev,
> > +					    &dev_attr_request_offline);
> > +		if (result)
> > +			return result;
> >  	}
> >  
> >  	if (dev->flags.power_manageable) {
> > @@ -623,8 +659,10 @@ void acpi_device_remove_files(struct acpi_device *dev)
> >  	/*
> >  	 * If device has _EJ0, remove 'eject' file.
> >  	 */
> > -	if (acpi_has_method(dev->handle, "_EJ0"))
> > +	if (acpi_has_method(dev->handle, "_EJ0")) {
> >  		device_remove_file(&dev->dev, &dev_attr_eject);
> > +		device_remove_file(&dev->dev, &dev_attr_request_offline);
> 
> You all really should be using an attribute group and the is_visible()
> callback to handle all of this for you automatically.
> 
> But that's a separate issue than this specific patch.
> 

That sounds good to me. I will refine this part by declaring an attribute_group
with a is_visible() callback.

> > +	}
> >  
> >  	if (acpi_has_method(dev->handle, "_SUN"))
> >  		device_remove_file(&dev->dev, &dev_attr_sun);
> > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> > index 6d3448895382..1cb39c5360cf 100644
> > --- a/drivers/acpi/scan.c
> > +++ b/drivers/acpi/scan.c
> > @@ -145,6 +145,7 @@ static acpi_status acpi_bus_offline(acpi_handle handle, u32 lvl, void *data,
> >  	struct acpi_device_physical_node *pn;
> >  	bool second_pass = (bool)data;
> >  	acpi_status status = AE_OK;
> > +	char *envp[] = { "EVENT=offline", NULL };
> >  
> >  	if (acpi_bus_get_device(handle, &device))
> >  		return AE_OK;
> > @@ -166,7 +167,18 @@ static acpi_status acpi_bus_offline(acpi_handle handle, u32 lvl, void *data,
> >  		} else {
> >  			pn->put_online = false;
> >  		}
> > -		ret = device_offline(pn->dev);
> > +
> > +		/* Don't offline directly but need to notify userland first */
> > +		if (device->request_offline) {
> > +			if (pn->dev->offline)
> > +				ret = 0;
> > +			else
> > +				ret = kobject_uevent_env(&pn->dev->kobj,
> > +							KOBJ_CHANGE, envp);
> 
> So this is a userspace visable change with regards to kobject events?
> 
> Are you sure that is ok?
> 

Since udev can see kobject events when devices are added, I haven't seen any
risk if we make offline events visible too. Besides, normally online/eject
attributes can only be written by root in userspace.

Thanks,
Chester Lin

Re: [RFC PATCH 1/3] ACPI: scan: add userland notification while handling eject events

[Chester Lin]

On Mon, Mar 30, 2020 at 04:50:20PM +0800, Chester Lin wrote:
> Hi Greg,
> 
> On Fri, Mar 27, 2020 at 12:38:42PM +0100, Greg KH wrote:
> > On Fri, Mar 27, 2020 at 07:22:45PM +0800, Chester Lin wrote:
> > > Add a request_offline attribute in order to tell the kernel if it's
> > > required to send notifications to userland first while handling an eject
> > > event. Userland will have to put the target device offline when this
> > > attribute is set.
> > > 
> > > Signed-off-by: Chester Lin <clin@suse.com>
> > > ---
> > >  Documentation/ABI/testing/sysfs-bus-acpi | 16 ++++++++++
> > >  drivers/acpi/device_sysfs.c              | 40 +++++++++++++++++++++++-
> > >  drivers/acpi/scan.c                      | 39 +++++++++++++++++++----
> > >  include/acpi/acpi_bus.h                  |  1 +
> > >  4 files changed, 89 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/Documentation/ABI/testing/sysfs-bus-acpi b/Documentation/ABI/testing/sysfs-bus-acpi
> > > index e7898cfe5fb1..b9c467704889 100644
> > > --- a/Documentation/ABI/testing/sysfs-bus-acpi
> > > +++ b/Documentation/ABI/testing/sysfs-bus-acpi
> > > @@ -93,3 +93,19 @@ Description:
> > >  		hardware, if the _HRV control method is present.  It is mostly
> > >  		useful for non-PCI devices because lspci can list the hardware
> > >  		version for PCI devices.
> > > +
> > > +What:		/sys/bus/acpi/devices/.../request_offline
> > > +Date:		Mar, 2020
> > > +Contact:	Chester Lin <clin@suse.com>
> > > +Description:
> > > +		(RW) Allows the userland to receive offline requests when
> > > +		devices are planning to be ejected.
> > > +
> > > +		If bit [0] is clear, the kernel will automatically try putting
> > > +		the target offline before the target can be ejected.
> > > +
> > > +		If bit [0] is set, a uevent will be sent to userland as an
> > > +		offline request and userland is responsible for handling offline
> > > +		operations before the target can be ejected. This approach
> > > +		provides flexibility while some applications could need more
> > > +		time to release resources.
> > 
> > Don't use "bit", use 1/0/y/n/Y/N as the kernel will parse all of that
> > for you with the kstrtobool() which was created just for this type of
> > sysfs file.
> > 
> 
> I'm sorry for this mistake. Based on my code they should be ASCII char '1' and
> '0' but not bitwise ops. I will fix this description.
> 
> > > diff --git a/drivers/acpi/device_sysfs.c b/drivers/acpi/device_sysfs.c
> > > index 96869f1538b9..453bd1b9edf5 100644
> > > --- a/drivers/acpi/device_sysfs.c
> > > +++ b/drivers/acpi/device_sysfs.c
> > > @@ -506,6 +506,37 @@ static ssize_t status_show(struct device *dev, struct device_attribute *attr,
> > >  }
> > >  static DEVICE_ATTR_RO(status);
> > >  
> > > +static ssize_t request_offline_show(struct device *dev,
> > > +		struct device_attribute *attr, char *buf)
> > > +{
> > > +	struct acpi_device *acpi_dev = to_acpi_device(dev);
> > > +
> > > +	return sprintf(buf, "%u\n", acpi_dev->request_offline?1:0);
> > > +}
> > > +
> > > +static ssize_t request_offline_store(struct device *dev,
> > > +		struct device_attribute *attr, const char *buf, size_t count)
> > > +{
> > > +	struct acpi_device *acpi_dev = to_acpi_device(dev);
> > > +
> > > +	if (!count)
> > > +		return -EINVAL;
> > > +
> > > +	switch (buf[0]) {
> > > +	case '0':
> > > +		acpi_dev->request_offline = false;
> > > +		break;
> > > +	case '1':
> > > +		acpi_dev->request_offline = true;
> > > +		break;
> > > +	default:
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	return count;
> > > +}
> > > +static DEVICE_ATTR_RW(request_offline);
> > > +
> > >  /**
> > >   * acpi_device_setup_files - Create sysfs attributes of an ACPI device.
> > >   * @dev: ACPI device object.
> > > @@ -580,6 +611,11 @@ int acpi_device_setup_files(struct acpi_device *dev)
> > >  		result = device_create_file(&dev->dev, &dev_attr_eject);
> > >  		if (result)
> > >  			return result;
> > > +
> > > +		result = device_create_file(&dev->dev,
> > > +					    &dev_attr_request_offline);
> > > +		if (result)
> > > +			return result;
> > >  	}
> > >  
> > >  	if (dev->flags.power_manageable) {
> > > @@ -623,8 +659,10 @@ void acpi_device_remove_files(struct acpi_device *dev)
> > >  	/*
> > >  	 * If device has _EJ0, remove 'eject' file.
> > >  	 */
> > > -	if (acpi_has_method(dev->handle, "_EJ0"))
> > > +	if (acpi_has_method(dev->handle, "_EJ0")) {
> > >  		device_remove_file(&dev->dev, &dev_attr_eject);
> > > +		device_remove_file(&dev->dev, &dev_attr_request_offline);
> > 
> > You all really should be using an attribute group and the is_visible()
> > callback to handle all of this for you automatically.
> > 
> > But that's a separate issue than this specific patch.
> > 
> 
> That sounds good to me. I will refine this part by declaring an attribute_group
> with a is_visible() callback.
> 
> > > +	}
> > >  
> > >  	if (acpi_has_method(dev->handle, "_SUN"))
> > >  		device_remove_file(&dev->dev, &dev_attr_sun);
> > > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> > > index 6d3448895382..1cb39c5360cf 100644
> > > --- a/drivers/acpi/scan.c
> > > +++ b/drivers/acpi/scan.c
> > > @@ -145,6 +145,7 @@ static acpi_status acpi_bus_offline(acpi_handle handle, u32 lvl, void *data,
> > >  	struct acpi_device_physical_node *pn;
> > >  	bool second_pass = (bool)data;
> > >  	acpi_status status = AE_OK;
> > > +	char *envp[] = { "EVENT=offline", NULL };
> > >  
> > >  	if (acpi_bus_get_device(handle, &device))
> > >  		return AE_OK;
> > > @@ -166,7 +167,18 @@ static acpi_status acpi_bus_offline(acpi_handle handle, u32 lvl, void *data,
> > >  		} else {
> > >  			pn->put_online = false;
> > >  		}
> > > -		ret = device_offline(pn->dev);
> > > +
> > > +		/* Don't offline directly but need to notify userland first */
> > > +		if (device->request_offline) {
> > > +			if (pn->dev->offline)
> > > +				ret = 0;
> > > +			else
> > > +				ret = kobject_uevent_env(&pn->dev->kobj,
> > > +							KOBJ_CHANGE, envp);
> > 
> > So this is a userspace visable change with regards to kobject events?
> > 
> > Are you sure that is ok?
> > 
> 
> Since udev can see kobject events when devices are added, I haven't seen any
> risk if we make offline events visible too. Besides, normally online/eject
> attributes can only be written by root in userspace.
> 
Correct my explanation here: So far udev can see several device events already,
such as add, online, offline and remove. So I think it should not be risky if
we send additional change events to userspace as notification.

> Thanks,
> Chester Lin