Messages in this thread | | | Date | Tue, 9 Dec 2014 19:25:50 +0900 | From | Yasuaki Ishimatsu <> | Subject | Re: [PATCH 2/2] Drivers: hv: balloon: Fix the deadlock issue in the memory hot-add code |
| |
(2014/12/09 18:08), Michal Hocko wrote: > On Tue 09-12-14 10:23:51, Yasuaki Ishimatsu wrote: >> (2014/12/09 0:04), Michal Hocko wrote: >>> On Fri 05-12-14 16:41:38, K. Y. Srinivasan wrote: >>>> Andy Whitcroft <apw@canonical.com> initially saw this deadlock. We >>>> have seen this as well. Here is the original description of the >>>> problem (and a potential solution) from Andy: >>>> >>>> https://lkml.org/lkml/2014/3/14/451 >>>> >>>> Here is an excerpt from that mail: >>>> >>>> "We are seeing machines lockup with what appears to be an ABBA >>>> deadlock in the memory hotplug system. These are from the 3.13.6 based Ubuntu kernels. >>>> The hv_balloon driver is adding memory using add_memory() which takes >>>> the hotplug lock >>> >>> Do you mean mem_hotplug_begin? >>> >> >>>> and then emits a udev event, and then attempts to >>>> lock the sysfs device. In response to the udev event udev opens the >>>> sysfs device and locks it, then attempts to grab the hotplug lock to online the memory. >>> >>> Cannot we simply teach online_pages to fail with EBUSY when the memory >>> hotplug is on the way. We shouldn't try to online something that is not >>> initialized yet, no? >> >> Yes. Memory online shouldn't try before initializing it. Then memory online >> should wait for initializing it, not easily fails. Generally, kernel sends >> memory ONLINE event to userland by kobject_uevent() during initializing memory >> and udev makes memory online after catching the event. Onlining memory by >> udev almost run during initializing memory. > > I guess this is because the event is sent after a mem section is > initialized while the overal hotplug operation is still not completed.
right.
> >> So if memory online easily fails, onlining memory by udev almost >> fails. > > Doesn't udev retry the operation if it gets EBUSY or EAGAIN?
It depend on implementation of udev.rules. So we can retry online/offline operation in udev.rules.
> >>> The memory hotplug log is global so we can get >>> false positives but that should be easier to deal with than exporting >>> lock_device_hotplug and adding yet another lock dependency. >>> >>>> This seems to be inverted nesting in the two cases, leading to the hangs below: >>>> >>>> [ 240.608612] INFO: task kworker/0:2:861 blocked for more than 120 seconds. >>>> [ 240.608705] INFO: task systemd-udevd:1906 blocked for more than 120 seconds. >>>> >>>> I note that the device hotplug locking allows complete retries (via >>>> ERESTARTSYS) and if we could detect this at the online stage it could >>>> be used to get us out. >>> >>> I am not sure I understand this but it suggests EBUSY above? >>> >>>> But before I go down this road I wanted to >>>> make sure I am reading this right. Or indeed if the hv_balloon driver >>>> is just doing this wrong." >>>> >>>> This patch is based on the suggestion from >>>> Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com> >>> >>> This changelog doesn't explain us much. And boy this whole thing is so >>> convoluted. E.g. I have hard time to see why ACPI hotplug is working >>> correctly. My trail got lost at acpi_memory_device_add level which is >>> a callback while acpi_device_hotplug is holding lock_device_hotplug but >>> then again the rest is hidden by callbacks. >> >> Commit 0f1cfe9d0d06 (mm/hotplug: remove stop_machine() from try_offline_node()) said: >> >> --- >> lock_device_hotplug() serializes hotplug & online/offline operations. The >> lock is held in common sysfs online/offline interfaces and ACPI hotplug >> code paths. >> >> And here are the code paths: >> >> - CPU & Mem online/offline via sysfs online >> store_online()->lock_device_hotplug() >> >> - Mem online via sysfs state: >> store_mem_state()->lock_device_hotplug() >> >> - ACPI CPU & Mem hot-add: >> acpi_scan_bus_device_check()->lock_device_hotplug() >> >> - ACPI CPU & Mem hot-delete: >> acpi_scan_hot_remove()->lock_device_hotplug() >> --- >> >> CPU & Memory online/offline/hotplug are serialized by lock_device_hotplug(). > > OK, this patch aimed at the complete nodes hotplug. I am not familiar > with the code enough to tell whether this is really needed but it sounds > like an overkill when we are interested only in the memory hotplug. Why > would we need stop_machine or anything for memory that is guaranteed to > be not used at the time of both online and offline.
As Srinivasan mentioned, there is ABBA issue. So to evade it, lock_device_hotplug() is needed.
> And again, why cannot we simply make the onlining fail or try_lock and > retry internally if the event consumer cannot cope with errors?
Did you mean the following Srinivasan's first patch looks good to you? https://lkml.org/lkml/2014/12/2/662
Thanks, Yasuaki Ishimatsu
> And even if that is not possible then do not export lock_device_hotplug > but export a memory hotplug functions which use it properly so that > other consumers (xen ballon seem to rely on add_memory as well) do not > need the same change as well.
> >>> I cannot seem to find any documentation which would explain all the >>> locking here. >> >> Yes. I need the documentation. > > Yes please! This code is incredibly hard to follow and deduce all the > hidden requirements and dependencies is even harder. >
| |