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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
