On Thu, May 15, 2014 at 10:05:55AM -0400, Alan Stern wrote: > On Wed, 14 May 2014, Todd E Brandt wrote: > > > > 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. > > Hmmm. Why did the kernel end up re-initializing those devices in the > first place? Under normal circumstances that wouldn't happen when > resuming from system suspend. > > If you were resuming from hibernation, then sure. But hibernation is > pretty slow anyway. Good question, I see reset behavior in S3 often enough for it to seem normal to me (even on different machines). In this case the USB WLAN was hotplugged a few minutes prior to the suspend, so I'm not sure it was wholely configured. If I take the system through another suspend/resume the WLAN doesn't get reset. The KVM switch just does that every time, I'm not sure why (it's a TRENDnet TK-409). It could be a bug with the hardware (maybe it should have been designed with an external power supply but lacks one?) or just a quirk of this particular device. Let me see if I can dig up some more cases of devices that get reset on S3 resume to get a better understanding of how common it is. > > > > > --- 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) > > bandwidth_mutex is a bad model, because when two buses share a single > host controller, their bandwidth calculations have to be mutually > exclusive (since they are carried out by the single controller). > Whereas when two devices use address 0 on different buses, it doesn't > matter if the two buses belong to the same controller. Maybe the hcd struct is a bad spot for the mutex to live on. I could add it to the usb_hub struct but it would need to be on the root instance. > > 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
