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 Thu, Feb 23, 2023 at 05:51:48PM +0100, Bastien Nocera wrote:
> On Thu, 2023-02-23 at 11:25 -0500, Alan Stern wrote:
> > On Thu, Feb 23, 2023 at 05:17:13PM +0100, Bastien Nocera wrote:
> > > On Thu, 2023-02-23 at 10:41 -0500, Alan Stern wrote:
> > > > On Thu, Feb 23, 2023 at 02:24:51PM +0100, Bastien Nocera wrote:
> > > > > Allow device specific drivers to change the wireless status of
> > > > > a
> > > > > device.
> > > > > This will allow user-space to know whether the device is
> > > > > available,
> > > > > whether or not specific USB interfaces can detect it.
> > > > > 
> > > > > This can be used by wireless headsets with USB receivers to
> > > > > propagate to
> > > > > user-space whether or not the headset is turned on, so as to
> > > > > consider it
> > > > > as unavailable, and not switch to it just because the receiver
> > > > > is
> > > > > plugged in.
> > > > > 
> > > > > Signed-off-by: Bastien Nocera <hadess@xxxxxxxxxx>
> > > > > ---
> > > > >  drivers/usb/core/message.c | 13 +++++++++++++
> > > > >  drivers/usb/core/usb.c     | 24 ++++++++++++++++++++++++
> > > > >  include/linux/usb.h        |  4 ++++
> > > > >  3 files changed, 41 insertions(+)
> > > > > 
> > > > > diff --git a/drivers/usb/core/message.c
> > > > > b/drivers/usb/core/message.c
> > > > > index 127fac1af676..d5c7749d515e 100644
> > > > > --- a/drivers/usb/core/message.c
> > > > > +++ b/drivers/usb/core/message.c
> > > > > @@ -1908,6 +1908,18 @@ static void
> > > > > __usb_queue_reset_device(struct
> > > > > work_struct *ws)
> > > > >         usb_put_intf(iface);    /* Undo _get_ in
> > > > > usb_queue_reset_device() */
> > > > >  }
> > > > >  
> > > > > +/*
> > > > > + * Internal function to set the wireless_status sysfs
> > > > > attribute
> > > > > + * See usb_set_wireless_status() for more details
> > > > > + */
> > > > > +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);
> > > > > +       usb_put_intf(iface);    /* Undo _get_ in
> > > > > usb_set_wireless_status() */
> > > > > +}
> > > > 
> > > > Have you thought about what will happen if this routine ends up
> > > > running 
> > > > after the interface has been deleted?
> > > 
> > > I believe that usb_release_interface() will only be called once the
> > > last reference to the interface is dropped, so bar any refcounting
> > > bugs, the interface should always exist when this function is
> > > called.
> > 
> > Yes, but what about the calls made by
> > usb_update_wireless_status_attr(): 
> > sysfs_update_group(), sysfs_notify(), and kobject_uevent()?  Will
> > they 
> > work properly when called for an object that has been unregistered
> > from 
> > sysfs?
> 
> Those calls are made before the last reference to the interface can be
> dropped by our own call to usb_put_intf(). So, in effect, the interface
> is kept alive until we're done with our work so we can't ever be poking
> at objects that disappeared.
> 
> Am I missing something about the object model, or the refcounting here?

Yes, you are.

There's a difference between object _existence_ and object 
_registration_.  An object (kobject, struct device, whatever) exists for 
as long as its memory is still allocated and in use.  This is what 
refcounting protects.

An object is registered in sysfs during the time between calls to
device_add() and device_del().  During this time it shows up as a 
directory under /sys, along with its various attribute files.  Calling 
device_del() gets rid of all that -- James Bottomley calls it "removing 
an object from visibility".  The object still exists, but it isn't part 
of sysfs any more.

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.

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