[PATCH] acpi : Add container online uevent to acpi_bus_attach

[PATCH] acpi : Add container online uevent to acpi_bus_attach

[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)

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 <isimatu.yasuaki@jp.fujitsu.com>
---
 drivers/acpi/container.c | 15 +++++++++++++++
 drivers/acpi/internal.h  |  7 +++++++
 drivers/acpi/scan.c      |  3 +++
 3 files changed, 25 insertions(+)

diff --git a/drivers/acpi/container.c b/drivers/acpi/container.c
index 76f7cff..eba5746 100644
--- a/drivers/acpi/container.c
+++ b/drivers/acpi/container.c
@@ -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);
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index 4c5cf77..12dd89b 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -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
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 9a92989..faa3d11 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -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

Re: [PATCH] acpi : Add container online uevent to acpi_bus_attach

[joeyli]

Hi Yasuaki, 

On Fri, Sep 05, 2014 at 03:57:43PM +0900, Yasuaki Ishimatsu wrote:
> 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)
> 
> 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 <isimatu.yasuaki@jp.fujitsu.com>

Your patch works on my side.

Tested-by: Lee, Chun-Yi <jlee@suse.com>

Thanks a lot!
joey Lee

> ---
>  drivers/acpi/container.c | 15 +++++++++++++++
>  drivers/acpi/internal.h  |  7 +++++++
>  drivers/acpi/scan.c      |  3 +++
>  3 files changed, 25 insertions(+)
> 
> diff --git a/drivers/acpi/container.c b/drivers/acpi/container.c
> index 76f7cff..eba5746 100644
> --- a/drivers/acpi/container.c
> +++ b/drivers/acpi/container.c
> @@ -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);
> diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
> index 4c5cf77..12dd89b 100644
> --- a/drivers/acpi/internal.h
> +++ b/drivers/acpi/internal.h
> @@ -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
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 9a92989..faa3d11 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] acpi : Add container online uevent to acpi_bus_attach

[Rafael J. Wysocki]

On Friday, September 05, 2014 03:57:43 PM Yasuaki Ishimatsu wrote:
> 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)
> 
> 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(+)

Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ 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;
 };
