Re: memory hotplug and force_remove

On Thu, Mar 30, 2017 at 10:15:04PM +0200, Rafael J. Wysocki wrote:

On Friday, March 31, 2017 12:57:29 AM joeyli wrote:

quoted

On Thu, Mar 30, 2017 at 06:20:34PM +0200, Michal Hocko wrote:

quoted

On Thu 30-03-17 10:47:52, Jiri Kosina wrote:

quoted

On Tue, 28 Mar 2017, Rafael J. Wysocki wrote:

quoted

quoted

quoted

quoted

we have been chasing the following BUG() triggering during the memory
hotremove (remove_memory):
	ret = walk_memory_range(PFN_DOWN(start), PFN_UP(start + size - 1), NULL,
				check_memblock_offlined_cb);
	if (ret)
		BUG();

and it took a while to learn that the issue is caused by
/sys/firmware/acpi/hotplug/force_remove being enabled. I was really
surprised to see such an option because at least for the memory hotplug
it cannot work at all. Memory hotplug fails when the memory is still
in use. Even if we do not BUG() here enforcing the hotplug operation
will lead to problematic behavior later like crash or a silent memory
corruption if the memory gets onlined back and reused by somebody else.

I am wondering what was the motivation for introducing this behavior and
whether there is a way to disallow it for memory hotplug. Or maybe drop
it completely. What would break in such a case?

Honestly, I don't remember from the top of my head and I haven't looked at
that code for several months.

I need some time to recall that.

Did you have any chance to look into this?

Well, yes.

It looks like that was added for some people who depended on the old behavior
at that time.

I guess we can try to drop it and see what happpens. :-)

I'd agree with that; at the same time, udev rule should be submitted to 
systemd folks though. I don't think there is anything existing in this 
area yet (neither do distros ship their own udev rules for this AFAIK).

Another option would keepint the force_remove knob but make the code be
error handling aware. In other words rather than ignoring offline error
simply propagate it up the chain and do not consider the offline. Would
that be acceptable?

Then the only difference between normal mode is that the force_remove mode
doesn't send out uevent for not-offline-yet container.

Which would be rather confusing.

The whole point of the thing was the "remove no matter what" behavior and
there's not much point in keeping it around without that.

OK~ Understood.

Then back the "remove no matter waht" behavior, the point is
force_remove knob causes that acpi_scan_try_to_offline() ignored
the offline error of parent/children devices in the second pass:

drivers/acpi/scan.c
static int acpi_scan_try_to_offline(struct acpi_device *device)
{  
...
	/* first pass to call bus offline for parent */
	acpi_bus_offline(handle, 0, (void *)false, (void **)&errdev);
	/* if failed, then second pass */
	if (errdev) { 
		errdev = NULL;
		/* children devices, second pass */
		acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX,
				    NULL, acpi_bus_offline, (void *)true,
				    (void **)&errdev);
		/* ignored children's offline error here */
		if (!errdev || acpi_force_hot_remove)
			/* ignored parent's offline error */
			acpi_bus_offline(handle, 0, (void *)true,
					 (void **)&errdev);

		/* will not set devices back to online */
		if (errdev && !acpi_force_hot_remove) { 
	...
	}
	return 0;
}

Then acpi_scan_try_to_offline() returns 0 and go to _remove_ stage, then
memory subsystem raises BUG() because the device offline state doesn't sync
with memory block state.


Thanks a lot!
Joey Lee

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>