Messages in this thread | | | Subject | Re: [PATCH v1] driver core: check for dead devices before onlining/offlining | From | David Hildenbrand <> | Date | Fri, 24 Jan 2020 14:48:19 +0100 |
| |
On 24.01.20 14:31, David Hildenbrand wrote: > On 24.01.20 10:12, Greg Kroah-Hartman wrote: >> On Fri, Jan 24, 2020 at 10:09:03AM +0100, David Hildenbrand wrote: >>> On 24.01.20 10:00, Greg Kroah-Hartman wrote: >>>> On Mon, Jan 20, 2020 at 11:49:09AM +0100, 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. >>>>> >>>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >>>>> Cc: "Rafael J. Wysocki" <rafael@kernel.org> >>>>> Cc: Suzuki K Poulose <suzuki.poulose@arm.com> >>>>> Cc: Saravana Kannan <saravanak@google.com> >>>>> Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com> >>>>> Cc: Dan Williams <dan.j.williams@intel.com> >>>>> Cc: Michal Hocko <mhocko@kernel.org> >>>>> Signed-off-by: David Hildenbrand <david@redhat.com> >>>>> --- >>>>> >>>>> Am I missing any obvious mechanism in the device core that handles >>>>> something like this already? (especially also for other sysfs attributes?) >>>> >>>> So is a sysfs attribute causing the device itself to go away? We have >>> >>> nope, removal is triggered via the driver, not via a sysfs attribute. >> >> But the idea is the same, it comes from the driver, not the driver core. >> >>> Regarding this patch: Is there anything prohibiting the possible >>> scenario I document above (IOW, is this patch applicable, or is there >>> another way to fence it properly (e.g., the "specific call" you mentioned))? >> >> I think it's the same thing, look at how scsi does it. > > I think you are talking about doing a "transport_remove_device(dev)" > before doing the "device_del(dev)", combined with proper locking. > > Will look into that for the memory subsystem ...
... looking into transports, it most probably does not apply here, hmm ...
-- Thanks,
David / dhildenb
| |