Re: Possible hungtask issue will be introduced with device_lock() in uevent_show()

["zhangzekun (A)" <zhangzekun11@huawei.com>]

> Hi Zekun,
> 
> If you have some cycles to help investigate the replacement fix that
> would be much appreciated.
> 
> So far I came up with this:
> 
> http://lore.kernel.org/172790598832.1168608.4519484276671503678.stgit@dwillia2-xfh.jf.intel.com
> 
> ...but have not had time to debug the 0day report.
> 
> My worry is still that Linux has long since shipped the expectation that
> reading 'uevent' bounces the device_lock() which, among other things,
> makes sure that any in-flight driver probing has completed.
> 
> The report of USB devices disappearing is consistent with a udev
> operation failing due to the driver not being done attaching, or
> something similar.
> 
> So even though you have not seen any issues, I suspect small differences
> in the devices on your system and the reporter's system, or udev rule
> differences could result in a failure to trigger the regression.
> 
> 
Hi, Dan

The patch trys to prevent the potential dead lock by letting 
device_lock() gets before kernfs_get_active(), but I think it might not 
be enough to solve all the potential dead lock. The device_lock() will 
be held while removing the driver in device_release_driver(), which 
means will be held through driver's .remove(). If .remove() is waiting 
for resources to be released by userspace process, and then the 
userspace process call device_lock() in uevent_show(), we will have dead 
locks here.

In the following case:

device_release_driver
device_lock()          blocked by
<operation 1> ---------------------------> <operation 2>
                                            uevent_show()
                                            device_lock()


In [1], It is beacause that pci_stop_and_remove_bus_device() will call 
device_release_driver() first and then call device_del() to remove the 
uevent sysfs attributes （Sorry for mistakes made in [1], 
device_release_driver() is called by pci_stop_bus_device()). So, I think 
it might not be a good idea to hold the device_lock() through uevent_show().

ioctl(..,VFIO_GROUP_GET_DEVICE_FD,..)
         ...
         vfio_device_get_from_name()
         vfio_devcie->refcount -> 2
                                         pci_stop_and_remove_bus_device()
                                         pci_stop_bus_device（）
                                         device_release_driver()
                                         device_lock()
                                         device_remove()
												vfio_unregister_group_dev()
												vfio_device_put_registration()
												vfio_devcie->refcount -> 1
												wait for refcount == 0
uevent_show()
         device_lock()

[1] 
https://lore.kernel.org/lkml/20241231075608.84009-1-zhangzekun11@huawei.com/

Best Regards,
Zekun