acpi_bus_trim does not detach devices in post order

I acked the following commit but I hit a problem by the commit. So I report it.

commit cecdb193c8d91a42d9489d00618cc3dfff92e55a

Author: Rafael J. Wysocki <[email protected]>
Date:   Tue Jan 15 13:24:02 2013 +0100

ACPI / scan: Change the implementation of acpi_bus_trim()

Before applying the commit, acpi_bus_trim() detachs devices in post order.

When I hot add memory devices and processor devices by container device in my x86 box, memory devices are added first and processor devices are added second. So I expect that processor devices are removed first and memory devices are removed second when I remove them. And before applying the commit, acpi_bus_trim() did so.

But after appling the commit, acpi_bus_trim() does not detach devices in post order. So when I remove them, memory devices are removed first and processor devices are removed second.

By this, I hit a problem.

In Linux on x86 arch, NUMA node is depend on memory devices. So new NUMA node is created at memory hot adding. Thus when I hot add memory devices and processor devices, we must hot add memory device first. Otherwise, processor devices are not set to correct NUMA node number.

And Linux expects that when removing them, processor devices are removed first before removing memory devices. But acpi_bus_trim() does not do so. By this, NUMA node is not cleared in my x86 box when hot removing memory device and processor devices. When removing memory devices, NUMA node is cleared. But if there are processor devices related with the NUMA node, NUMA node is not be cleared at memory hot removing.

So when I remove them, NUMA node's sysfs file remained as follows:

# ls /sys/devices/system/node/node1/
compact  cpumap    meminfo   power                   subsystem  vmstat
cpulist  distance  numastat  scan_unevictable_pages  uevent

CPU and memory sysfs files are removed correctly. But node1 sysfs file remained.

Thanks,
Yasuaki Ishimatsu

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

Quoted 36 lines · Yasuaki Ishimatsu

I acked the following commit but I hit a problem by the commit. So I report it.

commit cecdb193c8d91a42d9489d00618cc3dfff92e55a

Author: Rafael J. Wysocki <[email protected]>
Date:   Tue Jan 15 13:24:02 2013 +0100

ACPI / scan: Change the implementation of acpi_bus_trim()

Before applying the commit, acpi_bus_trim() detachs devices in post order.

When I hot add memory devices and processor devices by container device in my x86 box, memory devices are added first and processor devices are added second. So I expect that processor devices are removed first and memory devices are removed second when I remove them. And before applying the commit, acpi_bus_trim() did so.

But after appling the commit, acpi_bus_trim() does not detach devices in post order. So when I remove them, memory devices are removed first and processor devices are removed second.

By this, I hit a problem.

In Linux on x86 arch, NUMA node is depend on memory devices. So new NUMA node is created at memory hot adding. Thus when I hot add memory devices and processor devices, we must hot add memory device first. Otherwise, processor devices are not set to correct NUMA node number.

And Linux expects that when removing them, processor devices are removed first before removing memory devices. But acpi_bus_trim() does not do so. By this, NUMA node is not cleared in my x86 box when hot removing memory device and processor devices. When removing memory devices, NUMA node is cleared. But if there are processor devices related with the NUMA node, NUMA node is not be cleared at memory hot removing.

Quoted 1 line

So when I remove them, NUMA node's sysfs file remained as follows:

I had little mistake. CPU also tries to clear NUMA node. But current implementation has bug. So I'll fix it.

Quoted 37 lines · Yasuaki Ishimatsu

Quoted 36 lines · Yasuaki Ishimatsu

I acked the following commit but I hit a problem by the commit. So I report it.

commit cecdb193c8d91a42d9489d00618cc3dfff92e55a

Author: Rafael J. Wysocki <[email protected]>
Date:   Tue Jan 15 13:24:02 2013 +0100

ACPI / scan: Change the implementation of acpi_bus_trim()

Before applying the commit, acpi_bus_trim() detachs devices in post order.

When I hot add memory devices and processor devices by container device in my x86 box, memory devices are added first and processor devices are added second. So I expect that processor devices are removed first and memory devices are removed second when I remove them. And before applying the commit, acpi_bus_trim() did so.

But after appling the commit, acpi_bus_trim() does not detach devices in post order. So when I remove them, memory devices are removed first and processor devices are removed second.

By this, I hit a problem.

In Linux on x86 arch, NUMA node is depend on memory devices. So new NUMA node is created at memory hot adding. Thus when I hot add memory devices and processor devices, we must hot add memory device first. Otherwise, processor devices are not set to correct NUMA node number.

And Linux expects that when removing them, processor devices are removed first before removing memory devices. But acpi_bus_trim() does not do so. By this, NUMA node is not cleared in my x86 box when hot removing memory device and processor devices. When removing memory devices, NUMA node is cleared. But if there are processor devices related with the NUMA node, NUMA node is not be cleared at memory hot removing.

Quoted 4 lines

Quoted 1 line

So when I remove them, NUMA node's sysfs file remained as follows:

I had little mistake. CPU also tries to clear NUMA node. But current implementation has bug. So I'll fix it.