Index: linux-pm/drivers/acpi/container.c
===================================================================
--- linux-pm.orig/drivers/acpi/container.c
+++ 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,
 	},
 };
 
Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ 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);
 }
 
 /**

Re: [PATCH] acpi : Add container online uevent to acpi_bus_attach

[Rafael J. Wysocki]

On Friday, September 05, 2014 08:14:13 PM joeyli wrote:
> Hi Yasuaki, 
> 
> On Fri, Sep 05, 2014 at 03:57:43PM +0900, Yasuaki Ishimatsu wrote:
> > 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)
> > 
> > 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 <isimatu.yasuaki@jp.fujitsu.com>
> 
> Your patch works on my side.
> 
> Tested-by: Lee, Chun-Yi <jlee@suse.com>

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

Re: [PATCH] acpi : Add container online uevent to acpi_bus_attach

[Yasuaki Ishimatsu]

(2014/09/08 6:34), Rafael J. Wysocki wrote:
> On Friday, September 05, 2014 03:57:43 PM Yasuaki Ishimatsu wrote:
>> 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)
>>
>> 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?

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 <isimatu.yasuaki@jp.fujitsu.com>"
to the patch.

Thanks,
Yasuaki Ishimatsu

>
> 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
> ===================================================================
> --- linux-pm.orig/include/acpi/acpi_bus.h
> +++ 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;
>   };
> Index: linux-pm/drivers/acpi/container.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/container.c
> +++ 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,
>   	},
>   };
>
> Index: linux-pm/drivers/acpi/scan.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/scan.c
> +++ 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);
>   }
>
>   /**
>

Re: [PATCH] acpi : Add container online uevent to acpi_bus_attach

[Yasuaki Ishimatsu]

(2014/09/05 21:14), joeyli wrote:
> Hi Yasuaki,
>
> On Fri, Sep 05, 2014 at 03:57:43PM +0900, Yasuaki Ishimatsu wrote:
>> 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)
>>
>> 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 <isimatu.yasuaki@jp.fujitsu.com>
>
> Your patch works on my side.
>
> Tested-by: Lee, Chun-Yi <jlee@suse.com>

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

>
> Thanks a lot!
> joey Lee
>
>> ---
>>   drivers/acpi/container.c | 15 +++++++++++++++
>>   drivers/acpi/internal.h  |  7 +++++++
>>   drivers/acpi/scan.c      |  3 +++
>>   3 files changed, 25 insertions(+)
>>
>> diff --git a/drivers/acpi/container.c b/drivers/acpi/container.c
>> index 76f7cff..eba5746 100644
>> --- a/drivers/acpi/container.c
>> +++ b/drivers/acpi/container.c
>> @@ -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);
>> diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
>> index 4c5cf77..12dd89b 100644
>> --- a/drivers/acpi/internal.h
>> +++ b/drivers/acpi/internal.h
>> @@ -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
>> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
>> index 9a92989..faa3d11 100644
>> --- a/drivers/acpi/scan.c
>> +++ b/drivers/acpi/scan.c
>> @@ -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 majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] acpi : Add container online uevent to acpi_bus_attach

[Rafael J. Wysocki]

On Wednesday, September 10, 2014 12:16:43 PM Yasuaki Ishimatsu wrote:
> (2014/09/08 6:34), Rafael J. Wysocki wrote:
> > On Friday, September 05, 2014 03:57:43 PM Yasuaki Ishimatsu wrote:
> >> 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)
> >>
> >> 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?
> 
> 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 <isimatu.yasuaki@jp.fujitsu.com>"
> to the patch.

Thanks!

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

Re: [PATCH] acpi : Add container online uevent to acpi_bus_attach

[joeyli]

Hi Rafael, 

On Thu, Sep 11, 2014 at 01:38:37AM +0200, Rafael J. Wysocki wrote:
> On Wednesday, September 10, 2014 12:16:43 PM Yasuaki Ishimatsu wrote:
> > (2014/09/08 6:34), Rafael J. Wysocki wrote:
> > > On Friday, September 05, 2014 03:57:43 PM Yasuaki Ishimatsu wrote:
> > >> 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)
> > >>
> > >> 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?
> > 
> > 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 <isimatu.yasuaki@jp.fujitsu.com>"
> > to the patch.
> 
> Thanks!
>

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

Tested-by: Lee, Chun-Yi <jlee@suse.com>


Thanks a lot!
Joey Lee 

Re: [PATCH] acpi : Add container online uevent to acpi_bus_attach

[Rafael J. Wysocki]

On Thursday, September 11, 2014 03:17:17 PM joeyli wrote:
> Hi Rafael, 
> 
> On Thu, Sep 11, 2014 at 01:38:37AM +0200, Rafael J. Wysocki wrote:
> > On Wednesday, September 10, 2014 12:16:43 PM Yasuaki Ishimatsu wrote:
> > > (2014/09/08 6:34), Rafael J. Wysocki wrote:
> > > > On Friday, September 05, 2014 03:57:43 PM Yasuaki Ishimatsu wrote:
> > > >> 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)
> > > >>
> > > >> 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?
> > > 
> > > 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 <isimatu.yasuaki@jp.fujitsu.com>"
> > > to the patch.
> > 
> > Thanks!
> >
> 
> I just tested your new patch, it works on my side as Yasuaki's patch. 
> 
> Tested-by: Lee, Chun-Yi <jlee@suse.com>

Thanks!

Re: [PATCH] acpi : Add container online uevent to acpi_bus_attach

[Yasuaki Ishimatsu]

Hi Rafael,

>>>> So feel free to  add "Tested-by : Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>"
>>>> to the patch.
>>>
>>> Thanks!
>>>
>>
>> I just tested your new patch, it works on my side as Yasuaki's patch.
>>
>> Tested-by: Lee, Chun-Yi <jlee@suse.com>

Hi Rafael,

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

Thanks,
Yasuaki Ishimatsu

>
> Thanks!
>
> --
> 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] acpi : Add container online uevent to acpi_bus_attach

[Rafael J. Wysocki]

On Friday, September 19, 2014 02:53:59 PM Yasuaki Ishimatsu wrote:
> Hi Rafael,
> 
> >>>> So feel free to  add "Tested-by : Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>"
> >>>> to the patch.
> >>>
> >>> Thanks!
> >>>
> >>
> >> I just tested your new patch, it works on my side as Yasuaki's patch.
> >>
> >> Tested-by: Lee, Chun-Yi <jlee@suse.com>
> 
> 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

Re: [PATCH] acpi : Add container online uevent to acpi_bus_attach

[Yasuaki Ishimatsu]

(2014/09/21 9:01), Rafael J. Wysocki wrote:
> On Friday, September 19, 2014 02:53:59 PM Yasuaki Ishimatsu wrote:
>> Hi Rafael,
>>
>>>>>> So feel free to  add "Tested-by : Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>"
>>>>>> to the patch.
>>>>>
>>>>> Thanks!
>>>>>
>>>>
>>>> I just tested your new patch, it works on my side as Yasuaki's patch.
>>>>
>>>> Tested-by: Lee, Chun-Yi <jlee@suse.com>
>>
>> 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

>
> Rafael
>