On Wed, May 14, 2014 at 10:27:20AM -0400, Alan Stern wrote: > On Tue, 13 May 2014, Todd E Brandt wrote: > > > This patch creates a separate instance of the usb_address0 mutex for each host > > controller, and attaches it to the host controller device struct. This allows > > devices on separate hosts to be enumerated in parallel; saving time. > > > > In the current code, there is a single, global instance of the usb_address0 > > mutex which is used for all devices on any host. This isn't completely > > necessary, as this mutex is only needed to prevent address0 collisions for > > devices on the *same* host (usb 2.0 spec, sec 4.6.1). This superfluous coverage > > can cause additional delay in system resume on systems with multiple hosts > > (up to several seconds depending on what devices are attached). > > > > For instance, if I have a USB WLAN and a KVM switch attached to two ports, > > there's a chance that they could be initialized at the same time (e.g. on > > system resume). They would both be in the Default state and would be > > responding to requests from the default address (address 00H). If they were > > on the same host, there'd be no way of differentiating the two devices and thus > > the mutex is needed. But on separate hosts there's no chance of collision. > > > > Signed-off-by: Todd Brandt <todd.e.brandt@xxxxxxxxxxxxxxx> > > Does this really save any meaningful amount of time? Have you measured > it? Yes, here's the test results from the use-case that inspired the patch: https://01.org/suspendresume/blogs/tebrandt/2014/usb-resume-parallel-enumeration-separate-hosts That's over a second of time savings. > > > --- a/drivers/usb/core/hcd.c > > +++ b/drivers/usb/core/hcd.c > > @@ -2452,9 +2452,18 @@ struct usb_hcd *usb_create_shared_hcd(const struct hc_driver *driver, > > return NULL; > > } > > mutex_init(hcd->bandwidth_mutex); > > + hcd->usb_address0_mutex = > > + kmalloc(sizeof(*hcd->usb_address0_mutex), GFP_KERNEL); > > + if (!hcd->usb_address0_mutex) { > > + kfree(hcd); > > + dev_dbg(dev, "hcd usb_address0 mutex alloc failed\n"); > > + return NULL; > > + } > > + mutex_init(hcd->usb_address0_mutex); > > Why do you allocate the mutex dynamically? Why not simply use a static > mutex embedded in the usb_hcd structure? I was just copying the existing style of the bandwidth_mutex for code symmetry, but I can resubmit with a static mutex and without the kmalloc error print (from Greg KH's mail) > > Alan Stern > -- 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
