acpi : Add container online uevent to acpi_bus_attach

PATCH In review Yasuaki Ishimatsu 3 participants 12 messages Sep 5, 2014 – Sep 25, 2014

Subsystems (2)

ACPI ACPI component architecture (acpica)

lore

Share & tools

lore

Yasuaki Ishimatsu

author

2014-09-05 06:58 UTC #0 lore raw

PATCH 1/1 acpi : Add container online uevent to acpi_bus_attach

Container online uevent was deleted by "46394fd01 : ACPI / hotplug:
Move container-specific code out of the core" because container- specific uevent is raised to udev by attaching container device. But the container-specific uevent is not useful.

In my box, conainer device has CPU and memory devices. In this case, when hot adding container device, the following uevets are raised to udev.

  # udevadm monitor --kernel
  monitor will print the received events for:
  KERNEL - the kernel uevent

KERNEL[...] add /devices/system/container/ACPI0004:01 (container)
<snip>
KERNEL[...] add /devices/system/memory/memory2048 (memory)
KERNEL[...] add /devices/system/memory/memory2049 (memory)
<snip>
KERNEL[...] add /devices/system/memory/memory2063 (memory)
<snip>
KERNEL[...] add /devices/system/cpu/cpu60 (cpu)
<snip>
KERNEL[...] add /devices/system/cpu/cpu119 (cpu)

When udev catches the container add uevent in my box, udev executes user land script for onlining all child's devices. But memory and CPU devices have not been attached at this time. So user land script fails.

One of solutions is that user land script waits for all child's devices to attach. But user land script has no way to know all child's devices were attached.

So the patch adds container online uevent to acpi_bus_sttach(). By applying
the patch, container online uevent is raised to udev after all child's devices were attached as follows:

  # udevadm monitor --kernel
  monitor will print the received events for:
  KERNEL - the kernel uevent

So if user land script is executed after raising the container online uevent, it guarantees that all child's devices were attached.

Signed-off-by Yasuaki Ishimatsu <[email protected]>

---
 drivers/acpi/container.c | 15 +++++++++++++++
 drivers/acpi/internal.h  |  7 +++++++
 drivers/acpi/scan.c      |  3 +++
 3 files changed, 25 insertions(+)

drivers/acpi/container.c +15 -0

