2017-03-22 01:01:57

by Chun-Yi Lee

Subject: [PATCH] acpi: check the online state of all children in container

Just checking the state of container is not enough to confirm that
the whole container is offlined. Kernel should checks all children's
offline state as the logic in acpi_container_offline().

Cc: "Rafael J. Wysocki" <[email protected]>
Cc: Len Brown <[email protected]>
Cc: Michal Hocko <[email protected]>
Cc: Jiri Kosina <[email protected]>
Signed-off-by: "Lee, Chun-Yi" <[email protected]>
---
drivers/acpi/scan.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 1926918..f08ca31 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -260,13 +260,15 @@ static int acpi_scan_try_to_offline(struct acpi_device *device)
static int acpi_scan_hot_remove(struct acpi_device *device)
{
acpi_handle handle = device->handle;
+ struct acpi_device *child;
unsigned long long sta;
acpi_status status;

if (device->handler && device->handler->hotplug.demand_offline
&& !acpi_force_hot_remove) {
- if (!acpi_scan_is_offline(device, true))
- return -EBUSY;
+ list_for_each_entry(child, &device->children, node)
+ if (!acpi_scan_is_offline(child, false))
+ return -EBUSY;
} else {
int error = acpi_scan_try_to_offline(device);
if (error)
--
2.10.2

2017-03-22 01:06:12

by Rafael J. Wysocki

Subject: Re: [PATCH] acpi: check the online state of all children in container

On Wednesday, March 22, 2017 09:01:48 AM Lee, Chun-Yi wrote:
> Just checking the state of container is not enough to confirm that
> the whole container is offlined.

And why is that so?

> Kernel should checks all children's
> offline state as the logic in acpi_container_offline().
>
> Cc: "Rafael J. Wysocki" <[email protected]>
> Cc: Len Brown <[email protected]>
> Cc: Michal Hocko <[email protected]>
> Cc: Jiri Kosina <[email protected]>
> Signed-off-by: "Lee, Chun-Yi" <[email protected]>
> ---
> drivers/acpi/scan.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 1926918..f08ca31 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -260,13 +260,15 @@ static int acpi_scan_try_to_offline(struct acpi_device *device)
> static int acpi_scan_hot_remove(struct acpi_device *device)
> {
> acpi_handle handle = device->handle;
> + struct acpi_device *child;
> unsigned long long sta;
> acpi_status status;
>
> if (device->handler && device->handler->hotplug.demand_offline
> && !acpi_force_hot_remove) {
> - if (!acpi_scan_is_offline(device, true))
> - return -EBUSY;
> + list_for_each_entry(child, &device->children, node)
> + if (!acpi_scan_is_offline(child, false))
> + return -EBUSY;
> } else {
> int error = acpi_scan_try_to_offline(device);
> if (error)
>

2017-03-22 03:22:50

by joeyli

Subject: Re: [PATCH] acpi: check the online state of all children in container

On Wed, Mar 22, 2017 at 01:58:30AM +0100, Rafael J. Wysocki wrote:
> On Wednesday, March 22, 2017 09:01:48 AM Lee, Chun-Yi wrote:
> > Just checking the state of container is not enough to confirm that
> > the whole container is offlined.
>
> And why is that so?
>

Actually there does not have real kernel issue triggered by this code now.
I reviewed code and found the difference between acpi_container_offline().

Considering a container that it includes devices and sub-containers
like this:

Scope (_SB)
Device (MODU)
Name (_HID, "ACPI0004") <=== main-container
Device (PCIE)
Name (_HID, EisaId ("PNP0A08"))
Device (SUBM)
Name (_HID, "ACPI0004") <=== sub-container
Device (MEM0)
Name (_HID, EisaId ("PNP0C80"))
...

The original code checks the physical nodes on the main container but
doesn't check children's physical nodes. So, it may happen the sub-container
didn't offline but the offline checking of main container is pass.

Please kindly direct me if I misunderstood or missed any detail in the codes
about physcial node and container offline.

Thank a lot!
Joey Lee