[PATCH] usb: Use a workqueue in usb_add_hcd() to reduce boot time

[PATCH] usb: Use a workqueue in usb_add_hcd() to reduce boot time

[Simon Glass]

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.

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.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 drivers/usb/core/hcd.c  |   75 +++++++++++++++++++++++++++++++++++++++-------
 drivers/usb/core/usb.c  |    5 +++
 include/linux/usb/hcd.h |    9 +++++
 3 files changed, 77 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index eb19cba..a201062 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -111,6 +111,9 @@ static DEFINE_SPINLOCK(hcd_urb_unlink_lock);
 /* wait queue for synchronous unlinks */
 DECLARE_WAIT_QUEUE_HEAD(usb_kill_urb_queue);
 
+/* work queue to handle reset and probing */
+static struct workqueue_struct *hcd_workq;
+
 static inline int is_root_hub(struct usb_device *udev)
 {
 	return (udev->parent == NULL);
@@ -2362,17 +2365,7 @@ static int usb_hcd_request_irqs(struct usb_hcd *hcd,
 	return 0;
 }
 
-/**
- * usb_add_hcd - finish generic HCD structure initialization and register
- * @hcd: the usb_hcd structure to initialize
- * @irqnum: Interrupt line to allocate
- * @irqflags: Interrupt type flags
- *
- * Finish the remaining parts of generic HCD initialization: allocate the
- * buffers of consistent memory, register the bus, request the IRQ line,
- * and call the driver's reset() and start() routines.
- */
-int usb_add_hcd(struct usb_hcd *hcd,
+int usb_add_hcd_work(struct usb_hcd *hcd,
 		unsigned int irqnum, unsigned long irqflags)
 {
 	int retval;
@@ -2517,7 +2510,50 @@ err_allocate_root_hub:
 err_register_bus:
 	hcd_buffer_destroy(hcd);
 	return retval;
-} 
+}
+
+/* This is the work function for usb_add_hcd() */
+void probe_hcd(struct work_struct *item)
+{
+	struct usb_hcd *hcd = container_of(item, struct usb_hcd,
+					   init_work.work);
+	int err;
+
+	err = usb_add_hcd_work(hcd, hcd->init_irqnum, hcd->init_irqflags);
+	if (err)
+		printk(KERN_ERR "probe_hcd failed with error %d\n", err);
+}
+
+/**
+ * usb_add_hcd - finish generic HCD structure initialization and register
+ * @hcd: the usb_hcd structure to initialize
+ * @irqnum: Interrupt line to allocate
+ * @irqflags: Interrupt type flags
+ *
+ * Finish the remaining parts of generic HCD initialization: allocate the
+ * buffers of consistent memory, register the bus, request the IRQ line,
+ * and call the driver's reset() and start() routines.
+ */
+int usb_add_hcd(struct usb_hcd *hcd,
+		unsigned int irqnum, unsigned long irqflags)
+{
+	/*
+	 * Perhaps we should have a pointer to an allocated structure since
+	 * these fields are not used after init.
+	 */
+	INIT_DELAYED_WORK(&hcd->init_work, probe_hcd);
+	hcd->init_irqnum = irqnum;
+	hcd->init_irqflags = irqflags;
+
+	/*
+	 * I'm sure we can't delay this by a second. Should we start it
+	 * immediately? Are we allowed to delay a little? Sometimes USB will
+	 * provide the root disk, so perhaps not.
+	 */
+	if (!queue_delayed_work(hcd_workq, &hcd->init_work, HZ))
+		return -ENOMEM;
+	return 0;
+}
 EXPORT_SYMBOL_GPL(usb_add_hcd);
 
 /**
@@ -2591,6 +2627,21 @@ usb_hcd_platform_shutdown(struct platform_device* dev)
 }
 EXPORT_SYMBOL_GPL(usb_hcd_platform_shutdown);
 
+int usb_hcd_init(void)
+{
+	hcd_workq = alloc_workqueue("usb_hcd",
+				WQ_NON_REENTRANT | WQ_MEM_RECLAIM, 1);
+	if (!hcd_workq)
+		return -ENOMEM;
+
+	return 0;
+}
+
+void usb_hcd_cleanup(void)
+{
+	destroy_workqueue(hcd_workq);
+}
+
 /*-------------------------------------------------------------------------*/
 
 #if defined(CONFIG_USB_MON) || defined(CONFIG_USB_MON_MODULE)
diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 8ca9f99..5e5b944 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -1036,10 +1036,14 @@ static int __init usb_init(void)
 	retval = usb_hub_init();
 	if (retval)
 		goto hub_init_failed;
+	retval = usb_hcd_init();
+	if (retval)
+		goto hcd_init_failed;
 	retval = usb_register_device_driver(&usb_generic_driver, THIS_MODULE);
 	if (!retval)
 		goto out;
 
+hcd_init_failed:
 	usb_hub_cleanup();
 hub_init_failed:
 	usbfs_cleanup();
@@ -1069,6 +1073,7 @@ static void __exit usb_exit(void)
 		return;
 
 	usb_deregister_device_driver(&usb_generic_driver);
+	usb_hcd_cleanup();
 	usb_major_cleanup();
 	usbfs_cleanup();
 	usb_deregister(&usbfs_driver);
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index b2f62f3..49e445b 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -87,6 +87,9 @@ struct usb_hcd {
 #ifdef CONFIG_USB_SUSPEND
 	struct work_struct	wakeup_work;	/* for remote wakeup */
 #endif
+	struct delayed_work	init_work;	/* for initial init */
+	unsigned int		init_irqnum;	/* requested irqnum */
+	unsigned long		init_irqflags;	/* requested irq flags */
 
 	/*
 	 * hardware info/state
@@ -668,6 +671,12 @@ extern struct rw_semaphore ehci_cf_port_reset_rwsem;
 #define USB_EHCI_LOADED		2
 extern unsigned long usb_hcds_loaded;
 
+/* Initalise the HCD ready for use, Must be called before usb_add_hcd() */
+int usb_hcd_init(void);
+
+/* Clean up HCD */
+void usb_hcd_cleanup(void);
+
 #endif /* __KERNEL__ */
 
 #endif /* __USB_CORE_HCD_H */
-- 
1.7.7.3

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

[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

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

[Simon Glass]

Hi Alan,

On Fri, Jan 20, 2012 at 7:46 AM, Alan Stern <stern@rowland.harvard.edu> wrote:
> 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.
>

Thanks for the comments.

> 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.

Well the problem is that we are trying to boot the kernel, so
time-consuming things should be done later or in parallel where
possible.

>
>> 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?

First, backing up a bit, there are at least two ways to solve this.
One is to have the drivers only call usb_add_hcd() from a work queue
or similar - i.e. not as part of their initcall execution. The other
is what I have done here.

Before I go much further I would like to know which is
best/preferable. Because in answer to your questions I'm not sure
drivers have a way of dealing with failure of delayed init. It would
need notification back to the driver at least.

>
> 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

The irq member is normally only assigned when valid, but I'm sure I
could reuse it for init.

> 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

I originally did it that way, but it's more complicated. I can change
it back easily enough.

> workqueue just for registering new USB controllers; what's wrong with
> using the system workqueue?

Nothing - perhaps I could use system_long_wq without anyone
complaining about delays.

>
> 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.

My purpose was to get feedback on what is acceptable. Re your last
point it does seem to make a difference since other things can init in
that time.

As I said above my real uncertainty is where to consider that the
'pure init' is be done, and move the rest into a work queue. Maybe
usb_add_hcd() should stop at the transition to HC_STATE_RUNNING (i.e.
before calling the driver's ->start method), and return success?

Regards,
Simon
>
> Alan Stern
>

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

[Alan Stern]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: TEXT/PLAIN; charset=UTF-8, Size: 4170 bytes --]

On Fri, 20 Jan 2012, Simon Glass wrote:

> Thanks for the comments.
> 
> > 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.
> 
> Well the problem is that we are trying to boot the kernel, so
> time-consuming things should be done later or in parallel where
> possible.

Doing time-consuming things later won't make any difference.  That is,
suppose the boot sequence performs activities A and B, where A takes a
long time.  Doing A later, so that the boot sequence performs B and
then A, won't change the total time required.

Doing things in parallel, on the other hand, can indeed save time.

> >> 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?
> 
> First, backing up a bit, there are at least two ways to solve this.
> One is to have the drivers only call usb_add_hcd() from a work queue
> or similar - i.e. not as part of their initcall execution. The other
> is what I have done here.
> 
> Before I go much further I would like to know which is
> best/preferable. Because in answer to your questions I'm not sure
> drivers have a way of dealing with failure of delayed init. It would
> need notification back to the driver at least.

That's right; all the host controller drivers would need to be modified 
to handle async probing errors.  There's no way to do what you want by 
changing only the USB core.

However, because there are so many drivers, it would be best if the 
driver changes could be minimized.  It's up to you to decide the best 
balance of effort between the drivers and the core, but you can't avoid 
the need to change them at least a little.

> > 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
> 
> I originally did it that way, but it's more complicated. I can change
> it back easily enough.

Okay, good.

> > workqueue just for registering new USB controllers; what's wrong with
> > using the system workqueue?
> 
> Nothing - perhaps I could use system_long_wq without anyone
> complaining about delays.

system_freezable_wq would be better.  Among other things, usb_add_hcd 
registers the HC's root hub, which must not happen while the system is 
preparing for suspend.

> > 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.
> 
> My purpose was to get feedback on what is acceptable. Re your last
> point it does seem to make a difference since other things can init in
> that time.

See my comments above.  The total amount of work done by the system 
remains the same; the advantage to running in parallel is that one task 
can run while another is waiting.

> As I said above my real uncertainty is where to consider that the
> 'pure init' is be done, and move the rest into a work queue. Maybe
> usb_add_hcd() should stop at the transition to HC_STATE_RUNNING (i.e.
> before calling the driver's ->start method), and return success?

Have you collected timings for both the reset and start methods, as
well as the other parts of usb_add_hcd?  Knowing what parts take
longest will help guide your decision.  I suspect you'll find that both 
methods take long enough to be worth putting in the work queue.

Alan Stern

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

[Alan Stern]

On Fri, 20 Jan 2012, Simon Glass wrote:

> First, backing up a bit, there are at least two ways to solve this.
> One is to have the drivers only call usb_add_hcd() from a work queue
> or similar - i.e. not as part of their initcall execution. The other
> is what I have done here.
> 
> Before I go much further I would like to know which is
> best/preferable. Because in answer to your questions I'm not sure
> drivers have a way of dealing with failure of delayed init. It would
> need notification back to the driver at least.

I thought about this a little more.  Maybe it's okay not to unbind the 
driver when the async initialization fails.  After all, it's not like 
any other driver is going to want to manage that hardware.

But that still leaves problems.  Unregistration should work properly
even when async init fails.  The easiest solution seems to be to clear
hcd->self.root_hub whenever async initialization fails, and then in
usb_remove_hcd, return immediately if hcd->self.root_hub isn't set.  
Doing things this way, you avoid the need to modify all those
controller drivers.

That's not all.  You have a race between unbinding and async
initialization.  Probably you will need to make the work routine lock
the hcd->self.controller device, for mutual exclusion with unbinding.  
In addition, usb_remove_hcd should try to cancel the work routine, in
case it hasn't run yet.  And finally, for the case where both things
end up running concurrently, the work routine should check whether
unbinding has already started and fail immediately if it has.

Alan Stern

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

[Sebastian Andrzej Siewior]

* Alan Stern | 2012-01-21 11:11:09 [-0500]:

>Doing time-consuming things later won't make any difference.  That is,
>suppose the boot sequence performs activities A and B, where A takes a
>long time.  Doing A later, so that the boot sequence performs B and
>then A, won't change the total time required.

That is true. However if you show the gui _now_ and look for USB later
then it feels faster and this is usually enough.

>Alan Stern

Sebastian

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

[Greg KH]

On Tue, Jan 24, 2012 at 05:57:11PM +0100, Sebastian Andrzej Siewior wrote:
> * Alan Stern | 2012-01-21 11:11:09 [-0500]:
> 
> >Doing time-consuming things later won't make any difference.  That is,
> >suppose the boot sequence performs activities A and B, where A takes a
> >long time.  Doing A later, so that the boot sequence performs B and
> >then A, won't change the total time required.
> 
> That is true. However if you show the gui _now_ and look for USB later
> then it feels faster and this is usually enough.

Which has been done before, with great success (i.e. we did it for
Moblin boot times), so it shouldn't be that big of an issue.

Which points out the question, exactly what is the issue here?  We have
"multithreaded" pci driver startup for a long time now, did that stop
working here?  If you use bootchart, does it show that the USB host
driver is stoping the machine from proceeding to the next stage in the
boot process?

>From my experience, I never saw the USB host controller get in the way
at all, it was always somethine else happening that caused problems
(i.e. video.)

Simon, do you have boot charts anywhere showing this?

thanks,

greg k-h

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

[Simon Glass]

Hi Greg,

On Tue, Jan 24, 2012 at 9:22 AM, Greg KH <gregkh@suse.de> wrote:
> On Tue, Jan 24, 2012 at 05:57:11PM +0100, Sebastian Andrzej Siewior wrote:
>> * Alan Stern | 2012-01-21 11:11:09 [-0500]:
>>
>> >Doing time-consuming things later won't make any difference.  That is,
>> >suppose the boot sequence performs activities A and B, where A takes a
>> >long time.  Doing A later, so that the boot sequence performs B and
>> >then A, won't change the total time required.
>>
>> That is true. However if you show the gui _now_ and look for USB later
>> then it feels faster and this is usually enough.
>
> Which has been done before, with great success (i.e. we did it for
> Moblin boot times), so it shouldn't be that big of an issue.
>
> Which points out the question, exactly what is the issue here?  We have
> "multithreaded" pci driver startup for a long time now, did that stop
> working here?  If you use bootchart, does it show that the USB host
> driver is stoping the machine from proceeding to the next stage in the
> boot process?
>
> From my experience, I never saw the USB host controller get in the way
> at all, it was always somethine else happening that caused problems
> (i.e. video.)
>
> Simon, do you have boot charts anywhere showing this?

I did have initcall timing when I last looked at this. USB was holding
up progress. Sorry for the delay - I hope to get back to this next
week.

Regards,
Simon

>
> thanks,
>
> greg k-h

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

[Greg KH]

On Thu, Jan 26, 2012 at 03:48:43PM -0800, Simon Glass wrote:
> Hi Greg,
> 
> On Tue, Jan 24, 2012 at 9:22 AM, Greg KH <gregkh@suse.de> wrote:
> > On Tue, Jan 24, 2012 at 05:57:11PM +0100, Sebastian Andrzej Siewior wrote:
> >> * Alan Stern | 2012-01-21 11:11:09 [-0500]:
> >>
> >> >Doing time-consuming things later won't make any difference.  That is,
> >> >suppose the boot sequence performs activities A and B, where A takes a
> >> >long time.  Doing A later, so that the boot sequence performs B and
> >> >then A, won't change the total time required.
> >>
> >> That is true. However if you show the gui _now_ and look for USB later
> >> then it feels faster and this is usually enough.
> >
> > Which has been done before, with great success (i.e. we did it for
> > Moblin boot times), so it shouldn't be that big of an issue.
> >
> > Which points out the question, exactly what is the issue here?  We have
> > "multithreaded" pci driver startup for a long time now, did that stop
> > working here?  If you use bootchart, does it show that the USB host
> > driver is stoping the machine from proceeding to the next stage in the
> > boot process?
> >
> > From my experience, I never saw the USB host controller get in the way
> > at all, it was always somethine else happening that caused problems
> > (i.e. video.)
> >
> > Simon, do you have boot charts anywhere showing this?
> 
> I did have initcall timing when I last looked at this. USB was holding
> up progress. Sorry for the delay - I hope to get back to this next
> week.

Are you building your USB host controllers into the kernel, or are they
modules?

What is sitting on the USB bus that is needed for the boot process to
continue on to init and then later?  Is the root disk on it?

thanks,

greg k-h

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

[Simon Glass]

Hi Greg,

On Thu, Jan 26, 2012 at 3:51 PM, Greg KH <gregkh@suse.de> wrote:
> On Thu, Jan 26, 2012 at 03:48:43PM -0800, Simon Glass wrote:
>> Hi Greg,
>>
>> On Tue, Jan 24, 2012 at 9:22 AM, Greg KH <gregkh@suse.de> wrote:
>> > On Tue, Jan 24, 2012 at 05:57:11PM +0100, Sebastian Andrzej Siewior wrote:
>> >> * Alan Stern | 2012-01-21 11:11:09 [-0500]:
>> >>
>> >> >Doing time-consuming things later won't make any difference.  That is,
>> >> >suppose the boot sequence performs activities A and B, where A takes a
>> >> >long time.  Doing A later, so that the boot sequence performs B and
>> >> >then A, won't change the total time required.
>> >>
>> >> That is true. However if you show the gui _now_ and look for USB later
>> >> then it feels faster and this is usually enough.
>> >
>> > Which has been done before, with great success (i.e. we did it for
>> > Moblin boot times), so it shouldn't be that big of an issue.
>> >
>> > Which points out the question, exactly what is the issue here?  We have
>> > "multithreaded" pci driver startup for a long time now, did that stop
>> > working here?  If you use bootchart, does it show that the USB host
>> > driver is stoping the machine from proceeding to the next stage in the
>> > boot process?
>> >
>> > From my experience, I never saw the USB host controller get in the way
>> > at all, it was always somethine else happening that caused problems
>> > (i.e. video.)
>> >
>> > Simon, do you have boot charts anywhere showing this?
>>
>> I did have initcall timing when I last looked at this. USB was holding
>> up progress. Sorry for the delay - I hope to get back to this next
>> week.
>
> Are you building your USB host controllers into the kernel, or are they
> modules?
>
> What is sitting on the USB bus that is needed for the boot process to
> continue on to init and then later?  Is the root disk on it?

A hub with a few things on it (Ethernet, USB stick). No we don't
normally have a root disk on it - we could in fact init USB *much*
later and be quite happy. Sorry I don't have full answers until I get
back to this. But USB does 'start' each port within the initcall which
seems to take time. Maybe it is at bottom a driver issue?

Regards,
Simon

>
> thanks,
>
> greg k-h

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

[Greg KH]

On Thu, Jan 26, 2012 at 03:57:23PM -0800, Simon Glass wrote:
> Hi Greg,
> 
> On Thu, Jan 26, 2012 at 3:51 PM, Greg KH <gregkh@suse.de> wrote:
> > On Thu, Jan 26, 2012 at 03:48:43PM -0800, Simon Glass wrote:
> >> Hi Greg,
> >>
> >> On Tue, Jan 24, 2012 at 9:22 AM, Greg KH <gregkh@suse.de> wrote:
> >> > On Tue, Jan 24, 2012 at 05:57:11PM +0100, Sebastian Andrzej Siewior wrote:
> >> >> * Alan Stern | 2012-01-21 11:11:09 [-0500]:
> >> >>
> >> >> >Doing time-consuming things later won't make any difference.  That is,
> >> >> >suppose the boot sequence performs activities A and B, where A takes a
> >> >> >long time.  Doing A later, so that the boot sequence performs B and
> >> >> >then A, won't change the total time required.
> >> >>
> >> >> That is true. However if you show the gui _now_ and look for USB later
> >> >> then it feels faster and this is usually enough.
> >> >
> >> > Which has been done before, with great success (i.e. we did it for
> >> > Moblin boot times), so it shouldn't be that big of an issue.
> >> >
> >> > Which points out the question, exactly what is the issue here?  We have
> >> > "multithreaded" pci driver startup for a long time now, did that stop
> >> > working here?  If you use bootchart, does it show that the USB host
> >> > driver is stoping the machine from proceeding to the next stage in the
> >> > boot process?
> >> >
> >> > From my experience, I never saw the USB host controller get in the way
> >> > at all, it was always somethine else happening that caused problems
> >> > (i.e. video.)
> >> >
> >> > Simon, do you have boot charts anywhere showing this?
> >>
> >> I did have initcall timing when I last looked at this. USB was holding
> >> up progress. Sorry for the delay - I hope to get back to this next
> >> week.
> >
> > Are you building your USB host controllers into the kernel, or are they
> > modules?
> >
> > What is sitting on the USB bus that is needed for the boot process to
> > continue on to init and then later?  Is the root disk on it?
> 
> A hub with a few things on it (Ethernet, USB stick). No we don't
> normally have a root disk on it - we could in fact init USB *much*
> later and be quite happy. Sorry I don't have full answers until I get
> back to this. But USB does 'start' each port within the initcall which
> seems to take time. Maybe it is at bottom a driver issue?

Then load the usb module from a thread after init starts up and you
should be fine for boot speed, right?  That's what other distros did
with no known ill side effects.

greg k-h

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

[Simon Glass]

Hi Greg,

On Thu, Jan 26, 2012 at 4:07 PM, Greg KH <gregkh@suse.de> wrote:
> On Thu, Jan 26, 2012 at 03:57:23PM -0800, Simon Glass wrote:
>> Hi Greg,
>>
>> On Thu, Jan 26, 2012 at 3:51 PM, Greg KH <gregkh@suse.de> wrote:
>> > On Thu, Jan 26, 2012 at 03:48:43PM -0800, Simon Glass wrote:
>> >> Hi Greg,
>> >>
>> >> On Tue, Jan 24, 2012 at 9:22 AM, Greg KH <gregkh@suse.de> wrote:
>> >> > On Tue, Jan 24, 2012 at 05:57:11PM +0100, Sebastian Andrzej Siewior wrote:
>> >> >> * Alan Stern | 2012-01-21 11:11:09 [-0500]:
>> >> >>
>> >> >> >Doing time-consuming things later won't make any difference.  That is,
>> >> >> >suppose the boot sequence performs activities A and B, where A takes a
>> >> >> >long time.  Doing A later, so that the boot sequence performs B and
>> >> >> >then A, won't change the total time required.
>> >> >>
>> >> >> That is true. However if you show the gui _now_ and look for USB later
>> >> >> then it feels faster and this is usually enough.
>> >> >
>> >> > Which has been done before, with great success (i.e. we did it for
>> >> > Moblin boot times), so it shouldn't be that big of an issue.
>> >> >
>> >> > Which points out the question, exactly what is the issue here?  We have
>> >> > "multithreaded" pci driver startup for a long time now, did that stop
>> >> > working here?  If you use bootchart, does it show that the USB host
>> >> > driver is stoping the machine from proceeding to the next stage in the
>> >> > boot process?
>> >> >
>> >> > From my experience, I never saw the USB host controller get in the way
>> >> > at all, it was always somethine else happening that caused problems
>> >> > (i.e. video.)
>> >> >
>> >> > Simon, do you have boot charts anywhere showing this?
>> >>
>> >> I did have initcall timing when I last looked at this. USB was holding
>> >> up progress. Sorry for the delay - I hope to get back to this next
>> >> week.
>> >
>> > Are you building your USB host controllers into the kernel, or are they
>> > modules?
>> >
>> > What is sitting on the USB bus that is needed for the boot process to
>> > continue on to init and then later?  Is the root disk on it?
>>
>> A hub with a few things on it (Ethernet, USB stick). No we don't
>> normally have a root disk on it - we could in fact init USB *much*
>> later and be quite happy. Sorry I don't have full answers until I get
>> back to this. But USB does 'start' each port within the initcall which
>> seems to take time. Maybe it is at bottom a driver issue?
>
> Then load the usb module from a thread after init starts up and you
> should be fine for boot speed, right?  That's what other distros did
> with no known ill side effects.

Sorry I missed your question about modules. We don't have an initrd
and need to boot from USB sometimes (recovery mode). We don't use
modules for USB at present.

It seems to me that initcalls are not the place to start going off and
doing time-consuming hardware init, or at least not if we care about
boot time. I am hoping for a solution which has this happen in
parallel without the need for modules and the like.

I am going to get back into this soon and will go through your
comments and others thoughts and see what I can come up with.

Regards,
Simon

>
> greg k-h