Messages in this thread | | | From | Dan Williams <> | Date | Fri, 24 Jan 2020 10:40:46 -0800 | Subject | Re: [PATCH v1] driver core: check for dead devices before onlining/offlining |
| |
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?
| |