Messages in this thread | | | From | David Hildenbrand <> | Subject | Re: [PATCH v1] driver core: check for dead devices before onlining/offlining | Date | Fri, 24 Jan 2020 19:58:03 +0100 |
| |
> Am 24.01.2020 um 19:41 schrieb Dan Williams <dan.j.williams@intel.com>: > > On Fri, Jan 24, 2020 at 9:14 AM David Hildenbrand <david@redhat.com> wrote: >> >>> On 20.01.20 11:49, David Hildenbrand wrote: >>> We can have rare cases where the removal of a device races with >>> somebody trying to online it (esp. via sysfs). We can simply check >>> if the device is already removed or getting removed under the dev->lock. >>> >>> E.g., right now, if memory block devices are removed (remove_memory()), >>> we do a: >>> >>> remove_memory() -> lock_device_hotplug() -> mem_hotplug_begin() -> >>> lock_device() -> dev->dead = true >>> >>> Somebody coming via sysfs (/sys/devices/system/memory/memoryX/online) >>> triggers a: >>> >>> lock_device_hotplug_sysfs() -> device_online() -> lock_device() ... >>> >>> So if we made it just before the lock_device_hotplug_sysfs() but get >>> delayed until remove_memory() released all locks, we will continue >>> taking locks and trying to online the device - which is then a zombie >>> device. >>> >>> Note that at least the memory onlining path seems to be protected by >>> checking if all memory sections are still present (something we can then >>> get rid of). We do have other sysfs attributes >>> (e.g., /sys/devices/system/memory/memoryX/valid_zones) that don't do any >>> such locking yet and might race with memory removal in a similar way. For >>> these users, we can then do a >>> >>> device_lock(dev); >>> if (!device_is_dead(dev)) { >>> /* magic /* >>> } >>> device_unlock(dev); >>> >>> Introduce and use device_is_dead() right away. >>> >> >> So, I just added the following: >> >> diff --git a/drivers/base/core.c b/drivers/base/core.c >> index 01cd06eeb513..49c4d8671073 100644 >> --- a/drivers/base/core.c >> +++ b/drivers/base/core.c >> @@ -1567,6 +1567,7 @@ static ssize_t online_store(struct device *dev, >> struct device_attribute *attr, >> if (ret < 0) >> return ret; >> >> + msleep(10000); >> ret = lock_device_hotplug_sysfs(); >> if (ret) >> return ret; >> >> Then triggered >> echo 1 > /sys/devices/system/memory/memory51/online >> And quickly afterwards unplugged the DIMM. >> >> Good news is that we get (after 10 seconds) >> sh: echo: write error: No such device >> >> Reason is that unplug will not finish before all sysfs attributes have >> been exited by other threads. > > The unplug thread gets blocked for 10 seconds waiting for this thread > in online_store() to exit? >
Yes, that‘s what I observed.
| |