Re: [PATCH 4/5] USB: core: Add API to change the wireless_status

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux