On Fri, Feb 24, 2023 at 12:04:12AM +0100, Bastien Nocera wrote: > On Thu, 2023-02-23 at 12:07 -0500, Alan Stern wrote: > > The refcounting in your patch guarantees that when the work function > > runs, the interface structure will still exist. But refcounting does > > not guarantee that the interface will still be registered in sysfs, > > and > > this can actually happen if the work is scheduled immediately before > > the > > interface is unregistered. > > > > So my question is: What will happen when sysfs_update_group(), > > sysfs_notify(), and kobject_uevent() are called after the interface > > has > > been unregistered from sysfs? Maybe they will work okay -- I simply > > don't know, and I wanted to find out whether you had considered the > > issue. > > A long week-end started for me a couple of hours ago, but I wanted to > dump my thoughts before either I forgot, or it took over my whole week- > end ;) > > I had thought about the problem, and didn't think that sysfs files > would get removed before the interface got released/unref'ed and > usb_remove_sysfs_intf_files() got called. > > If the device gets removed from the device bus before it's released, > then this patch should fix it: > --- a/drivers/usb/core/message.c > +++ b/drivers/usb/core/message.c > @@ -1917,7 +1917,8 @@ static void __usb_wireless_status_intf(struct work_struct *ws) > struct usb_interface *iface = > container_of(ws, struct usb_interface, wireless_status_work); > > - usb_update_wireless_status_attr(iface); > + if (intf->sysfs_files_created) > + usb_update_wireless_status_attr(iface); > usb_put_intf(iface); /* Undo _get_ in usb_set_wireless_status() */ > > The callback would be a no-op if the device's sysfs is already > unregistered, just unref'ing the reference it held. > > What do you think? I'll amend that into my patchset on Monday. That's a good way to do it, but it does race with usb_remove_sysfs_intf_files(). To prevent this race, you can protect the test and function call with device_lock(iface->dev.parent) (that is, lock the interface's parent usb_device). Alan Stern
