On Fri, Sep 13, 2013 at 12:25 PM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > On Fri, 13 Sep 2013, Bjorn Helgaas wrote: > >> usb_bus_list_lock protects the usb_bus_list, and we don't touch that list >> in usb_disconnect(), so there's no reason to hold the lock here. > > The code says: > > /* used when updating list of hcds */ > DEFINE_MUTEX(usb_bus_list_lock); /* exported only for usbfs */ > EXPORT_SYMBOL_GPL (usb_bus_list_lock); > > Unforunately, the first comment is incomplete. usb_bus_list_lock > protects both the list of buses and also the set of root hubs. That's > why it isn't needed in hub_quiesce() and hub_port_connect_change(); > those routines remove only non-root devices. > > Removing the usages in usb_add_hcd() and usb_remove_hcd() isn't safe. > You can see where this usage is important in > devices.c:usb_device_read(). I don't know how the set of root hubs is managed, so I don't see the conflict between, e.g., usb_remove_hcd() and usb_device_read(). I'm not looking to become a USB expert, so I'll just take your word for it -- I don't want to waste more of your time on it. > Ironically, the second comment above is wrong as well. Although usbfs > does use the lock, usbfs is part of the same module as hcd.c. > Therefore there's no reason to EXPORT usb_bus_list_lock. I'll update the comments and the export and repost these. > P.S.: What led you to submit these changes? PCI locking of device addition and removal is a mess, and I thought maybe I could learn from or emulate USB. Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
