Re: [PATCH v1] driver core: check for dead devices before onlining/offlining

[David Hildenbrand]

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@xxxxxxxxxxxxxxxxxxx>
>> Cc: "Rafael J. Wysocki" <rafael@xxxxxxxxxx>
>> Cc: Suzuki K Poulose <suzuki.poulose@xxxxxxx>
>> Cc: Saravana Kannan <saravanak@xxxxxxxxxx>
>> Cc: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
>> Cc: Dan Williams <dan.j.williams@xxxxxxxxx>
>> Cc: Michal Hocko <mhocko@xxxxxxxxxx>
>> Signed-off-by: David Hildenbrand <david@xxxxxxxxxx>
>> ---
>>
>> 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.

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))?

-- 
Thanks,

David / dhildenb