Re: [PATCH] usb: Use a workqueue in usb_add_hcd() to reduce boottime

[Alan Stern]

On Thu, 19 Jan 2012, Simon Glass wrote:

> This allows the boot to progress while USB is being probed - which
> otherwise takes about 70ms per controller on my Tegra2 system.
> 
> It was mentioned some years ago in an email from Linus Torvalds:
> 
> https://lkml.org/lkml/2008/10/10/411
> 
> >  - they call usb_add_hcd, and usb_add_hcd is a horrible and slow
> > piece of crap that doesn't just add the host controller, but does all
> > the probing too.
> >
> > In other words, what should be fixed is not the initcall sequence,
> > and certainly not make PCI device probing (or other random buses) be
> > partly asynchronous, but simply make that USB host controller startup
> > function be asynchronous.

Too bad Linus didn't send this message to the linux-usb mailing list; 
it might have received more attention.  Also, it's not clear what he 
meant by "does all the probing too" -- since usb_add_hcd() is called 
from within various drivers' probe routines, it seems only reasonable 
that it _should_ do probing.

> It might be better to delay the work until much later unless USB is needed
> for the root disk, but that might have to be a command line option.

This may be a worthy goal, but your implementation is not good.  In
particular, if asynchronous usb_add_hcd fails then the driver should
not remain bound to the controller device.  Did you test what happens
when the delayed routine fails and you then try to unload the host
controller driver?

Also, you added at least one new field to the usb_hcd structure that is
pretty much just a copy of a field that already exists.  And you added
a delayed_work structure that is only going to be used once -- why not
allocate and free it separately instead?  And you created an entire new
workqueue just for registering new USB controllers; what's wrong with
using the system workqueue?

Finally, what justification is there for delaying everything by one 
second?  If a USB controller is critical then you don't want to delay 
it at all, if the controller was hotplugged some time after booting 
then a delay doesn't matter, and if the action takes place during 
booting then a 1-second delay isn't going to be long enough to make any 
difference.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/