Skip to contentSkip to conversation

ACPI / scan: Send the change uevent with offine environmental data

PATCH Superseded Lee, Chun-Yi 4 participants 5 messages Mar 2, 2018 – Mar 20, 2018 ACPI

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ACPI / scan: Send the change uevent with offine environmental data

In current design of ACPI container offline, Kernel emits KOBJ_CHANGE uevent to user space to indidate that the ejection of the container was triggered by platform. (caa73ea15 patch)

A pure KOBJ_CHANGE uevent is not enough for user space to identify the purpose. For example, a "udevadm trigger" command can also be used to emit change event to all udev rules. A udev rule can not identify that the event is from kernel for offline or from udevadm for other purpose. Then the offline action in udev rule may also be triggered by udevadm tool.

So, similar to the change uevent of dock, kernel sends the KOBJ_CHANGE uevent with a offline environmental data to indicate purpose. It's useful by udev rule for using ENV{EVENT} filter.

Cc Michal Hocko <[email protected]>
Cc "Rafael J. Wysocki" <[email protected]>
Cc Len Brown <[email protected]>
---
 drivers/acpi/scan.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/acpi/scan.c +2 -1
index 8e63d93..f6dca9b 100644
@@ -116,6 +116,7 @@ bool acpi_scan_is_offline(struct acpi_device *adev, bool uevent)
 {
 	struct acpi_device_physical_node *pn;
 	bool offline = true;
+	static const char *envp[] = { "EVENT=offline", NULL };
 
 	/*
 	 * acpi_container_offline() calls this for all of the container's
@@ -126,7 +127,7 @@ bool acpi_scan_is_offline(struct acpi_device *adev, bool uevent)
 	list_for_each_entry(pn, &adev->physical_node_list, node)
 		if (device_supports_offline(pn->dev) && !pn->dev->offline) {
 			if (uevent)
-				kobject_uevent(&pn->dev->kobj, KOBJ_CHANGE);
+				kobject_uevent_env(&pn->dev->kobj, KOBJ_CHANGE, envp);
 
 			offline = false;
 			break;
Signature · 1 line
2.10.2
Acked-by Tested-by on v1 1/1
#1 lore raw
Quoted 11 lines · Chun-Yi

In current design of ACPI container offline, Kernel emits KOBJ_CHANGE uevent to user space to indidate that the ejection of the container was triggered by platform. (caa73ea15 patch)

A pure KOBJ_CHANGE uevent is not enough for user space to identify the purpose. For example, a "udevadm trigger" command can also be used to emit change event to all udev rules. A udev rule can not identify that the event is from kernel for offline or from udevadm for other purpose. Then the offline action in udev rule may also be triggered by udevadm tool.

Quoted 3 lines

So, similar to the change uevent of dock, kernel sends the KOBJ_CHANGE uevent with a offline environmental data to indicate purpose. It's useful by udev rule for using ENV{EVENT} filter.

Looks reasonable to me. I have also tested this on Huawei Kunlun server which hits the offline & online storm as a result of udevadm triggered and a container udev rule which hooks into change event and offlines all devices attached to the container. This patch allowed the udev rule to be more targeted at the offline event.

Quoted 4 lines
Cc Michal Hocko <[email protected]>
Cc "Rafael J. Wysocki" <[email protected]>
Cc Len Brown <[email protected]>
Quoted 27 lines
---
 drivers/acpi/scan.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/acpi/scan.c +2 -1
index 8e63d93..f6dca9b 100644
@@ -116,6 +116,7 @@ bool acpi_scan_is_offline(struct acpi_device *adev, bool uevent)
 {
 	struct acpi_device_physical_node *pn;
 	bool offline = true;
+	static const char *envp[] = { "EVENT=offline", NULL };
 
 	/*
 	 * acpi_container_offline() calls this for all of the container's
@@ -126,7 +127,7 @@ bool acpi_scan_is_offline(struct acpi_device *adev, bool uevent)
 	list_for_each_entry(pn, &adev->physical_node_list, node)
 		if (device_supports_offline(pn->dev) && !pn->dev->offline) {
 			if (uevent)
-				kobject_uevent(&pn->dev->kobj, KOBJ_CHANGE);
+				kobject_uevent_env(&pn->dev->kobj, KOBJ_CHANGE, envp);
 
 			offline = false;
 			break;
Signature · 1 line
2.10.2
Signature · 2 lines
Michal Hocko
SUSE Labs
#2 lore raw
Quoted 22 lines · Michal Hocko
Quoted 11 lines · Chun-Yi

In current design of ACPI container offline, Kernel emits KOBJ_CHANGE uevent to user space to indidate that the ejection of the container was triggered by platform. (caa73ea15 patch)

A pure KOBJ_CHANGE uevent is not enough for user space to identify the purpose. For example, a "udevadm trigger" command can also be used to emit change event to all udev rules. A udev rule can not identify that the event is from kernel for offline or from udevadm for other purpose. Then the offline action in udev rule may also be triggered by udevadm tool.

Quoted 3 lines

So, similar to the change uevent of dock, kernel sends the KOBJ_CHANGE uevent with a offline environmental data to indicate purpose. It's useful by udev rule for using ENV{EVENT} filter.

Looks reasonable to me. I have also tested this on Huawei Kunlun server which hits the offline & online storm as a result of udevadm triggered and a container udev rule which hooks into change event and offlines all devices attached to the container. This patch allowed the udev rule to be more targeted at the offline event.

Quoted 8 lines
Quoted 4 lines
Cc Michal Hocko <[email protected]>
Cc "Rafael J. Wysocki" <[email protected]>
Cc Len Brown <[email protected]>

Thank you for review and testing.

Joey Lee

Quoted 31 lines
Quoted 27 lines
---
 drivers/acpi/scan.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/acpi/scan.c +2 -1
index 8e63d93..f6dca9b 100644
@@ -116,6 +116,7 @@ bool acpi_scan_is_offline(struct acpi_device *adev, bool uevent)
 {
 	struct acpi_device_physical_node *pn;
 	bool offline = true;
+	static const char *envp[] = { "EVENT=offline", NULL };
 
 	/*
 	 * acpi_container_offline() calls this for all of the container's
@@ -126,7 +127,7 @@ bool acpi_scan_is_offline(struct acpi_device *adev, bool uevent)
 	list_for_each_entry(pn, &adev->physical_node_list, node)
 		if (device_supports_offline(pn->dev) && !pn->dev->offline) {
 			if (uevent)
-				kobject_uevent(&pn->dev->kobj, KOBJ_CHANGE);
+				kobject_uevent_env(&pn->dev->kobj, KOBJ_CHANGE, envp);
 
 			offline = false;
 			break;
Signature · 1 line
2.10.2
Signature · 2 lines
Michal Hocko
SUSE Labs
#3 lore raw
In reply to Lee, Chun-Yi
Quoted 37 lines · Chun-Yi

In current design of ACPI container offline, Kernel emits KOBJ_CHANGE uevent to user space to indidate that the ejection of the container was triggered by platform. (caa73ea15 patch)

A pure KOBJ_CHANGE uevent is not enough for user space to identify the purpose. For example, a "udevadm trigger" command can also be used to emit change event to all udev rules. A udev rule can not identify that the event is from kernel for offline or from udevadm for other purpose. Then the offline action in udev rule may also be triggered by udevadm tool.

So, similar to the change uevent of dock, kernel sends the KOBJ_CHANGE uevent with a offline environmental data to indicate purpose. It's useful by udev rule for using ENV{EVENT} filter.

Cc Michal Hocko <[email protected]>
Cc "Rafael J. Wysocki" <[email protected]>
Cc Len Brown <[email protected]>
---
 drivers/acpi/scan.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/acpi/scan.c +1 -0
index 8e63d93..f6dca9b 100644
@@ -116,6 +116,7 @@ bool acpi_scan_is_offline(struct acpi_device *adev, bool uevent)
 {
 	struct acpi_device_physical_node *pn;
 	bool offline = true;
+	static const char *envp[] = { "EVENT=offline", NULL };
 
 	/*
 	 * acpi_container_offline() calls this for all of the container's

drivers/acpi/scan.c bool acpi_scan_is_offline(struct acpi_device *adev, bool uevent) · line 133

Quoted 8 lines
drivers/acpi/scan.c +1 -1
@@ -126,7 +127,7 @@ bool acpi_scan_is_offline(struct acpi_device *adev, bool uevent)
 	list_for_each_entry(pn, &adev->physical_node_list, node)
 		if (device_supports_offline(pn->dev) && !pn->dev->offline) {
 			if (uevent)
-				kobject_uevent(&pn->dev->kobj, KOBJ_CHANGE);
+				kobject_uevent_env(&pn->dev->kobj, KOBJ_CHANGE, envp);
 
 			offline = false;
 			break;

This causes build issues when applied, please fix.

Thanks!

#4 lore raw

Hi Rafael,

Quoted 43 lines · Rafael J. Wysocki
Quoted 42 lines · Chun-Yi

In current design of ACPI container offline, Kernel emits KOBJ_CHANGE uevent to user space to indidate that the ejection of the container was triggered by platform. (caa73ea15 patch)

A pure KOBJ_CHANGE uevent is not enough for user space to identify the purpose. For example, a "udevadm trigger" command can also be used to emit change event to all udev rules. A udev rule can not identify that the event is from kernel for offline or from udevadm for other purpose. Then the offline action in udev rule may also be triggered by udevadm tool.

So, similar to the change uevent of dock, kernel sends the KOBJ_CHANGE uevent with a offline environmental data to indicate purpose. It's useful by udev rule for using ENV{EVENT} filter.

Cc Michal Hocko <[email protected]>
Cc "Rafael J. Wysocki" <[email protected]>
Cc Len Brown <[email protected]>
---
 drivers/acpi/scan.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/acpi/scan.c +2 -1
index 8e63d93..f6dca9b 100644
@@ -116,6 +116,7 @@ bool acpi_scan_is_offline(struct acpi_device *adev, bool uevent)
 {
 	struct acpi_device_physical_node *pn;
 	bool offline = true;
+	static const char *envp[] = { "EVENT=offline", NULL };
 
 	/*
 	 * acpi_container_offline() calls this for all of the container's
@@ -126,7 +127,7 @@ bool acpi_scan_is_offline(struct acpi_device *adev, bool uevent)
 	list_for_each_entry(pn, &adev->physical_node_list, node)
 		if (device_supports_offline(pn->dev) && !pn->dev->offline) {
 			if (uevent)
-				kobject_uevent(&pn->dev->kobj, KOBJ_CHANGE);
+				kobject_uevent_env(&pn->dev->kobj, KOBJ_CHANGE, envp);
 
Quoted 6 lines

drivers/acpi/scan.c bool acpi_scan_is_offline(struct acpi_device *adev, bool uevent) · line 133

Quoted 3 lines
drivers/acpi/scan.c +0 -0
 			offline = false;
 			break;

This causes build issues when applied, please fix.

I just sent the v2 patch. Thanks for your review!

Joey Lee