Do I understand correctly that with your fix at

https://patchwork.kernel.org/patch/2839298/

the current acpi_bus_trim() implementation will be sufficient?

Rafael

Quoted 43 lines · Rafael J. Wysocki

Quoted 41 lines · Yasuaki Ishimatsu

Quoted 29 lines · Yasuaki Ishimatsu

I acked the following commit but I hit a problem by the commit. So I report it.

commit cecdb193c8d91a42d9489d00618cc3dfff92e55a

Author: Rafael J. Wysocki <[email protected]>
Date:   Tue Jan 15 13:24:02 2013 +0100

ACPI / scan: Change the implementation of acpi_bus_trim()

Before applying the commit, acpi_bus_trim() detachs devices in post order.

When I hot add memory devices and processor devices by container device in my x86 box, memory devices are added first and processor devices are added second. So I expect that processor devices are removed first and memory devices are removed second when I remove them. And before applying the commit, acpi_bus_trim() did so.

But after appling the commit, acpi_bus_trim() does not detach devices in post order. So when I remove them, memory devices are removed first and processor devices are removed second.

By this, I hit a problem.

In Linux on x86 arch, NUMA node is depend on memory devices. So new NUMA node is created at memory hot adding. Thus when I hot add memory devices and processor devices, we must hot add memory device first. Otherwise, processor devices are not set to correct NUMA node number.

Quoted 8 lines

And Linux expects that when removing them, processor devices are removed first before removing memory devices. But acpi_bus_trim() does not do so. By this, NUMA node is not cleared in my x86 box when hot removing memory device and processor devices. When removing memory devices, NUMA node is cleared. But if there are processor devices related with the NUMA node, NUMA node is not be cleared at memory hot removing.

So when I remove them, NUMA node's sysfs file remained as follows:

I had little mistake. CPU also tries to clear NUMA node. But current implementation has bug. So I'll fix it.

Quoted 5 lines

Do I understand correctly that with your fix at

https://patchwork.kernel.org/patch/2839298/

the current acpi_bus_trim() implementation will be sufficient?

No. The patch just fixed implementation of CPU hotplug.

A problem I think is that acpi_bus_trim() does not detach devices in post-order.

And my patch does not fix it. So if some device has dependency of other device, similar problem will occur.

Quoted 51 lines · Yasuaki Ishimatsu

Quoted 49 lines · Rafael J. Wysocki

Quoted 37 lines · Yasuaki Ishimatsu

Quoted 36 lines · Yasuaki Ishimatsu

I acked the following commit but I hit a problem by the commit. So I report it.

commit cecdb193c8d91a42d9489d00618cc3dfff92e55a

Author: Rafael J. Wysocki <[email protected]>
Date:   Tue Jan 15 13:24:02 2013 +0100

ACPI / scan: Change the implementation of acpi_bus_trim()

Before applying the commit, acpi_bus_trim() detachs devices in post order.

When I hot add memory devices and processor devices by container device in my x86 box, memory devices are added first and processor devices are added second. So I expect that processor devices are removed first and memory devices are removed second when I remove them. And before applying the commit, acpi_bus_trim() did so.

But after appling the commit, acpi_bus_trim() does not detach devices in post order. So when I remove them, memory devices are removed first and processor devices are removed second.

By this, I hit a problem.

In Linux on x86 arch, NUMA node is depend on memory devices. So new NUMA node is created at memory hot adding. Thus when I hot add memory devices and processor devices, we must hot add memory device first. Otherwise, processor devices are not set to correct NUMA node number.

And Linux expects that when removing them, processor devices are removed first before removing memory devices. But acpi_bus_trim() does not do so. By this, NUMA node is not cleared in my x86 box when hot removing memory device and processor devices. When removing memory devices, NUMA node is cleared. But if there are processor devices related with the NUMA node, NUMA node is not be cleared at memory hot removing.

Quoted 4 lines

Quoted 1 line

So when I remove them, NUMA node's sysfs file remained as follows:

I had little mistake. CPU also tries to clear NUMA node. But current implementation has bug. So I'll fix it.

Do I understand correctly that with your fix at

https://patchwork.kernel.org/patch/2839298/

the current acpi_bus_trim() implementation will be sufficient?

Quoted 1 line

No. The patch just fixed implementation of CPU hotplug.

My question was not sufficiently precise. :-)

I wanted to ask if your patch was sufficient to address the specific breakage you were seeing without modifying acpi_bus_trim().

Quoted 3 lines

A problem I think is that acpi_bus_trim() does not detach devices in post-order.

That is not exactly post-order, but children are guaranteed to be processed before their parents. If that guarantee is sufficient, there's no problem. Otherwise, acpi_bus_trim() may need to be modified, but first I'd like to see a real life example where that really matters.

Quoted 2 lines

And my patch does not fix it. So if some device has dependency of other device, similar problem will occur.

If there is a dependency that is not a parent-child one, we'll have a problem, but in that case relying on ordering will not be robust enough anyway in my opinion.

Thanks,
Rafael