index 76f7cff..eba5746 100644
@@ -43,6 +43,21 @@ static const struct acpi_device_id container_device_ids[] = {
 
 #ifdef CONFIG_ACPI_CONTAINER
 
+void notify_container_device(struct acpi_device *adev)
+{
+	struct device *dev = acpi_driver_data(adev);
+
+	kobject_uevent(&dev->kobj, KOBJ_ONLINE);
+}
+
+int is_container_device(struct acpi_device *adev)
+{
+	if (acpi_match_device_ids(adev, container_device_ids))
+		return 0;
+
+	return 1;
+}
+
 static int acpi_container_offline(struct container_dev *cdev)
 {
 	struct acpi_device *adev = ACPI_COMPANION(&cdev->dev);

drivers/acpi/internal.h +7 -0

index 4c5cf77..12dd89b 100644
@@ -45,6 +45,13 @@ static inline void register_dock_dependent_device(struct acpi_device *adev,
 static inline int dock_notify(struct acpi_device *adev, u32 event) { return -ENODEV; }
 static inline void acpi_dock_add(struct acpi_device *adev) {}
 #endif
+#if CONFIG_ACPI_CONTAINER
+int is_container_device(struct acpi_device *adev);
+void notify_container_device(struct acpi_device *adev);
+#else
+static inline int is_container_device(struct acpi_device *adev) { return 0; }
+static inline void notify_container_device(struct acpi_device *adev) {}
+#endif
 #ifdef CONFIG_X86
 void acpi_cmos_rtc_init(void);
 #else

drivers/acpi/scan.c +3 -0

index 9a92989..faa3d11 100644
@@ -2188,6 +2188,9 @@ static void acpi_bus_attach(struct acpi_device *device)
  ok:
 	list_for_each_entry(child, &device->children, node)
 		acpi_bus_attach(child);
+
+	if (is_container_device(device))
+		notify_container_device(device);
 }
 
 /**

Signature · 1 line

1.8.3.1

joeyli

Tested-by on v1 1/1

2014-09-05 12:14 UTC #1 lore raw

Hi Yasuaki,

Quoted 53 lines · Yasuaki Ishimatsu

Container online uevent was deleted by "46394fd01 : ACPI / hotplug:
Move container-specific code out of the core" because container- specific uevent is raised to udev by attaching container device. But the container-specific uevent is not useful.
In my box, conainer device has CPU and memory devices. In this case, when hot adding container device, the following uevets are raised to udev.
  # udevadm monitor --kernel
  monitor will print the received events for:
  KERNEL - the kernel uevent
KERNEL[...] add /devices/system/container/ACPI0004:01 (container)
<snip>
KERNEL[...] add /devices/system/memory/memory2048 (memory)
KERNEL[...] add /devices/system/memory/memory2049 (memory)
<snip>
KERNEL[...] add /devices/system/memory/memory2063 (memory)
<snip>
KERNEL[...] add /devices/system/cpu/cpu60 (cpu)
<snip>
KERNEL[...] add /devices/system/cpu/cpu119 (cpu)
When udev catches the container add uevent in my box, udev executes user land script for onlining all child's devices. But memory and CPU devices have not been attached at this time. So user land script fails.
One of solutions is that user land script waits for all child's devices to attach. But user land script has no way to know all child's devices were attached.
So the patch adds container online uevent to acpi_bus_sttach(). By applying
the patch, container online uevent is raised to udev after all child's devices were attached as follows:
  # udevadm monitor --kernel
  monitor will print the received events for:
  KERNEL - the kernel uevent
KERNEL[...] add /devices/system/container/ACPI0004:01 (container)
<snip>
KERNEL[...] add /devices/system/memory/memory2048 (memory)
KERNEL[...] add /devices/system/memory/memory2049 (memory)
<snip>
KERNEL[...] add /devices/system/memory/memory2063 (memory)
<snip>
KERNEL[...] add /devices/system/cpu/cpu60 (cpu)
<snip>
KERNEL[...] add /devices/system/cpu/cpu119 (cpu)
KERNEL[...] online /devices/system/container/ACPI0004:01 (container)

Quoted 4 lines

So if user land script is executed after raising the container online uevent, it guarantees that all child's devices were attached.
Signed-off-by Yasuaki Ishimatsu <[email protected]>

Your patch works on my side.

Tested-by Lee, Chun-Yi <[email protected]>

Thanks a lot!
joey Lee

Quoted 72 lines

---
 drivers/acpi/container.c | 15 +++++++++++++++
 drivers/acpi/internal.h  |  7 +++++++
 drivers/acpi/scan.c      |  3 +++
 3 files changed, 25 insertions(+)

drivers/acpi/container.c +15 -0

index 76f7cff..eba5746 100644
@@ -43,6 +43,21 @@ static const struct acpi_device_id container_device_ids[] = {
 
 #ifdef CONFIG_ACPI_CONTAINER
 
+void notify_container_device(struct acpi_device *adev)
+{
+	struct device *dev = acpi_driver_data(adev);
+
+	kobject_uevent(&dev->kobj, KOBJ_ONLINE);
+}
+
+int is_container_device(struct acpi_device *adev)
+{
+	if (acpi_match_device_ids(adev, container_device_ids))
+		return 0;
+
+	return 1;
+}
+
 static int acpi_container_offline(struct container_dev *cdev)
 {
 	struct acpi_device *adev = ACPI_COMPANION(&cdev->dev);

drivers/acpi/internal.h +7 -0

index 4c5cf77..12dd89b 100644
@@ -45,6 +45,13 @@ static inline void register_dock_dependent_device(struct acpi_device *adev,
 static inline int dock_notify(struct acpi_device *adev, u32 event) { return -ENODEV; }
 static inline void acpi_dock_add(struct acpi_device *adev) {}
 #endif
+#if CONFIG_ACPI_CONTAINER
+int is_container_device(struct acpi_device *adev);
+void notify_container_device(struct acpi_device *adev);
+#else
+static inline int is_container_device(struct acpi_device *adev) { return 0; }
+static inline void notify_container_device(struct acpi_device *adev) {}
+#endif
 #ifdef CONFIG_X86
 void acpi_cmos_rtc_init(void);
 #else

drivers/acpi/scan.c +3 -0

index 9a92989..faa3d11 100644
@@ -2188,6 +2188,9 @@ static void acpi_bus_attach(struct acpi_device *device)
  ok:
 	list_for_each_entry(child, &device->children, node)
 		acpi_bus_attach(child);
+
+	if (is_container_device(device))
+		notify_container_device(device);
 }
 
 /**

Signature · 7 lines

1.8.3.1


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

Rafael J. Wysocki

2014-09-09 13:36 UTC #2 lore raw

Quoted 56 lines · joeyli

Hi Yasuaki,
Quoted 53 lines · Yasuaki Ishimatsu
Container online uevent was deleted by "46394fd01 : ACPI / hotplug:
Move container-specific code out of the core" because container- specific uevent is raised to udev by attaching container device. But the container-specific uevent is not useful.
In my box, conainer device has CPU and memory devices. In this case, when hot adding container device, the following uevets are raised to udev.
  # udevadm monitor --kernel
  monitor will print the received events for:
  KERNEL - the kernel uevent
KERNEL[...] add /devices/system/container/ACPI0004:01 (container)
<snip>
KERNEL[...] add /devices/system/memory/memory2048 (memory)
KERNEL[...] add /devices/system/memory/memory2049 (memory)
<snip>
KERNEL[...] add /devices/system/memory/memory2063 (memory)
<snip>
KERNEL[...] add /devices/system/cpu/cpu60 (cpu)
<snip>
KERNEL[...] add /devices/system/cpu/cpu119 (cpu)
When udev catches the container add uevent in my box, udev executes user land script for onlining all child's devices. But memory and CPU devices have not been attached at this time. So user land script fails.
One of solutions is that user land script waits for all child's devices to attach. But user land script has no way to know all child's devices were attached.
So the patch adds container online uevent to acpi_bus_sttach(). By applying
the patch, container online uevent is raised to udev after all child's devices were attached as follows:
  # udevadm monitor --kernel
  monitor will print the received events for:
  KERNEL - the kernel uevent
KERNEL[...] add /devices/system/container/ACPI0004:01 (container)
<snip>
KERNEL[...] add /devices/system/memory/memory2048 (memory)
KERNEL[...] add /devices/system/memory/memory2049 (memory)
<snip>
KERNEL[...] add /devices/system/memory/memory2063 (memory)
<snip>
KERNEL[...] add /devices/system/cpu/cpu60 (cpu)
<snip>
KERNEL[...] add /devices/system/cpu/cpu119 (cpu)
KERNEL[...] online /devices/system/container/ACPI0004:01 (container)

Quoted 8 lines

Quoted 4 lines
So if user land script is executed after raising the container online uevent, it guarantees that all child's devices were attached.
Signed-off-by Yasuaki Ishimatsu <[email protected]>
Your patch works on my side.
Tested-by Lee, Chun-Yi <[email protected]>

Can you please try this one too: https://patchwork.kernel.org/patch/4859321/

Yasuaki Ishimatsu

author

2014-09-10 03:20 UTC #3 lore raw

In reply to joeyli

Quoted 56 lines · joeyli

Hi Yasuaki,
Quoted 53 lines · Yasuaki Ishimatsu
Container online uevent was deleted by "46394fd01 : ACPI / hotplug:
Move container-specific code out of the core" because container- specific uevent is raised to udev by attaching container device. But the container-specific uevent is not useful.
In my box, conainer device has CPU and memory devices. In this case, when hot adding container device, the following uevets are raised to udev.
   # udevadm monitor --kernel
   monitor will print the received events for:
   KERNEL - the kernel uevent
   KERNEL[...] add      /devices/system/container/ACPI0004:01 (container)
   <snip>
   KERNEL[...] add      /devices/system/memory/memory2048 (memory)
   KERNEL[...] add      /devices/system/memory/memory2049 (memory)
   <snip>
   KERNEL[...] add      /devices/system/memory/memory2063 (memory)
   <snip>
   KERNEL[...] add      /devices/system/cpu/cpu60 (cpu)
   <snip>
   KERNEL[...] add      /devices/system/cpu/cpu119 (cpu)
When udev catches the container add uevent in my box, udev executes user land script for onlining all child's devices. But memory and CPU devices have not been attached at this time. So user land script fails.
One of solutions is that user land script waits for all child's devices to attach. But user land script has no way to know all child's devices were attached.
So the patch adds container online uevent to acpi_bus_sttach(). By applying
the patch, container online uevent is raised to udev after all child's devices were attached as follows:
   # udevadm monitor --kernel
   monitor will print the received events for:
   KERNEL - the kernel uevent
   KERNEL[...] add      /devices/system/container/ACPI0004:01 (container)
   <snip>
   KERNEL[...] add      /devices/system/memory/memory2048 (memory)
   KERNEL[...] add      /devices/system/memory/memory2049 (memory)
   <snip>
   KERNEL[...] add      /devices/system/memory/memory2063 (memory)
   <snip>
   KERNEL[...] add      /devices/system/cpu/cpu60 (cpu)
   <snip>
   KERNEL[...] add      /devices/system/cpu/cpu119 (cpu)
   KERNEL[...] online   /devices/system/container/ACPI0004:01 (container)

Quoted 8 lines

Quoted 4 lines
So if user land script is executed after raising the container online uevent, it guarantees that all child's devices were attached.
Signed-off-by Yasuaki Ishimatsu <[email protected]>
Your patch works on my side.
Tested-by Lee, Chun-Yi <[email protected]>

Hi Lee,

Thank you for your test. But Rafael posted new patch. Could you try the patch. In my box, the Rafael's patch works well.

Thanks,
Yasuaki Ishimatsu

Quoted 76 lines

Thanks a lot!
joey Lee

Quoted 72 lines

---
  drivers/acpi/container.c | 15 +++++++++++++++
  drivers/acpi/internal.h  |  7 +++++++
  drivers/acpi/scan.c      |  3 +++
  3 files changed, 25 insertions(+)

drivers/acpi/container.c +15 -0

index 76f7cff..eba5746 100644
@@ -43,6 +43,21 @@ static const struct acpi_device_id container_device_ids[] = {
 
  #ifdef CONFIG_ACPI_CONTAINER
 
+void notify_container_device(struct acpi_device *adev)
+{
+	struct device *dev = acpi_driver_data(adev);
+
+	kobject_uevent(&dev->kobj, KOBJ_ONLINE);
+}
+
+int is_container_device(struct acpi_device *adev)
+{
+	if (acpi_match_device_ids(adev, container_device_ids))
+		return 0;
+
+	return 1;
+}
+
  static int acpi_container_offline(struct container_dev *cdev)
  {
  	struct acpi_device *adev = ACPI_COMPANION(&cdev->dev);

drivers/acpi/internal.h +7 -0

index 4c5cf77..12dd89b 100644
@@ -45,6 +45,13 @@ static inline void register_dock_dependent_device(struct acpi_device *adev,
  static inline int dock_notify(struct acpi_device *adev, u32 event) { return -ENODEV; }
  static inline void acpi_dock_add(struct acpi_device *adev) {}
  #endif
+#if CONFIG_ACPI_CONTAINER
+int is_container_device(struct acpi_device *adev);
+void notify_container_device(struct acpi_device *adev);
+#else
+static inline int is_container_device(struct acpi_device *adev) { return 0; }
+static inline void notify_container_device(struct acpi_device *adev) {}
+#endif
  #ifdef CONFIG_X86
  void acpi_cmos_rtc_init(void);
  #else

drivers/acpi/scan.c +3 -1

index 9a92989..faa3d11 100644
@@ -2188,6 +2188,9 @@ static void acpi_bus_attach(struct acpi_device *device)
   ok:
  	list_for_each_entry(child, &device->children, node)
  		acpi_bus_attach(child);
+
+	if (is_container_device(device))
+		notify_container_device(device);
  }
 
  /**
--

1.8.3.1

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

Rafael J. Wysocki

2014-09-07 21:15 UTC #4 lore raw

In reply to Yasuaki Ishimatsu

Quoted 53 lines · Yasuaki Ishimatsu

Container online uevent was deleted by "46394fd01 : ACPI / hotplug:
Move container-specific code out of the core" because container- specific uevent is raised to udev by attaching container device. But the container-specific uevent is not useful.
In my box, conainer device has CPU and memory devices. In this case, when hot adding container device, the following uevets are raised to udev.
  # udevadm monitor --kernel
  monitor will print the received events for:
  KERNEL - the kernel uevent
KERNEL[...] add /devices/system/container/ACPI0004:01 (container)
<snip>
KERNEL[...] add /devices/system/memory/memory2048 (memory)
KERNEL[...] add /devices/system/memory/memory2049 (memory)
<snip>
KERNEL[...] add /devices/system/memory/memory2063 (memory)
<snip>
KERNEL[...] add /devices/system/cpu/cpu60 (cpu)
<snip>
KERNEL[...] add /devices/system/cpu/cpu119 (cpu)
When udev catches the container add uevent in my box, udev executes user land script for onlining all child's devices. But memory and CPU devices have not been attached at this time. So user land script fails.
One of solutions is that user land script waits for all child's devices to attach. But user land script has no way to know all child's devices were attached.
So the patch adds container online uevent to acpi_bus_sttach(). By applying
the patch, container online uevent is raised to udev after all child's devices were attached as follows:
  # udevadm monitor --kernel
  monitor will print the received events for:
  KERNEL - the kernel uevent
KERNEL[...] add /devices/system/container/ACPI0004:01 (container)
<snip>
KERNEL[...] add /devices/system/memory/memory2048 (memory)
KERNEL[...] add /devices/system/memory/memory2049 (memory)
<snip>
KERNEL[...] add /devices/system/memory/memory2063 (memory)
<snip>
KERNEL[...] add /devices/system/cpu/cpu60 (cpu)
<snip>
KERNEL[...] add /devices/system/cpu/cpu119 (cpu)
KERNEL[...] online /devices/system/container/ACPI0004:01 (container)

Quoted 2 lines

So if user land script is executed after raising the container online uevent, it guarantees that all child's devices were attached.

I see the problem, but I don't like the patch.

For example, we don't need to match the container's name again, we can use the fact that we've already matched the container scan handler to it.

Does the one below work too?

Rafael

---
 drivers/acpi/container.c |    8 ++++++++
 drivers/acpi/scan.c      |    3 +++
 include/acpi/acpi_bus.h  |    1 +
 3 files changed, 12 insertions(+)

include/acpi/acpi_bus.h +1 -0

Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
@@ -118,6 +118,7 @@ struct acpi_device;
 struct acpi_hotplug_profile {
 	struct kobject kobj;
 	int (*scan_dependent)(struct acpi_device *adev);
+	void (*notify_online)(struct acpi_device *adev);
 	bool enabled:1;
 	bool demand_offline:1;
 };

drivers/acpi/container.c +8 -0

Index: linux-pm/drivers/acpi/container.c
===================================================================
@@ -99,6 +99,13 @@ static void container_device_detach(stru
 		device_unregister(dev);
 }
 
+static void container_device_online(struct acpi_device *adev)
+{
+	struct device *dev = acpi_driver_data(adev);
+
+	kobject_uevent(&dev->kobj, KOBJ_ONLINE);
+}
+
 static struct acpi_scan_handler container_handler = {
 	.ids = container_device_ids,
 	.attach = container_device_attach,

@@ -106,6 +113,7 @@ static struct acpi_scan_handler containe
 	.hotplug = {
 		.enabled = true,
 		.demand_offline = true,
+		.notify_online = container_device_online,
 	},
 };

drivers/acpi/scan.c +3 -0

Index: linux-pm/drivers/acpi/scan.c
===================================================================
@@ -2190,6 +2190,9 @@ static void acpi_bus_attach(struct acpi_
  ok:
 	list_for_each_entry(child, &device->children, node)
 		acpi_bus_attach(child);
+
+	if (device->handler && device->handler->hotplug.notify_online)
+		device->handler->hotplug.notify_online(device);
 }
 
 /**

Yasuaki Ishimatsu

author

2014-09-10 03:17 UTC #5 lore raw

Quoted 57 lines · Rafael J. Wysocki

Quoted 53 lines · Yasuaki Ishimatsu
Container online uevent was deleted by "46394fd01 : ACPI / hotplug:
Move container-specific code out of the core" because container- specific uevent is raised to udev by attaching container device. But the container-specific uevent is not useful.
In my box, conainer device has CPU and memory devices. In this case, when hot adding container device, the following uevets are raised to udev.
   # udevadm monitor --kernel
   monitor will print the received events for:
   KERNEL - the kernel uevent
   KERNEL[...] add      /devices/system/container/ACPI0004:01 (container)
   <snip>
   KERNEL[...] add      /devices/system/memory/memory2048 (memory)
   KERNEL[...] add      /devices/system/memory/memory2049 (memory)
   <snip>
   KERNEL[...] add      /devices/system/memory/memory2063 (memory)
   <snip>
   KERNEL[...] add      /devices/system/cpu/cpu60 (cpu)
   <snip>
   KERNEL[...] add      /devices/system/cpu/cpu119 (cpu)
When udev catches the container add uevent in my box, udev executes user land script for onlining all child's devices. But memory and CPU devices have not been attached at this time. So user land script fails.
One of solutions is that user land script waits for all child's devices to attach. But user land script has no way to know all child's devices were attached.
So the patch adds container online uevent to acpi_bus_sttach(). By applying
the patch, container online uevent is raised to udev after all child's devices were attached as follows:
   # udevadm monitor --kernel
   monitor will print the received events for:
   KERNEL - the kernel uevent
   KERNEL[...] add      /devices/system/container/ACPI0004:01 (container)
   <snip>
   KERNEL[...] add      /devices/system/memory/memory2048 (memory)
   KERNEL[...] add      /devices/system/memory/memory2049 (memory)
   <snip>
   KERNEL[...] add      /devices/system/memory/memory2063 (memory)
   <snip>
   KERNEL[...] add      /devices/system/cpu/cpu60 (cpu)
   <snip>
   KERNEL[...] add      /devices/system/cpu/cpu119 (cpu)
   KERNEL[...] online   /devices/system/container/ACPI0004:01 (container)
Quoted 2 lines
So if user land script is executed after raising the container online uevent, it guarantees that all child's devices were attached.

Quoted 6 lines

I see the problem, but I don't like the patch.
For example, we don't need to match the container's name again, we can use the fact that we've already matched the container scan handler to it.
Does the one below work too?

Hi Rafael,

Thank you for posting your patch. I confirmed that your patch fixed my issue as folloes:

# udevadm monitor --kernel
monitor will print the received events for:
KERNEL - the kernel uevent

KERNEL[...] add /devices/system/container/ACPI0004:01 (container)
<snip>
KERNEL[...] add /devices/system/memory/memory2048 (memory)
<snip>
KERNEL[...] add /devices/system/memory/memory2063 (memory)
<snip>
KERNEL[...] add /devices/system/cpu/cpu60 (cpu)
<snip>
KERNEL[...] add /devices/system/cpu/cpu119 (cpu)
KERNEL[...] online /devices/system/container/ACPI0004:01 (container)

So feel free to add "Tested-by : Yasuaki Ishimatsu <[email protected]>" to the patch.

Thanks,
Yasuaki Ishimatsu

Quoted 63 lines

Rafael

---
  drivers/acpi/container.c |    8 ++++++++
  drivers/acpi/scan.c      |    3 +++
  include/acpi/acpi_bus.h  |    1 +
  3 files changed, 12 insertions(+)

Index: linux-pm/include/acpi/acpi_bus.h
===================================================================

include/acpi/acpi_bus.h +1 -0

@@ -118,6 +118,7 @@ struct acpi_device;
  struct acpi_hotplug_profile {
  	struct kobject kobj;
  	int (*scan_dependent)(struct acpi_device *adev);
+	void (*notify_online)(struct acpi_device *adev);
  	bool enabled:1;
  	bool demand_offline:1;
  };

Index: linux-pm/drivers/acpi/container.c
===================================================================

drivers/acpi/container.c +8 -0

@@ -99,6 +99,13 @@ static void container_device_detach(stru
  		device_unregister(dev);
  }
 
+static void container_device_online(struct acpi_device *adev)
+{
+	struct device *dev = acpi_driver_data(adev);
+
+	kobject_uevent(&dev->kobj, KOBJ_ONLINE);
+}
+
  static struct acpi_scan_handler container_handler = {
  	.ids = container_device_ids,
  	.attach = container_device_attach,

@@ -106,6 +113,7 @@ static struct acpi_scan_handler containe
  	.hotplug = {
  		.enabled = true,
  		.demand_offline = true,
+		.notify_online = container_device_online,
  	},
  };

Index: linux-pm/drivers/acpi/scan.c
===================================================================

drivers/acpi/scan.c +3 -0

@@ -2190,6 +2190,9 @@ static void acpi_bus_attach(struct acpi_
   ok:
  	list_for_each_entry(child, &device->children, node)
  		acpi_bus_attach(child);
+
+	if (device->handler && device->handler->hotplug.notify_online)
+		device->handler->hotplug.notify_online(device);
  }
 
  /**

Rafael J. Wysocki

2014-09-10 23:19 UTC #6 lore raw

Quoted 85 lines · Yasuaki Ishimatsu

Quoted 57 lines · Rafael J. Wysocki
Quoted 53 lines · Yasuaki Ishimatsu
Container online uevent was deleted by "46394fd01 : ACPI / hotplug:
Move container-specific code out of the core" because container- specific uevent is raised to udev by attaching container device. But the container-specific uevent is not useful.
In my box, conainer device has CPU and memory devices. In this case, when hot adding container device, the following uevets are raised to udev.
   # udevadm monitor --kernel
   monitor will print the received events for:
   KERNEL - the kernel uevent
   KERNEL[...] add      /devices/system/container/ACPI0004:01 (container)
   <snip>
   KERNEL[...] add      /devices/system/memory/memory2048 (memory)
   KERNEL[...] add      /devices/system/memory/memory2049 (memory)
   <snip>
   KERNEL[...] add      /devices/system/memory/memory2063 (memory)
   <snip>
   KERNEL[...] add      /devices/system/cpu/cpu60 (cpu)
   <snip>
   KERNEL[...] add      /devices/system/cpu/cpu119 (cpu)
When udev catches the container add uevent in my box, udev executes user land script for onlining all child's devices. But memory and CPU devices have not been attached at this time. So user land script fails.
One of solutions is that user land script waits for all child's devices to attach. But user land script has no way to know all child's devices were attached.
So the patch adds container online uevent to acpi_bus_sttach(). By applying
the patch, container online uevent is raised to udev after all child's devices were attached as follows:
   # udevadm monitor --kernel
   monitor will print the received events for:
   KERNEL - the kernel uevent
   KERNEL[...] add      /devices/system/container/ACPI0004:01 (container)
   <snip>
   KERNEL[...] add      /devices/system/memory/memory2048 (memory)
   KERNEL[...] add      /devices/system/memory/memory2049 (memory)
   <snip>
   KERNEL[...] add      /devices/system/memory/memory2063 (memory)
   <snip>
   KERNEL[...] add      /devices/system/cpu/cpu60 (cpu)
   <snip>
   KERNEL[...] add      /devices/system/cpu/cpu119 (cpu)
   KERNEL[...] online   /devices/system/container/ACPI0004:01 (container)
Quoted 2 lines
So if user land script is executed after raising the container online uevent, it guarantees that all child's devices were attached.
Quoted 6 lines
I see the problem, but I don't like the patch.
For example, we don't need to match the container's name again, we can use the fact that we've already matched the container scan handler to it.
Does the one below work too?
Hi Rafael,
Thank you for posting your patch. I confirmed that your patch fixed my issue as folloes:
# udevadm monitor --kernel
monitor will print the received events for:
KERNEL - the kernel uevent
KERNEL[...] add /devices/system/container/ACPI0004:01 (container)
<snip>
KERNEL[...] add /devices/system/memory/memory2048 (memory)
<snip>
KERNEL[...] add /devices/system/memory/memory2063 (memory)
<snip>
KERNEL[...] add /devices/system/cpu/cpu60 (cpu)
<snip>
KERNEL[...] add /devices/system/cpu/cpu119 (cpu)
KERNEL[...] online /devices/system/container/ACPI0004:01 (container)

Quoted 2 lines

So feel free to add "Tested-by : Yasuaki Ishimatsu <[email protected]>" to the patch.

Thanks!

Signature · 2 lines

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

joeyli

Tested-by

2014-09-11 07:17 UTC #7 lore raw

Hi Rafael,

Quoted 86 lines · Rafael J. Wysocki

Quoted 85 lines · Yasuaki Ishimatsu
Quoted 57 lines · Rafael J. Wysocki
Quoted 53 lines · Yasuaki Ishimatsu
Container online uevent was deleted by "46394fd01 : ACPI / hotplug:
Move container-specific code out of the core" because container- specific uevent is raised to udev by attaching container device. But the container-specific uevent is not useful.
In my box, conainer device has CPU and memory devices. In this case, when hot adding container device, the following uevets are raised to udev.
   # udevadm monitor --kernel
   monitor will print the received events for:
   KERNEL - the kernel uevent
   KERNEL[...] add      /devices/system/container/ACPI0004:01 (container)
   <snip>
   KERNEL[...] add      /devices/system/memory/memory2048 (memory)
   KERNEL[...] add      /devices/system/memory/memory2049 (memory)
   <snip>
   KERNEL[...] add      /devices/system/memory/memory2063 (memory)
   <snip>
   KERNEL[...] add      /devices/system/cpu/cpu60 (cpu)
   <snip>
   KERNEL[...] add      /devices/system/cpu/cpu119 (cpu)
When udev catches the container add uevent in my box, udev executes user land script for onlining all child's devices. But memory and CPU devices have not been attached at this time. So user land script fails.
One of solutions is that user land script waits for all child's devices to attach. But user land script has no way to know all child's devices were attached.
So the patch adds container online uevent to acpi_bus_sttach(). By applying
the patch, container online uevent is raised to udev after all child's devices were attached as follows:
   # udevadm monitor --kernel
   monitor will print the received events for:
   KERNEL - the kernel uevent
   KERNEL[...] add      /devices/system/container/ACPI0004:01 (container)
   <snip>
   KERNEL[...] add      /devices/system/memory/memory2048 (memory)
   KERNEL[...] add      /devices/system/memory/memory2049 (memory)
   <snip>
   KERNEL[...] add      /devices/system/memory/memory2063 (memory)
   <snip>
   KERNEL[...] add      /devices/system/cpu/cpu60 (cpu)
   <snip>
   KERNEL[...] add      /devices/system/cpu/cpu119 (cpu)
   KERNEL[...] online   /devices/system/container/ACPI0004:01 (container)
Quoted 2 lines
So if user land script is executed after raising the container online uevent, it guarantees that all child's devices were attached.
Quoted 6 lines
I see the problem, but I don't like the patch.
For example, we don't need to match the container's name again, we can use the fact that we've already matched the container scan handler to it.
Does the one below work too?
Hi Rafael,
Thank you for posting your patch. I confirmed that your patch fixed my issue as folloes:
# udevadm monitor --kernel
monitor will print the received events for:
KERNEL - the kernel uevent
KERNEL[...] add /devices/system/container/ACPI0004:01 (container)
<snip>
KERNEL[...] add /devices/system/memory/memory2048 (memory)
<snip>
KERNEL[...] add /devices/system/memory/memory2063 (memory)
<snip>
KERNEL[...] add /devices/system/cpu/cpu60 (cpu)
<snip>
KERNEL[...] add /devices/system/cpu/cpu119 (cpu)
KERNEL[...] online /devices/system/container/ACPI0004:01 (container)

Quoted 5 lines

Quoted 2 lines
So feel free to add "Tested-by : Yasuaki Ishimatsu <[email protected]>" to the patch.
Thanks!

I just tested your new patch, it works on my side as Yasuaki's patch.

Tested-by Lee, Chun-Yi <[email protected]>

Thanks a lot!
Joey Lee

Rafael J. Wysocki

2014-09-11 12:54 UTC #8 lore raw

Quoted 92 lines · joeyli

Hi Rafael,
Quoted 89 lines · Rafael J. Wysocki
Quoted 87 lines · Yasuaki Ishimatsu
Quoted 57 lines · Rafael J. Wysocki
Quoted 53 lines · Yasuaki Ishimatsu
Container online uevent was deleted by "46394fd01 : ACPI / hotplug:
Move container-specific code out of the core" because container- specific uevent is raised to udev by attaching container device. But the container-specific uevent is not useful.
In my box, conainer device has CPU and memory devices. In this case, when hot adding container device, the following uevets are raised to udev.
   # udevadm monitor --kernel
   monitor will print the received events for:
   KERNEL - the kernel uevent
   KERNEL[...] add      /devices/system/container/ACPI0004:01 (container)
   <snip>
   KERNEL[...] add      /devices/system/memory/memory2048 (memory)
   KERNEL[...] add      /devices/system/memory/memory2049 (memory)
   <snip>
   KERNEL[...] add      /devices/system/memory/memory2063 (memory)
   <snip>
   KERNEL[...] add      /devices/system/cpu/cpu60 (cpu)
   <snip>
   KERNEL[...] add      /devices/system/cpu/cpu119 (cpu)
When udev catches the container add uevent in my box, udev executes user land script for onlining all child's devices. But memory and CPU devices have not been attached at this time. So user land script fails.
One of solutions is that user land script waits for all child's devices to attach. But user land script has no way to know all child's devices were attached.
So the patch adds container online uevent to acpi_bus_sttach(). By applying
the patch, container online uevent is raised to udev after all child's devices were attached as follows:
   # udevadm monitor --kernel
   monitor will print the received events for:
   KERNEL - the kernel uevent
   KERNEL[...] add      /devices/system/container/ACPI0004:01 (container)
   <snip>
   KERNEL[...] add      /devices/system/memory/memory2048 (memory)
   KERNEL[...] add      /devices/system/memory/memory2049 (memory)
   <snip>
   KERNEL[...] add      /devices/system/memory/memory2063 (memory)
   <snip>
   KERNEL[...] add      /devices/system/cpu/cpu60 (cpu)
   <snip>
   KERNEL[...] add      /devices/system/cpu/cpu119 (cpu)
   KERNEL[...] online   /devices/system/container/ACPI0004:01 (container)
Quoted 2 lines
So if user land script is executed after raising the container online uevent, it guarantees that all child's devices were attached.
Quoted 6 lines
I see the problem, but I don't like the patch.
For example, we don't need to match the container's name again, we can use the fact that we've already matched the container scan handler to it.
Does the one below work too?
Hi Rafael,
Thank you for posting your patch. I confirmed that your patch fixed my issue as folloes:
# udevadm monitor --kernel
monitor will print the received events for:
KERNEL - the kernel uevent
KERNEL[...] add /devices/system/container/ACPI0004:01 (container)
<snip>
KERNEL[...] add /devices/system/memory/memory2048 (memory)
<snip>
KERNEL[...] add /devices/system/memory/memory2063 (memory)
<snip>
KERNEL[...] add /devices/system/cpu/cpu60 (cpu)
<snip>
KERNEL[...] add /devices/system/cpu/cpu119 (cpu)
KERNEL[...] online /devices/system/container/ACPI0004:01 (container)
So feel free to add "Tested-by : Yasuaki Ishimatsu <[email protected]>" to the patch.

Quoted 6 lines

Quoted 2 lines
Thanks!
I just tested your new patch, it works on my side as Yasuaki's patch.
Tested-by Lee, Chun-Yi <[email protected]>

Thanks!

Yasuaki Ishimatsu

author

2014-09-19 05:55 UTC #9 lore raw

Hi Rafael,

Quoted 9 lines

Quoted 9 lines
Quoted 5 lines
Quoted 2 lines
So feel free to add "Tested-by : Yasuaki Ishimatsu <[email protected]>" to the patch.
Thanks!
I just tested your new patch, it works on my side as Yasuaki's patch.
Tested-by Lee, Chun-Yi <[email protected]>

Hi Rafael,

When will you merge the patch into your tree?
I would like you to do it soon.

Thanks,
Yasuaki Ishimatsu

Quoted 8 lines

Thanks!
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Rafael J. Wysocki

2014-09-20 23:41 UTC #10 lore raw

Quoted 8 lines · Yasuaki Ishimatsu

Hi Rafael,
Quoted 6 lines
Quoted 6 lines
Quoted 5 lines
Quoted 2 lines
So feel free to add "Tested-by : Yasuaki Ishimatsu <[email protected]>" to the patch.
Thanks!

Quoted 8 lines

Quoted 3 lines
Quoted 3 lines
I just tested your new patch, it works on my side as Yasuaki's patch.
Tested-by Lee, Chun-Yi <[email protected]>
Hi Rafael,
When will you merge the patch into your tree?
I would like you to do it soon.

I'm going to push it for 3.17-rc7 next week.

Rafael

Yasuaki Ishimatsu

author

2014-09-25 02:24 UTC #11 lore raw

Quoted 11 lines · Rafael J. Wysocki

Quoted 10 lines · Yasuaki Ishimatsu
Hi Rafael,
Quoted 8 lines
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
So feel free to add "Tested-by : Yasuaki Ishimatsu <[email protected]>" to the patch.
Thanks!
I just tested your new patch, it works on my side as Yasuaki's patch.

Quoted 8 lines

Quoted 6 lines
Quoted 1 line
Quoted 1 line
Tested-by Lee, Chun-Yi <[email protected]>
Hi Rafael,
When will you merge the patch into your tree?
I would like you to do it soon.
I'm going to push it for 3.17-rc7 next week.

Thank you for your cooperation.

I confirmed that the patch is merged into bleeding-dege now.

Thanks,
Yasuaki Ishimatsu

Quoted 3 lines

Rafael