2008-07-20 15:59:34

by Arjan van de Ven

[permalink] [raw]
Subject: [patch 4/3] fastboot: hold the BKL over the async init call sequence

From: Arjan van de Ven <[email protected]>
Subject: [PATCH] fastboot: hold the BKL over the async init call sequence

Regular init calls are called with the BKL held; make sure
the async init calls are also called with the BKL held.
While this reduces parallelism a little, it does provide
lock-for-lock compatibility. The hit to prallelism isn't too
bad, most of the init calls are done immediately or actually
block for their delays.

Signed-off-by: Arjan van de Ven <[email protected]>
---
init/main.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/init/main.c b/init/main.c
index bf79b83..dcb2c32 100644
--- a/init/main.c
+++ b/init/main.c
@@ -746,8 +746,14 @@ static void __init do_async_initcalls(struct work_struct *dummy)
{
initcall_t *call;

+ /*
+ * For compatibility with normal init calls... take the BKL
+ * not pretty, not desirable, but compatibility first
+ */
+ lock_kernel();
for (call = __async_initcall_start; call < __async_initcall_end; call++)
do_one_initcall(*call);
+ unlock_kernel();
}

static struct workqueue_struct *async_init_wq;
--
1.5.5.1


2008-07-20 16:00:58

by Arjan van de Ven

[permalink] [raw]
Subject: [patch 5/3] fastboot: sync the async execution before late_initcall and move level 6s (sync) first

From: Arjan van de Ven <[email protected]>
Subject: [PATCH] fastboot: sync the async execution before late_initcall and move level 6s (sync) first

Rene Herman points out several cases where it's basically needed to have all
level 6/6a/6s calls done before the level 7 (late_initcall) code runs. This patch
adds a sync point in the transition from the 6's to the 7's.

Second, this patch makes sure that level 6s (sync) happens before the async code starts,
and puts a user in driver/pci in this category that needs to happen before device init.

Signed-off-by: Arjan van de Ven <[email protected]>
---
drivers/pci/pci.c | 2 +-
include/asm-generic/vmlinux.lds.h | 3 ++-
init/main.c | 8 +++++++-
3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 44a46c9..d75295d 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1889,7 +1889,7 @@ static int __devinit pci_setup(char *str)
}
early_param("pci", pci_setup);

-device_initcall(pci_init);
+device_initcall_sync(pci_init);

EXPORT_SYMBOL(pci_reenable_device);
EXPORT_SYMBOL(pci_enable_device_io);
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 39c1afc..514dbdf 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -372,11 +372,12 @@
*(.initcall5.init) \
*(.initcall5s.init) \
*(.initcallrootfs.init) \
+ *(.initcall6s.init) \
__async_initcall_start = .; \
*(.initcall6a.init) \
__async_initcall_end = .; \
*(.initcall6.init) \
- *(.initcall6s.init) \
+ __device_initcall_end = .; \
*(.initcall7.init) \
*(.initcall7s.init)

diff --git a/init/main.c b/init/main.c
index dcb2c32..5c9e90e 100644
--- a/init/main.c
+++ b/init/main.c
@@ -741,6 +741,7 @@ int do_one_initcall(initcall_t fn)

extern initcall_t __initcall_start[], __initcall_end[];
extern initcall_t __async_initcall_start[], __async_initcall_end[];
+extern initcall_t __device_initcall_end[];

static void __init do_async_initcalls(struct work_struct *dummy)
{
@@ -764,7 +765,7 @@ static void __init do_initcalls(void)
{
initcall_t *call;
static DECLARE_WORK(async_work, do_async_initcalls);
- int phase = 0; /* 0 = levels 0 - 6, 1 = level 6a, 2 = after level 6a */
+ int phase = 0; /* 0 = levels 0 - 6, 1 = level 6a, 2 = after level 6a, 3 = after level 6 */

async_init_wq = create_singlethread_workqueue("kasyncinit");

@@ -775,6 +776,11 @@ static void __init do_initcalls(void)
}
if (phase == 1 && call >= __async_initcall_end)
phase = 2;
+ if (phase == 2 && call >= __device_initcall_end) {
+ phase = 3;
+ /* make sure all async work is done before level 7 */
+ flush_workqueue(async_init_wq);
+ }
if (phase != 1)
do_one_initcall(*call);
}
--
1.5.5.1

2008-07-20 16:40:55

by Ingo Molnar

[permalink] [raw]
Subject: Re: [patch 5/3] fastboot: sync the async execution before late_initcall and move level 6s (sync) first


* Arjan van de Ven <[email protected]> wrote:

> From: Arjan van de Ven <[email protected]>
> Subject: [PATCH] fastboot: sync the async execution before late_initcall and move level 6s (sync) first
>
> Rene Herman points out several cases where it's basically needed to
> have all level 6/6a/6s calls done before the level 7 (late_initcall)
> code runs. This patch adds a sync point in the transition from the 6's
> to the 7's.
>
> Second, this patch makes sure that level 6s (sync) happens before the
> async code starts, and puts a user in driver/pci in this category that
> needs to happen before device init.

incidentally, this fixed an USB related boot hang i found today on one
of my testsystems running tip/master (which had patches 1-2-3 already),
which i was about to report. Good spotting Rene! I've applied patches
4/5 to tip/fastboot.

Ingo

2008-07-20 21:15:17

by Daniel Walker

[permalink] [raw]
Subject: Re: [patch 5/3] fastboot: sync the async execution before late_initcall and move level 6s (sync) first

On Sun, 2008-07-20 at 09:00 -0700, Arjan van de Ven wrote:
> @@ -775,6 +776,11 @@ static void __init do_initcalls(void)
> }
> if (phase == 1 && call >= __async_initcall_end)
> phase = 2;
> + if (phase == 2 && call >= __device_initcall_end) {
> + phase = 3;
> + /* make sure all async work is done before level 7 */
> + flush_workqueue(async_init_wq);
> + }
> if (phase != 1)
> do_one_initcall(*call);
> }

Did this impact the boot time improvements at all?

Daniel

2008-07-20 21:23:38

by Simon Arlott

[permalink] [raw]
Subject: Re: [patch 5/3] fastboot: sync the async execution before late_initcall and move level 6s (sync) first

On 20/07/08 22:14, Daniel Walker wrote:
> On Sun, 2008-07-20 at 09:00 -0700, Arjan van de Ven wrote:
>> @@ -775,6 +776,11 @@ static void __init do_initcalls(void)
>> }
>> if (phase == 1 && call >= __async_initcall_end)
>> phase = 2;
>> + if (phase == 2 && call >= __device_initcall_end) {
>> + phase = 3;
>> + /* make sure all async work is done before level 7 */
>> + flush_workqueue(async_init_wq);
>> + }
>> if (phase != 1)
>> do_one_initcall(*call);
>> }
>
> Did this impact the boot time improvements at all?

The USB HCD initcalls take so little time to complete (100ms
each) that ensuring they have finished makes no difference.

USB devices get detected after those initcalls finish in
parallel with the rest of the boot process (and there's
about 1.5s before the first USB device driver initcall
runs).

--
Simon Arlott

2008-07-20 21:50:25

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [patch 5/3] fastboot: sync the async execution before late_initcall and move level 6s (sync) first

On Sun, 20 Jul 2008 14:14:59 -0700
Daniel Walker <[email protected]> wrote:

> On Sun, 2008-07-20 at 09:00 -0700, Arjan van de Ven wrote:
> > @@ -775,6 +776,11 @@ static void __init do_initcalls(void)
> > }
> > if (phase == 1 && call >= __async_initcall_end)
> > phase = 2;
> > + if (phase == 2 && call >= __device_initcall_end) {
> > + phase = 3;
> > + /* make sure all async work is done before
> > level 7 */
> > + flush_workqueue(async_init_wq);
> > + }
> > if (phase != 1)
> > do_one_initcall(*call);
> > }
>
> Did this impact the boot time improvements at all?

not much in my measurement; but on my system .. level 7 is mostly empty
so there's not much difference


--
If you want to reach me at my work email, use [email protected]
For development, discussion and tips for power savings,
visit http://www.lesswatts.org

2008-07-29 20:58:17

by Rene Herman

[permalink] [raw]
Subject: Re: [patch 5/3] fastboot: sync the async execution before late_initcall and move level 6s (sync) first

On 20-07-08 18:00, Arjan van de Ven wrote:

> From: Arjan van de Ven <[email protected]>
> Subject: [PATCH] fastboot: sync the async execution before late_initcall and move level 6s (sync) first

> Second, this patch makes sure that level 6s (sync) happens before the async code starts,
> and puts a user in driver/pci in this category that needs to happen before device init.

[ ... ]

> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 44a46c9..d75295d 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -1889,7 +1889,7 @@ static int __devinit pci_setup(char *str)
> }
> early_param("pci", pci_setup);
>
> -device_initcall(pci_init);
> +device_initcall_sync(pci_init);
>
> EXPORT_SYMBOL(pci_reenable_device);
> EXPORT_SYMBOL(pci_enable_device_io);
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index 39c1afc..514dbdf 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -372,11 +372,12 @@
> *(.initcall5.init) \
> *(.initcall5s.init) \
> *(.initcallrootfs.init) \
> + *(.initcall6s.init) \
> __async_initcall_start = .; \
> *(.initcall6a.init) \
> __async_initcall_end = .; \
> *(.initcall6.init) \
> - *(.initcall6s.init) \
> + __device_initcall_end = .; \
> *(.initcall7.init) \
> *(.initcall7s.init)

Isn't this a bit confusing? All the other sync levels are directly after
their respective levels. I can see why you want another level now, but
shouldn't that mean late_initcall now wants to be 8, device_initcall 7
and your new 6s just 6 (device_core_initcall or something...)?

Rene.

2008-07-29 21:04:53

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [patch 5/3] fastboot: sync the async execution before late_initcall and move level 6s (sync) first

On Tue, 29 Jul 2008 23:00:29 +0200
> Isn't this a bit confusing? All the other sync levels are directly
> after their respective levels. I can see why you want another level
> now, but shouldn't that mean late_initcall now wants to be 8,
> device_initcall 7 and your new 6s just 6 (device_core_initcall or
> something...)?
>

yeah it is.. but nobody is using them

I'll make a note to clean this up

(by removing the unused ones)

--
If you want to reach me at my work email, use [email protected]
For development, discussion and tips for power savings,
visit http://www.lesswatts.org

2008-07-29 21:09:51

by Rene Herman

[permalink] [raw]
Subject: Re: [patch 5/3] fastboot: sync the async execution before late_initcall and move level 6s (sync) first

On 29-07-08 23:04, Arjan van de Ven wrote:

> On Tue, 29 Jul 2008 23:00:29 +0200
>> Isn't this a bit confusing? All the other sync levels are directly
>> after their respective levels. I can see why you want another level
>> now, but shouldn't that mean late_initcall now wants to be 8,
>> device_initcall 7 and your new 6s just 6 (device_core_initcall or
>> something...)?
>>
>
> yeah it is.. but nobody is using them
>
> I'll make a note to clean this up
>
> (by removing the unused ones)

Fair enough. By the way:

> @@ -775,6 +776,11 @@ static void __init do_initcalls(void)
> }
> if (phase == 1 && call >= __async_initcall_end)
> phase = 2;
> + if (phase == 2 && call >= __device_initcall_end) {
> + phase = 3;
> + /* make sure all async work is done before level 7 */
> + flush_workqueue(async_init_wq);
> + }
> if (phase != 1)
> do_one_initcall(*call);

After this patch, there are now 2 flush_workqueue(async_init_wq) calls
in do_initcalls. Should the other one remain as well?

Rene.

2008-07-29 21:21:40

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [patch 5/3] fastboot: sync the async execution before late_initcall and move level 6s (sync) first

On Tue, 29 Jul 2008 23:12:11 +0200
Rene Herman <[email protected]> wrote:

> On 29-07-08 23:04, Arjan van de Ven wrote:
>
> > On Tue, 29 Jul 2008 23:00:29 +0200
> >> Isn't this a bit confusing? All the other sync levels are directly
> >> after their respective levels. I can see why you want another level
> >> now, but shouldn't that mean late_initcall now wants to be 8,
> >> device_initcall 7 and your new 6s just 6 (device_core_initcall or
> >> something...)?
> >>
> >
> > yeah it is.. but nobody is using them
> >
> > I'll make a note to clean this up
> >
> > (by removing the unused ones)
>
> Fair enough. By the way:
>
> > @@ -775,6 +776,11 @@ static void __init do_initcalls(void)
> > }
> > if (phase == 1 && call >= __async_initcall_end)
> > phase = 2;
> > + if (phase == 2 && call >= __device_initcall_end) {
> > + phase = 3;
> > + /* make sure all async work is done before
> > level 7 */
> > + flush_workqueue(async_init_wq);
> > + }
> > if (phase != 1)
> > do_one_initcall(*call);
>
> After this patch, there are now 2 flush_workqueue(async_init_wq)
> calls in do_initcalls. Should the other one remain as well?

yes because if you don't have any level 7's then you won't hit this
condition... you need the second one.

flush_workqueue is cheap for the nothing-in-there case.


--
If you want to reach me at my work email, use [email protected]
For development, discussion and tips for power savings,
visit http://www.lesswatts.org

2008-07-29 22:27:44

by Rene Herman

[permalink] [raw]
Subject: Re: [patch 5/3] fastboot: sync the async execution before late_initcall and move level 6s (sync) first

On 29-07-08 23:21, Arjan van de Ven wrote:

> On Tue, 29 Jul 2008 23:12:11 +0200
> Rene Herman <[email protected]> wrote:

>> After this patch, there are now 2 flush_workqueue(async_init_wq)
>> calls in do_initcalls. Should the other one remain as well?
>
> yes because if you don't have any level 7's then you won't hit this
> condition... you need the second one.
>
> flush_workqueue is cheap for the nothing-in-there case.

Ah, yes.

For what it's worth by the way, I'm running that which is available from
your fastboot repo (12 patches currently) on top of 2.6.26. Not seen any
trouble. Nor improvements that I've noticed but this is a rather minimal
and fast booting kernel/system anyway.

Rene.

2008-07-29 22:34:39

by Simon Arlott

[permalink] [raw]
Subject: Re: [patch 5/3] fastboot: sync the async execution before late_initcall and move level 6s (sync) first

On 29/07/08 23:30, Rene Herman wrote:
> On 29-07-08 23:21, Arjan van de Ven wrote:
>
>> On Tue, 29 Jul 2008 23:12:11 +0200
>> Rene Herman <[email protected]> wrote:
>
>>> After this patch, there are now 2 flush_workqueue(async_init_wq)
>>> calls in do_initcalls. Should the other one remain as well?
>>
>> yes because if you don't have any level 7's then you won't hit this
>> condition... you need the second one.
>>
>> flush_workqueue is cheap for the nothing-in-there case.
>
> Ah, yes.
>
> For what it's worth by the way, I'm running that which is available from
> your fastboot repo (12 patches currently) on top of 2.6.26. Not seen any
> trouble. Nor improvements that I've noticed but this is a rather minimal
> and fast booting kernel/system anyway.

It doesn't appear to be possible to init multiple PCI devices at once...
I haven't looked into what is doing it exactly but presumably there's a
lock being held over the whole device probe process.

The speedup from usb seems to be primarily from initialising devices in
the background... perhaps there's some way to do that without doing hcd
init from a second thread?

I get a really slow booting system if I enable the SAS controller... it
requires 14 seconds to initialise itself, even with no drives attached
(LSI 1068E).

--
Simon Arlott

2008-07-30 14:08:54

by Alan Stern

[permalink] [raw]
Subject: Re: [patch 5/3] fastboot: sync the async execution before late_initcall and move level 6s (sync) first

On Tue, 29 Jul 2008, Simon Arlott wrote:

> It doesn't appear to be possible to init multiple PCI devices at once...
> I haven't looked into what is doing it exactly but presumably there's a
> lock being held over the whole device probe process.
>
> The speedup from usb seems to be primarily from initialising devices in
> the background... perhaps there's some way to do that without doing hcd
> init from a second thread?

You could provide useful details by booting a kernel with
CONFIG_USB_DEBUG enabled.

The USB stack _already_ initializes USB devices (i.e., not host
controllers) in a separate thread.

Alan Stern

2008-07-30 18:25:46

by Simon Arlott

[permalink] [raw]
Subject: Re: [patch 5/3] fastboot: sync the async execution before late_initcall and move level 6s (sync) first

On 30/07/08 15:08, Alan Stern wrote:
> On Tue, 29 Jul 2008, Simon Arlott wrote:
>
>> It doesn't appear to be possible to init multiple PCI devices at once...
>> I haven't looked into what is doing it exactly but presumably there's a
>> lock being held over the whole device probe process.
>>
>> The speedup from usb seems to be primarily from initialising devices in
>> the background... perhaps there's some way to do that without doing hcd
>> init from a second thread?
>
> You could provide useful details by booting a kernel with
> CONFIG_USB_DEBUG enabled.
>
> The USB stack _already_ initializes USB devices (i.e., not host
> controllers) in a separate thread.

With fastboot:
162 ohci_hcd_mod_init+0x0/0xa6
167 pcie_portdrv_init+0x0/0x4d
182 saa7134_init+0x0/0x4a
205 ehci_hcd_init+0x0/0x8b
299 snd_usb_audio_init+0x0/0x3d
557 e1000_init_module+0x0/0x88
1227 amd74xx_ide_init+0x0/0x1b
2306 nv_init+0x0/0x1b

Without fastboot:
103 ehci_hcd_init+0x0/0x8b
113 raid5_init+0x0/0x3e
127 pci_iommu_init+0x0/0x17
148 ohci_hcd_mod_init+0x0/0xa4
183 saa7134_init+0x0/0x4a
297 snd_usb_audio_init+0x0/0x3d
557 e1000_init_module+0x0/0x88
1227 amd74xx_ide_init+0x0/0x1b
2303 nv_init+0x0/0x1b
2859 usblp_init+0x0/0x1b

Boot log attached.

It looks like usb device driver init requires it to immediately block and
wait for all devices to have completed init - so regardless of where we
put the usb/ directory in the initcall order, it will always wait a while
because the drivers will be immediately after the hcd init... perhaps
if we moved the hcd init to before net/, sata/, ide/ etc. (all the things
that take time themselves) but left usb device drivers to the end it
would actually get the benefit of that separate thread.

I don't have the time to rework how usb/ is linked in order to try that,
but moving usb/ to before net/ and sata/ while making all the usb device
drivers that get used be late initcalls could be used to test it.

--
Simon Arlott


Attachments:
dmesg-net-sata-usb-nfe0.bz2 (24.55 kB)

2008-07-30 19:41:35

by Alan Stern

[permalink] [raw]
Subject: Re: [patch 5/3] fastboot: sync the async execution before late_initcall and move level 6s (sync) first

On Wed, 30 Jul 2008, Simon Arlott wrote:

> > The USB stack _already_ initializes USB devices (i.e., not host
> > controllers) in a separate thread.
>
> With fastboot:
> 162 ohci_hcd_mod_init+0x0/0xa6
> 167 pcie_portdrv_init+0x0/0x4d
> 182 saa7134_init+0x0/0x4a
> 205 ehci_hcd_init+0x0/0x8b
> 299 snd_usb_audio_init+0x0/0x3d
> 557 e1000_init_module+0x0/0x88
> 1227 amd74xx_ide_init+0x0/0x1b
> 2306 nv_init+0x0/0x1b
>
> Without fastboot:
> 103 ehci_hcd_init+0x0/0x8b
> 113 raid5_init+0x0/0x3e
> 127 pci_iommu_init+0x0/0x17
> 148 ohci_hcd_mod_init+0x0/0xa4
> 183 saa7134_init+0x0/0x4a
> 297 snd_usb_audio_init+0x0/0x3d
> 557 e1000_init_module+0x0/0x88
> 1227 amd74xx_ide_init+0x0/0x1b
> 2303 nv_init+0x0/0x1b
> 2859 usblp_init+0x0/0x1b
>
> Boot log attached.

The timings in the boot log agree with your "Without fastboot:"
figures, so I assume that's the log you attached. The only timings at
issue here are ehci_hcd_init and ohci_hcd_mod_init. It's not clear
that the with-fastboot and without-fastboot values are directly
comparable; during startup there's a lot of activity, and interrupt
handlers can throw the timings off.

> It looks like usb device driver init requires it to immediately block and
> wait for all devices to have completed init

Which USB device driver init are you talking about? Your log includes
usblp_init, usb_stor_init, usb_usual_init, hid_init (for a USB mouse),
and snd_usb_audio_init. Each one completed before the next one
started; none of them blocked waiting for any devices (other than their
own, of course) to finish initializing.

> - so regardless of where we
> put the usb/ directory in the initcall order, it will always wait a while
> because the drivers will be immediately after the hcd init...

No, you're wrong. To prove it, try patching the start of hub_events()
in drivers/usb/core/hub.c like this:

* Not the most efficient, but avoids deadlocks.
*/
while (1) {
+ ssleep(5);

/* Grab the first entry at the beginning of the list */
spin_lock_irq(&hub_event_lock);

Then see what happens.

> perhaps
> if we moved the hcd init to before net/, sata/, ide/ etc. (all the things
> that take time themselves) but left usb device drivers to the end it
> would actually get the benefit of that separate thread.

They are already running in a separate thread. Of course, the
different threads will contend for CPU resources -- just putting things
into multiple threads doesn't mean they will necessarily run
concurrently.

> I don't have the time to rework how usb/ is linked in order to try that,
> but moving usb/ to before net/ and sata/ while making all the usb device
> drivers that get used be late initcalls could be used to test it.

You seem to have a completely mixed-up idea of how the USB stack works.
All the device drivers you're worried about are initialized within the
khubd kernel thread.

Alan Stern

2008-07-31 11:53:35

by Simon Arlott

[permalink] [raw]
Subject: Re: [patch 5/3] fastboot: sync the async execution before late_initcall and move level 6s (sync) first

On Wed, July 30, 2008 20:41, Alan Stern wrote:
> The timings in the boot log agree with your "Without fastboot:"
> figures, so I assume that's the log you attached. The only timings at
> issue here are ehci_hcd_init and ohci_hcd_mod_init. It's not clear
> that the with-fastboot and without-fastboot values are directly
> comparable; during startup there's a lot of activity, and interrupt
> handlers can throw the timings off.

I wasn't suggesting comparing ehci_hcd_init/ohci_hcd_mod_init times,
with fastboot I'm assuming it may manage to take a lock those need in
the main initcall process and delay hcd inits.

>> It looks like usb device driver init requires it to immediately block and
>> wait for all devices to have completed init
>
> Which USB device driver init are you talking about? Your log includes
> usblp_init, usb_stor_init, usb_usual_init, hid_init (for a USB mouse),
> and snd_usb_audio_init. Each one completed before the next one
> started; none of them blocked waiting for any devices (other than their
> own, of course) to finish initializing.

Yes - and that's the point. The initcall process when it reaches usb/ is
this:
1. ehci_hcd_init
2. ohci_hcd_mod_init
3. usblp_init

There is nothing else to run between 1-2 and 3, so there is no opportunity
to initialise devices in the background and usblp_init blocks for a while.

If ehci_hcd_init and ohci_hcd_mod_init were moved up to before sata/ide/net
but usblp_init was kept after these then the devices should be ready by the
time it gets there.

>> - so regardless of where we
>> put the usb/ directory in the initcall order, it will always wait a while
>> because the drivers will be immediately after the hcd init...
>
> No, you're wrong. To prove it, try patching the start of hub_events()
> in drivers/usb/core/hub.c like this:
>
> * Not the most efficient, but avoids deadlocks.
> */
> while (1) {
> + ssleep(5);
>
> /* Grab the first entry at the beginning of the list */
> spin_lock_irq(&hub_event_lock);
>
> Then see what happens.

I'll try this tonight, but all I'd expect to see is the background thread
take longer to do anything?

>> perhaps
>> if we moved the hcd init to before net/, sata/, ide/ etc. (all the things
>> that take time themselves) but left usb device drivers to the end it
>> would actually get the benefit of that separate thread.
>
> They are already running in a separate thread. Of course, the
> different threads will contend for CPU resources -- just putting things
> into multiple threads doesn't mean they will necessarily run
> concurrently.

There's a lot of delays going on during sata/ide/net init, waiting for
the device to do something - usb init continues concurrently.

>> I don't have the time to rework how usb/ is linked in order to try that,
>> but moving usb/ to before net/ and sata/ while making all the usb device
>> drivers that get used be late initcalls could be used to test it.
>
> You seem to have a completely mixed-up idea of how the USB stack works.
> All the device drivers you're worried about are initialized within the
> khubd kernel thread.

The usb device driver initcalls aren't initialised by khubd and they're
definitely blocking on the khubd device initialisation [for that device,
as you said earlier] being completed.

--
Simon Arlott

2008-07-31 15:34:20

by Alan Stern

[permalink] [raw]
Subject: Re: [patch 5/3] fastboot: sync the async execution before late_initcall and move level 6s (sync) first

On Thu, 31 Jul 2008, Simon Arlott wrote:

> I wasn't suggesting comparing ehci_hcd_init/ohci_hcd_mod_init times,
> with fastboot I'm assuming it may manage to take a lock those need in
> the main initcall process and delay hcd inits.

What does your "it" above refer to? One of the USB driver initcalls?

> >> It looks like usb device driver init requires it to immediately block and
> >> wait for all devices to have completed init
> >
> > Which USB device driver init are you talking about? Your log includes
> > usblp_init, usb_stor_init, usb_usual_init, hid_init (for a USB mouse),
> > and snd_usb_audio_init. Each one completed before the next one
> > started; none of them blocked waiting for any devices (other than their
> > own, of course) to finish initializing.
>
> Yes - and that's the point. The initcall process when it reaches usb/ is
> this:
> 1. ehci_hcd_init
> 2. ohci_hcd_mod_init
> 3. usblp_init

It sounds like you have usblp compiled into the kernel instead of
building it as a module. Do things change for the better if you make
usblp a module?

> There is nothing else to run between 1-2 and 3, so there is no opportunity
> to initialise devices in the background and usblp_init blocks for a while.

If it were a module then it would block in a separate thread and
wouldn't hold up the main init process.

> If ehci_hcd_init and ohci_hcd_mod_init were moved up to before sata/ide/net
> but usblp_init was kept after these then the devices should be ready by the
> time it gets there.

It's not entirely clear why usblp is blocking at all. Probably because
it is waiting on the device semaphores for devices that are currently
being probed -- the driver core won't allow two threads to probe the
same device concurrently.

> >> - so regardless of where we
> >> put the usb/ directory in the initcall order, it will always wait a while
> >> because the drivers will be immediately after the hcd init...
> >
> > No, you're wrong. To prove it, try patching the start of hub_events()
> > in drivers/usb/core/hub.c like this:
> >
> > * Not the most efficient, but avoids deadlocks.
> > */
> > while (1) {
> > + ssleep(5);
> >
> > /* Grab the first entry at the beginning of the list */
> > spin_lock_irq(&hub_event_lock);
> >
> > Then see what happens.
>
> I'll try this tonight, but all I'd expect to see is the background thread
> take longer to do anything?

Well, yes, of course. My point is that the drivers (those in modules,
anyway) won't initialize _immediately_ after the HCD init, as you
wrote above. There will be this 5-second delay.

> The usb device driver initcalls aren't initialised by khubd

Um. The initcalls for USB device drivers built as modules are made by
the modprobe thread, which is started in response to a uevent created
by khub. So while they aren't initialized directly by khubd, they are
initialized indirectly by it.

On the other hand, the initcalls for drivers compiled into the kernel
are a different story.

> and they're
> definitely blocking on the khubd device initialisation [for that device,
> as you said earlier] being completed.

Driver probing is always going to be a bottleneck, even for
non-matching device/driver pairs. That's because the driver core
acquires the device semaphore even before calling the bus's match
routine. When a new driver is registered, the driver core just goes
down the list of all devices registered on the bus, locking each one in
turn and trying to match & probe it.

Alan Stern

2008-07-31 18:29:53

by Simon Arlott

[permalink] [raw]
Subject: Re: [patch 5/3] fastboot: sync the async execution before late_initcall and move level 6s (sync) first

On 31/07/08 16:34, Alan Stern wrote:
> On Thu, 31 Jul 2008, Simon Arlott wrote:
>
>> I wasn't suggesting comparing ehci_hcd_init/ohci_hcd_mod_init times,
>> with fastboot I'm assuming it may manage to take a lock those need in
>> the main initcall process and delay hcd inits.
>
> What does your "it" above refer to? One of the USB driver initcalls?

No, a PCI device init in the main initcall process could prevent the
parallel hcd init from completing in the usual time.

>> >> It looks like usb device driver init requires it to immediately block and
>> >> wait for all devices to have completed init
>> >
>> > Which USB device driver init are you talking about? Your log includes
>> > usblp_init, usb_stor_init, usb_usual_init, hid_init (for a USB mouse),
>> > and snd_usb_audio_init. Each one completed before the next one
>> > started; none of them blocked waiting for any devices (other than their
>> > own, of course) to finish initializing.
>>
>> Yes - and that's the point. The initcall process when it reaches usb/ is
>> this:
>> 1. ehci_hcd_init
>> 2. ohci_hcd_mod_init
>> 3. usblp_init
>
> It sounds like you have usblp compiled into the kernel instead of
> building it as a module. Do things change for the better if you make
> usblp a module?

Only if I make ALL usb device drivers modules, otherwise the first one has
to wait for all devices to finish initialising. (I get the same delay on
usb_stor_init instead).

>> There is nothing else to run between 1-2 and 3, so there is no opportunity
>> to initialise devices in the background and usblp_init blocks for a while.
>
> If it were a module then it would block in a separate thread and
> wouldn't hold up the main init process.

Right, but I want to compile all of this into the kernel.

>> If ehci_hcd_init and ohci_hcd_mod_init were moved up to before sata/ide/net
>> but usblp_init was kept after these then the devices should be ready by the
>> time it gets there.
>
> It's not entirely clear why usblp is blocking at all. Probably because
> it is waiting on the device semaphores for devices that are currently
> being probed -- the driver core won't allow two threads to probe the
> same device concurrently.

Ok - so there could be some big improvements to be had by making the hcd
init happen as early as possible and the device initcalls later?

>> >> - so regardless of where we
>> >> put the usb/ directory in the initcall order, it will always wait a while
>> >> because the drivers will be immediately after the hcd init...
>> >
>> > No, you're wrong. To prove it, try patching the start of hub_events()
>> > in drivers/usb/core/hub.c like this:
>> >
>> > * Not the most efficient, but avoids deadlocks.
>> > */
>> > while (1) {
>> > + ssleep(5);
>> >
>> > /* Grab the first entry at the beginning of the list */
>> > spin_lock_irq(&hub_event_lock);
>> >
>> > Then see what happens.
>>
>> I'll try this tonight, but all I'd expect to see is the background thread
>> take longer to do anything?
>
> Well, yes, of course. My point is that the drivers (those in modules,
> anyway) won't initialize _immediately_ after the HCD init, as you
> wrote above. There will be this 5-second delay.

Right, devices wouldn't show up for modules for 5 seconds.

>> The usb device driver initcalls aren't initialised by khubd
>
> Um. The initcalls for USB device drivers built as modules are made by
> the modprobe thread, which is started in response to a uevent created
> by khub. So while they aren't initialized directly by khubd, they are
> initialized indirectly by it.
>
> On the other hand, the initcalls for drivers compiled into the kernel
> are a different story.
>
>> and they're
>> definitely blocking on the khubd device initialisation [for that device,
>> as you said earlier] being completed.
>
> Driver probing is always going to be a bottleneck, even for
> non-matching device/driver pairs. That's because the driver core
> acquires the device semaphore even before calling the bus's match
> routine. When a new driver is registered, the driver core just goes
> down the list of all devices registered on the bus, locking each one in
> turn and trying to match & probe it.

So could HCD init be moved higher in the initcall order so that the devices
have been initialised enough by khubd that device driver initcalls take
much less time?

I'm not really sure how to do that, except by doing usb/core/ usb/host/
and then usb/class/ usb/storage/ etc....

--
Simon Arlott

2008-07-31 18:54:15

by Rene Herman

[permalink] [raw]
Subject: Re: [patch 5/3] fastboot: sync the async execution before late_initcall and move level 6s (sync) first

On 31-07-08 20:29, Simon Arlott wrote:

> Ok - so there could be some big improvements to be had by making the hcd
> init happen as early as possible and the device initcalls later?

Arjan also needed a pre device_initcall() level for PCI core init now
that the async device initcalls weren't governed just by link order
anymore. He reused the device_initcall_sync() level, moving it to before
device_initcall() itself (it used to be just behind).

Your above notion sounds like another good reason for inserting a real
new level just before device_initcall(); if you move any of the device
init to late initcall(), late_initcall() loses too much of its utility
I'd feel (see start of this thread with various late_initcalls wanting
to assume stuff).

Rene.

2008-07-31 19:16:46

by Alan Stern

[permalink] [raw]
Subject: Re: [patch 5/3] fastboot: sync the async execution before late_initcall and move level 6s (sync) first

On Thu, 31 Jul 2008, Simon Arlott wrote:

> > If it were a module then it would block in a separate thread and
> > wouldn't hold up the main init process.
>
> Right, but I want to compile all of this into the kernel.

Okay, that explains things. But this means that changes which are
helpful on your system might not be so helpful (or might even be
detrimental) on other people's systems.

> > It's not entirely clear why usblp is blocking at all. Probably because
> > it is waiting on the device semaphores for devices that are currently
> > being probed -- the driver core won't allow two threads to probe the
> > same device concurrently.
>
> Ok - so there could be some big improvements to be had by making the hcd
> init happen as early as possible and the device initcalls later?

Maybe. Perhaps a better approach would be to make the device driver
initcalls before there are any devices for their probe routines to
block on.

> > Driver probing is always going to be a bottleneck, even for
> > non-matching device/driver pairs. That's because the driver core
> > acquires the device semaphore even before calling the bus's match
> > routine. When a new driver is registered, the driver core just goes
> > down the list of all devices registered on the bus, locking each one in
> > turn and trying to match & probe it.
>
> So could HCD init be moved higher in the initcall order so that the devices
> have been initialised enough by khubd that device driver initcalls take
> much less time?
>
> I'm not really sure how to do that, except by doing usb/core/ usb/host/
> and then usb/class/ usb/storage/ etc....

Try doing this instead: Put a 5-second delay at the start of
hub_thread(). That will give the in-kernel device drivers time to
initialize before any USB devices -- other than root hubs -- are
discovered. That should speed up the init timings.

For everyone else, it will slow down USB device discovery by 5 seconds.
I don't know how important that is, considering how much other stuff is
going on at boot time. You may find that 5 seconds is more than you
need; perhaps 1 second will be enough.

Alan Stern

2008-07-31 19:27:24

by Simon Arlott

[permalink] [raw]
Subject: Re: [patch 5/3] fastboot: sync the async execution before late_initcall and move level 6s (sync) first

On 31/07/08 19:56, Rene Herman wrote:
> On 31-07-08 20:29, Simon Arlott wrote:
>
>> Ok - so there could be some big improvements to be had by making the
>> hcd init happen as early as possible and the device initcalls later?
>
> Arjan also needed a pre device_initcall() level for PCI core init now
> that the async device initcalls weren't governed just by link order
> anymore. He reused the device_initcall_sync() level, moving it to before
> device_initcall() itself (it used to be just behind).
>
> Your above notion sounds like another good reason for inserting a real
> new level just before device_initcall(); if you move any of the device
> init to late initcall(), late_initcall() loses too much of its utility
> I'd feel (see start of this thread with various late_initcalls wanting
> to assume stuff).

Well, the late_initcall idea was just for testing, to make the device
driver parts later while moving usb/ up in link order.

--
Simon Arlott

2008-07-31 21:58:42

by Greg KH

[permalink] [raw]
Subject: Re: [patch 5/3] fastboot: sync the async execution before late_initcall and move level 6s (sync) first

On Thu, Jul 31, 2008 at 07:29:21PM +0100, Simon Arlott wrote:
>>> There is nothing else to run between 1-2 and 3, so there is no
>>> opportunity
>>> to initialise devices in the background and usblp_init blocks for a
>>> while.
>> If it were a module then it would block in a separate thread and wouldn't
>> hold up the main init process.
>
> Right, but I want to compile all of this into the kernel.

Why? It sounds like a trivial solution for you is to actually use
modules. Why go through a lot of extra work to solve something in a
different way that is already solved for you?

Who is imposing the "no modules allowed" rule on you, and why was it
made?

thanks,

greg k-h

2008-07-31 22:14:27

by Simon Arlott

[permalink] [raw]
Subject: Re: [patch 5/3] fastboot: sync the async execution before late_initcall and move level 6s (sync) first

On 31/07/08 22:56, Greg KH wrote:
> On Thu, Jul 31, 2008 at 07:29:21PM +0100, Simon Arlott wrote:
>>>> There is nothing else to run between 1-2 and 3, so there is no
>>>> opportunity
>>>> to initialise devices in the background and usblp_init blocks for a
>>>> while.
>>> If it were a module then it would block in a separate thread and wouldn't
>>> hold up the main init process.
>>
>> Right, but I want to compile all of this into the kernel.
>
> Why? It sounds like a trivial solution for you is to actually use
> modules. Why go through a lot of extra work to solve something in a
> different way that is already solved for you?

Because it looks like doing HCD init early enough is a simple way to
speed up boot time if there are any compiled-in usb device drivers,
without running the HCD init itself from a separate thread.

Arjan, are you able to test this?
I'll try doing what I've suggested later tonight if I have time.

> Who is imposing the "no modules allowed" rule on you, and why was it
> made?

No one is imposing anything on me. I don't yet have an USB keyboard
so I could compile all the drivers as modules without any problems.

There's some strange animosity to compiling things that one knows will
always be needed into the kernel... like the V4L DVB code that used
to fail to work properly, perhaps because everyone uses modules
(4abdcf933f647763592db6bef001d1fae61a5527).

It also leads to people unloading/loading the module to workaround
bugs that could otherwise be fixed
(c278850206fd9df0bb62a72ca0b277fe20c5a452).

--
Simon Arlott

2008-07-31 22:38:04

by Simon Arlott

[permalink] [raw]
Subject: Re: [patch 5/3] fastboot: sync the async execution before late_initcall and move level 6s (sync) first

On 31/07/08 23:12, Simon Arlott wrote:
> On 31/07/08 22:56, Greg KH wrote:
>> On Thu, Jul 31, 2008 at 07:29:21PM +0100, Simon Arlott wrote:
>>>>> There is nothing else to run between 1-2 and 3, so there is no
>>>>> opportunity
>>>>> to initialise devices in the background and usblp_init blocks for a
>>>>> while.
>>>> If it were a module then it would block in a separate thread and
>>>> wouldn't hold up the main init process.
>>>
>>> Right, but I want to compile all of this into the kernel.
>>
>> Why? It sounds like a trivial solution for you is to actually use
>> modules. Why go through a lot of extra work to solve something in a
>> different way that is already solved for you?
>
> Because it looks like doing HCD init early enough is a simple way to
> speed up boot time if there are any compiled-in usb device drivers,
> without running the HCD init itself from a separate thread.
>
> Arjan, are you able to test this?
> I'll try doing what I've suggested later tonight if I have time.

I've tested this with fastboot disabled, the hcd initcalls changed
back to module_init, all my drivers changed to late_initcall to
force them to be later while usb/ is before net/ in the drivers/
Makefile.

--
Simon Arlott


Attachments:
dmesg.bz2 (23.16 kB)

2008-08-06 18:48:23

by Simon Arlott

[permalink] [raw]
Subject: [PATCH RFC] USB: Add HCD fastboot

On 31/07/08 20:16, Alan Stern wrote:
>> > It's not entirely clear why usblp is blocking at all. Probably because
>> > it is waiting on the device semaphores for devices that are currently
>> > being probed -- the driver core won't allow two threads to probe the
>> > same device concurrently.
>>
>> Ok - so there could be some big improvements to be had by making the hcd
>> init happen as early as possible and the device initcalls later?
>
> Maybe. Perhaps a better approach would be to make the device driver
> initcalls before there are any devices for their probe routines to
> block on.

What about this?

The Makefiles become a bit messy, but by moving things around I get the
desired effect without splitting their initcalls.

[ 7.941890] Write protecting the kernel read-only data: 5656k
[ 5.437709] Write protecting the kernel read-only data: 5656k

2.5s faster, which is almost half the boot time.

Signed-off-by: Simon Arlott <[email protected]>

---
drivers/Makefile | 13 ++++++++++---
drivers/usb/Makefile | 30 ++++++++++++++++++++----------
drivers/usb/core/Kconfig | 17 +++++++++++++++++
3 files changed, 47 insertions(+), 13 deletions(-)

diff --git a/drivers/Makefile b/drivers/Makefile
index a280ab3..d68151f 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -33,7 +33,12 @@ obj-$(CONFIG_FB_INTEL) += video/intelfb/

obj-y += serial/
obj-$(CONFIG_PARPORT) += parport/
-obj-y += base/ block/ misc/ mfd/ net/ media/
+obj-y += base/ block/
+ifdef CONFIG_USB_FASTBOOT
+ obj-$(CONFIG_USB) += usb/
+ obj-$(CONFIG_PCI) += usb/
+endif
+obj-y += net/ media/
obj-$(CONFIG_NUBUS) += nubus/
obj-$(CONFIG_ATM) += atm/
obj-y += macintosh/
@@ -56,8 +61,10 @@ obj-$(CONFIG_MAC) += macintosh/
obj-$(CONFIG_ATA_OVER_ETH) += block/aoe/
obj-$(CONFIG_PARIDE) += block/paride/
obj-$(CONFIG_TC) += tc/
-obj-$(CONFIG_USB) += usb/
-obj-$(CONFIG_PCI) += usb/
+ifndef CONFIG_USB_FASTBOOT
+ obj-$(CONFIG_USB) += usb/
+ obj-$(CONFIG_PCI) += usb/
+endif
obj-$(CONFIG_USB_GADGET) += usb/gadget/
obj-$(CONFIG_SERIO) += input/serio/
obj-$(CONFIG_GAMEPORT) += input/gameport/
diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
index a419c42..35766a1 100644
--- a/drivers/usb/Makefile
+++ b/drivers/usb/Makefile
@@ -8,16 +8,22 @@ obj-$(CONFIG_USB) += core/

obj-$(CONFIG_USB_MON) += mon/

-obj-$(CONFIG_PCI) += host/
-obj-$(CONFIG_USB_EHCI_HCD) += host/
-obj-$(CONFIG_USB_ISP116X_HCD) += host/
-obj-$(CONFIG_USB_OHCI_HCD) += host/
-obj-$(CONFIG_USB_UHCI_HCD) += host/
-obj-$(CONFIG_USB_SL811_HCD) += host/
-obj-$(CONFIG_USB_U132_HCD) += host/
-obj-$(CONFIG_USB_R8A66597_HCD) += host/
-
-obj-$(CONFIG_USB_C67X00_HCD) += c67x00/
+tmp-y :=
+tmp-m :=
+tmp-$(CONFIG_PCI) += host/
+tmp-$(CONFIG_USB_EHCI_HCD) += host/
+tmp-$(CONFIG_USB_ISP116X_HCD) += host/
+tmp-$(CONFIG_USB_OHCI_HCD) += host/
+tmp-$(CONFIG_USB_UHCI_HCD) += host/
+tmp-$(CONFIG_USB_SL811_HCD) += host/
+tmp-$(CONFIG_USB_U132_HCD) += host/
+tmp-$(CONFIG_USB_R8A66597_HCD) += host/
+tmp-$(CONFIG_USB_C67X00_HCD) += c67x00/
+obj-m += $(tmp-m)
+
+ifndef CONFIG_USB_FASTBOOT
+ obj-y += $(tmp-y)
+endif

obj-$(CONFIG_USB_ACM) += class/
obj-$(CONFIG_USB_PRINTER) += class/
@@ -34,3 +40,7 @@ obj-$(CONFIG_USB) += misc/

obj-$(CONFIG_USB_ATM) += atm/
obj-$(CONFIG_USB_SPEEDTOUCH) += atm/
+
+ifdef CONFIG_USB_FASTBOOT
+ obj-y += $(tmp-y)
+endif
diff --git a/drivers/usb/core/Kconfig b/drivers/usb/core/Kconfig
index cc9f397..ea04a02 100644
--- a/drivers/usb/core/Kconfig
+++ b/drivers/usb/core/Kconfig
@@ -136,3 +136,20 @@ config USB_OTG_BLACKLIST_HUB
and software costs by not supporting external hubs. So
are "Emedded Hosts" that don't offer OTG support.

+
+config USB_FASTBOOT
+ bool "USB Fastboot (EXPERIMENTAL)"
+ depends on USB && EXPERIMENTAL
+ default n
+ help
+ If you say Y here, USB device driver initialisation will be
+ performed much earlier on boot followed by HCD initialisation.
+ Devices will then be initialised in the background USB hub
+ thread during the rest of the boot process.
+
+ Side effects of enabling this include IRQ assignments being
+ different from a normal bootup and the potential for USB and
+ non-USB devices to be ordered differently on each startup.
+ (i.e. usb sda, sata sdb vs. sata sda, usb sdb)
+
+ If you are unsure about this, say N here.
--
1.5.6.3

--
Simon Arlott

2008-08-06 19:11:26

by Alan Stern

[permalink] [raw]
Subject: Re: [PATCH RFC] USB: Add HCD fastboot

On Wed, 6 Aug 2008, Simon Arlott wrote:

> On 31/07/08 20:16, Alan Stern wrote:
> >> > It's not entirely clear why usblp is blocking at all. Probably because
> >> > it is waiting on the device semaphores for devices that are currently
> >> > being probed -- the driver core won't allow two threads to probe the
> >> > same device concurrently.
> >>
> >> Ok - so there could be some big improvements to be had by making the hcd
> >> init happen as early as possible and the device initcalls later?
> >
> > Maybe. Perhaps a better approach would be to make the device driver
> > initcalls before there are any devices for their probe routines to
> > block on.
>
> What about this?
>
> The Makefiles become a bit messy, but by moving things around I get the
> desired effect without splitting their initcalls.

Wouldn't it be much simpler, and less objectionable, to do what I
suggested earlier? That is, add a 5-second delay at the start of
hub_thread() in drivers/usb/core/hub.c. No messing with Makefiles, no
changes to the initcall scheduling.

Alan Stern

2008-08-06 19:22:14

by Simon Arlott

[permalink] [raw]
Subject: Re: [PATCH RFC] USB: Add HCD fastboot

On 06/08/08 20:11, Alan Stern wrote:
> On Wed, 6 Aug 2008, Simon Arlott wrote:
>
>> On 31/07/08 20:16, Alan Stern wrote:
>> >> > It's not entirely clear why usblp is blocking at all. Probably because
>> >> > it is waiting on the device semaphores for devices that are currently
>> >> > being probed -- the driver core won't allow two threads to probe the
>> >> > same device concurrently.
>> >>
>> >> Ok - so there could be some big improvements to be had by making the hcd
>> >> init happen as early as possible and the device initcalls later?
>> >
>> > Maybe. Perhaps a better approach would be to make the device driver
>> > initcalls before there are any devices for their probe routines to
>> > block on.
>>
>> What about this?
>>
>> The Makefiles become a bit messy, but by moving things around I get the
>> desired effect without splitting their initcalls.
>
> Wouldn't it be much simpler, and less objectionable, to do what I
> suggested earlier? That is, add a 5-second delay at the start of
> hub_thread() in drivers/usb/core/hub.c. No messing with Makefiles, no
> changes to the initcall scheduling.

Aside from 5 seconds being too long, and anything less being a race between
hub_thread() and driver initcalls, it doesn't improve anything because it'll
still have to wait for the devices to finish initialising in userspace instead.

--
Simon Arlott

2008-08-06 19:32:08

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH RFC] USB: Add HCD fastboot

On Wed, Aug 06, 2008 at 08:20:13PM +0100, Simon Arlott wrote:
> On 06/08/08 20:11, Alan Stern wrote:
>> On Wed, 6 Aug 2008, Simon Arlott wrote:
>>> On 31/07/08 20:16, Alan Stern wrote:
>>> >> > It's not entirely clear why usblp is blocking at all. Probably
>>> because
>>> >> > it is waiting on the device semaphores for devices that are
>>> currently
>>> >> > being probed -- the driver core won't allow two threads to probe the
>>> >> > same device concurrently.
>>> >> >> Ok - so there could be some big improvements to be had by making
>>> the hcd >> init happen as early as possible and the device initcalls
>>> later?
>>> > > Maybe. Perhaps a better approach would be to make the device driver
>>> > initcalls before there are any devices for their probe routines to >
>>> block on.
>>> What about this?
>>> The Makefiles become a bit messy, but by moving things around I get the
>>> desired effect without splitting their initcalls.
>> Wouldn't it be much simpler, and less objectionable, to do what I
>> suggested earlier? That is, add a 5-second delay at the start of
>> hub_thread() in drivers/usb/core/hub.c. No messing with Makefiles, no
>> changes to the initcall scheduling.
>
> Aside from 5 seconds being too long, and anything less being a race between
> hub_thread() and driver initcalls, it doesn't improve anything because
> it'll still have to wait for the devices to finish initialising in
> userspace instead.

And what is your boot time if the usb drivers are modules?

I'd much prefer that requirement to get a faster boot than to go around
mucking with the link-time ordering...

thanks,

greg k-h

2008-08-06 19:49:38

by Alan Stern

[permalink] [raw]
Subject: Re: [PATCH RFC] USB: Add HCD fastboot

On Wed, 6 Aug 2008, Simon Arlott wrote:

> > Wouldn't it be much simpler, and less objectionable, to do what I
> > suggested earlier? That is, add a 5-second delay at the start of
> > hub_thread() in drivers/usb/core/hub.c. No messing with Makefiles, no
> > changes to the initcall scheduling.
>
> Aside from 5 seconds being too long, and anything less being a race between
> hub_thread() and driver initcalls, it doesn't improve anything because it'll
> still have to wait for the devices to finish initialising in userspace instead.

Why is 5 seconds too long? Too long for what?

What you're doing is already a race between hub_thread() and driver
initcalls. My suggestion is no worse.

"it'll still have to wait..." If by "it" you mean the initcall
thread, you're wrong. If by "it" you mean the user, you still aren't
necessarily correct; the user can do plenty of other things while
waiting for USB devices to initialize.

I suppose you could make the hub_thread delay time a module parameter
for usbcore, defaulting to 0. Then it could be set by just the people
who want to use it -- many (most?) people keep their drivers in
modules, and it wouldn't do them any good.

Alan Stern

2008-08-06 19:56:37

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [PATCH RFC] USB: Add HCD fastboot

On Wed, 6 Aug 2008 15:49:27 -0400 (EDT)
Alan Stern <[email protected]> wrote:

> On Wed, 6 Aug 2008, Simon Arlott wrote:
>
> > > Wouldn't it be much simpler, and less objectionable, to do what I
> > > suggested earlier? That is, add a 5-second delay at the start of
> > > hub_thread() in drivers/usb/core/hub.c. No messing with
> > > Makefiles, no changes to the initcall scheduling.
> >
> > Aside from 5 seconds being too long, and anything less being a race
> > between hub_thread() and driver initcalls, it doesn't improve
> > anything because it'll still have to wait for the devices to finish
> > initialising in userspace instead.
>
> Why is 5 seconds too long? Too long for what?

because I'm done booting to full UI in that time.

it's forever.

--
If you want to reach me at my work email, use [email protected]
For development, discussion and tips for power savings,
visit http://www.lesswatts.org

2008-08-06 20:07:25

by Simon Arlott

[permalink] [raw]
Subject: Re: [PATCH RFC] USB: Add HCD fastboot

On 06/08/08 20:49, Alan Stern wrote:
> On Wed, 6 Aug 2008, Simon Arlott wrote:
>
>> > Wouldn't it be much simpler, and less objectionable, to do what I
>> > suggested earlier? That is, add a 5-second delay at the start of
>> > hub_thread() in drivers/usb/core/hub.c. No messing with Makefiles, no
>> > changes to the initcall scheduling.
>>
>> Aside from 5 seconds being too long, and anything less being a race between
>> hub_thread() and driver initcalls, it doesn't improve anything because it'll
>> still have to wait for the devices to finish initialising in userspace instead.
>
> Why is 5 seconds too long? Too long for what?
>
> What you're doing is already a race between hub_thread() and driver
> initcalls. My suggestion is no worse.

No, by adding a 5 second delay you're intending for the device driver initcalls
to complete within that 5 seconds. If they take too long then the last one
blocks everything (I realise that's ridiculous, these initcalls take <1ms when
there are no devices yet). The best way to do is to make the driver initcalls
before the host ones, like you suggested.

> "it'll still have to wait..." If by "it" you mean the initcall
> thread, you're wrong. If by "it" you mean the user, you still aren't
> necessarily correct; the user can do plenty of other things while
> waiting for USB devices to initialize.

Assuming userspace doesn't wait for all devices to settle and appear in /dev etc.
before continuing.

> I suppose you could make the hub_thread delay time a module parameter
> for usbcore, defaulting to 0. Then it could be set by just the people
> who want to use it -- many (most?) people keep their drivers in
> modules, and it wouldn't do them any good.

It really needs to have hcd initcalls done very early so that device init
has the rest of the (kernel and userspace) boot process to complete in the
background. This is negated by having device drivers initialised immediately
afterwards. Re-ordering initcalls and doing more of the init process
asynchronously is likely to expose bugs and cause inconsistent device order
on some systems, so if the makefile mess could be reduced then it can be a
Kconfig option.

How many people have *all* their USB components (hcd, drivers) as modules?
What do they do with their USB keyboards in the period between init and module
load? If even one device driver and the hcd is compiled in, they'd need to
wait for every USB device to finish init before the usbhid probe could complete.

--
Simon Arlott

2008-08-06 20:09:32

by Alan Stern

[permalink] [raw]
Subject: Re: [PATCH RFC] USB: Add HCD fastboot

On Wed, 6 Aug 2008, Arjan van de Ven wrote:

> On Wed, 6 Aug 2008 15:49:27 -0400 (EDT)
> Alan Stern <[email protected]> wrote:
>
> > On Wed, 6 Aug 2008, Simon Arlott wrote:
> >
> > > > Wouldn't it be much simpler, and less objectionable, to do what I
> > > > suggested earlier? That is, add a 5-second delay at the start of
> > > > hub_thread() in drivers/usb/core/hub.c. No messing with
> > > > Makefiles, no changes to the initcall scheduling.
> > >
> > > Aside from 5 seconds being too long, and anything less being a race
> > > between hub_thread() and driver initcalls, it doesn't improve
> > > anything because it'll still have to wait for the devices to finish
> > > initialising in userspace instead.
> >
> > Why is 5 seconds too long? Too long for what?
>
> because I'm done booting to full UI in that time.
>
> it's forever.

This doesn't answer my question. Why does the fact that you're already
in the UI after 5 seconds mean that 5 seconds is too long to delay the
start of khubd?

Besides, if your machine can boot to the UI in under 5 seconds then you
have no need to worry about improving boot-up speed anyway.

Alan Stern

2008-08-06 20:17:43

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [PATCH RFC] USB: Add HCD fastboot

On Wed, 6 Aug 2008 16:09:21 -0400 (EDT)
Alan Stern <[email protected]> wrote:

> On Wed, 6 Aug 2008, Arjan van de Ven wrote:
>
> > On Wed, 6 Aug 2008 15:49:27 -0400 (EDT)
> > Alan Stern <[email protected]> wrote:
> >
> > > On Wed, 6 Aug 2008, Simon Arlott wrote:
> > >
> > > > > Wouldn't it be much simpler, and less objectionable, to do
> > > > > what I suggested earlier? That is, add a 5-second delay at
> > > > > the start of hub_thread() in drivers/usb/core/hub.c. No
> > > > > messing with Makefiles, no changes to the initcall scheduling.
> > > >
> > > > Aside from 5 seconds being too long, and anything less being a
> > > > race between hub_thread() and driver initcalls, it doesn't
> > > > improve anything because it'll still have to wait for the
> > > > devices to finish initialising in userspace instead.
> > >
> > > Why is 5 seconds too long? Too long for what?
> >
> > because I'm done booting to full UI in that time.
> >
> > it's forever.
>
> This doesn't answer my question. Why does the fact that you're
> already in the UI after 5 seconds mean that 5 seconds is too long to
> delay the start of khubd?


because it means that while the rest of boot is done and the system
idle, the mouse and keyboard aren't going to work.

--
If you want to reach me at my work email, use [email protected]
For development, discussion and tips for power savings,
visit http://www.lesswatts.org

2008-08-06 20:26:53

by Alan Stern

[permalink] [raw]
Subject: Re: [PATCH RFC] USB: Add HCD fastboot

On Wed, 6 Aug 2008, Simon Arlott wrote:

> No, by adding a 5 second delay you're intending for the device driver initcalls
> to complete within that 5 seconds. If they take too long then the last one
> blocks everything (I realise that's ridiculous, these initcalls take <1ms when
> there are no devices yet). The best way to do is to make the driver initcalls
> before the host ones, like you suggested.

Doing the HCD initcalls last certainly ought to work.

> > "it'll still have to wait..." If by "it" you mean the initcall
> > thread, you're wrong. If by "it" you mean the user, you still aren't
> > necessarily correct; the user can do plenty of other things while
> > waiting for USB devices to initialize.
>
> Assuming userspace doesn't wait for all devices to settle and appear in /dev etc.
> before continuing.

Whatever that involves... /dev never truly settles; it's always
possible to plug in a new device or remove an old one.

> > I suppose you could make the hub_thread delay time a module parameter
> > for usbcore, defaulting to 0. Then it could be set by just the people
> > who want to use it -- many (most?) people keep their drivers in
> > modules, and it wouldn't do them any good.
>
> It really needs to have hcd initcalls done very early so that device init

Please stop using the word "it" with no antecedent! Do you mean "we"?

> has the rest of the (kernel and userspace) boot process to complete in the
> background. This is negated by having device drivers initialised immediately
> afterwards. Re-ordering initcalls and doing more of the init process
> asynchronously is likely to expose bugs and cause inconsistent device order
> on some systems, so if the makefile mess could be reduced then it can be a
> Kconfig option.

So what exactly do you recommend?

> How many people have *all* their USB components (hcd, drivers) as modules?

All the major distributions do, as far as I know. (I haven't actually
checked them all to be certain.)

> What do they do with their USB keyboards in the period between init and module
> load?

Probably nothing. What do you do on your keyboard while waiting for
system initialization to complete?

> If even one device driver and the hcd is compiled in, they'd need to
> wait for every USB device to finish init before the usbhid probe could complete.

What if usbhid is the one device driver that is compiled in? :-)

Alan Stern

2008-08-06 20:28:20

by Alan Stern

[permalink] [raw]
Subject: Re: [PATCH RFC] USB: Add HCD fastboot

On Wed, 6 Aug 2008, Arjan van de Ven wrote:

> > > > Why is 5 seconds too long? Too long for what?
> > >
> > > because I'm done booting to full UI in that time.
> > >
> > > it's forever.
> >
> > This doesn't answer my question. Why does the fact that you're
> > already in the UI after 5 seconds mean that 5 seconds is too long to
> > delay the start of khubd?
>
>
> because it means that while the rest of boot is done and the system
> idle, the mouse and keyboard aren't going to work.

Ah, okay. Then if the delay time were set up as a module parameter,
you'd be one of the people who leaves the parameter set at its default
value of 0.

Alan Stern

2008-08-06 21:50:33

by Simon Arlott

[permalink] [raw]
Subject: Re: [PATCH RFC] USB: Add HCD fastboot

On 06/08/08 21:26, Alan Stern wrote:
> On Wed, 6 Aug 2008, Simon Arlott wrote:
>
>> No, by adding a 5 second delay you're intending for the device driver initcalls
>> to complete within that 5 seconds. If they take too long then the last one
>> blocks everything (I realise that's ridiculous, these initcalls take <1ms when
>> there are no devices yet). The best way to do is to make the driver initcalls
>> before the host ones, like you suggested.
>
> Doing the HCD initcalls last certainly ought to work.

Right - so then all that's needed is to move usb/ before most other things that
take a while to init, and it has some time to initialise usb devices in the
background.

>> > "it'll still have to wait..." If by "it" you mean the initcall
>> > thread, you're wrong. If by "it" you mean the user, you still aren't
>> > necessarily correct; the user can do plenty of other things while
>> > waiting for USB devices to initialize.
>>
>> Assuming userspace doesn't wait for all devices to settle and appear in /dev etc.
>> before continuing.
>
> Whatever that involves... /dev never truly settles; it's always
> possible to plug in a new device or remove an old one.

Yes... I'm not sure how that part would work.

>> > I suppose you could make the hub_thread delay time a module parameter
>> > for usbcore, defaulting to 0. Then it could be set by just the people
>> > who want to use it -- many (most?) people keep their drivers in
>> > modules, and it wouldn't do them any good.
>>
>> It really needs to have hcd initcalls done very early so that device init
>
> Please stop using the word "it" with no antecedent! Do you mean "we"?

Yes. I'll try to avoid doing that.

>> has the rest of the (kernel and userspace) boot process to complete in the
>> background. This is negated by having device drivers initialised immediately
>> afterwards. Re-ordering initcalls and doing more of the init process
>> asynchronously is likely to expose bugs and cause inconsistent device order
>> on some systems, so if the makefile mess could be reduced then it can be a
>> Kconfig option.
>
> So what exactly do you recommend?

I'm not an expert on what can be done with the Makefile process, perhaps there's
an easier way to move things around based on a config option. If host init is
moved before device init for everyone, wouldn't there be too many side effects?

I've not tried to use usb-storage as root but presumably that works. If that
already uses a hack of making the kernel wait X seconds before trying to mount
root then it'll still work.

>> How many people have *all* their USB components (hcd, drivers) as modules?
>
> All the major distributions do, as far as I know. (I haven't actually
> checked them all to be certain.)
>
>> What do they do with their USB keyboards in the period between init and module
>> load?
>
> Probably nothing. What do you do on your keyboard while waiting for
> system initialization to complete?

Lack of a keyboard makes it impossible to do anything if the module fails to
load... as I understand it when the HCD init runs, any BIOS emulation stops?
Although if HCD is a module too...

>> If even one device driver and the hcd is compiled in, they'd need to
>> wait for every USB device to finish init before the usbhid probe could complete.
>
> What if usbhid is the one device driver that is compiled in? :-)

That was the situation I was thinking of - surely that would be compiled in
if the HCDs were?

--
Simon Arlott

2008-08-06 22:35:07

by Alan Stern

[permalink] [raw]
Subject: Re: [PATCH RFC] USB: Add HCD fastboot

On Wed, 6 Aug 2008, Simon Arlott wrote:

> > Doing the HCD initcalls last certainly ought to work.
>
> Right - so then all that's needed is to move usb/ before most other things that
> take a while to init, and it has some time to initialise usb devices in the
> background.

Not exactly -- you should move most of usb/ forward so that it comes
before usb/host/. Or alternatively move usb/host/ later so that it
comes after the rest of usb/.

> > So what exactly do you recommend?
>
> I'm not an expert on what can be done with the Makefile process, perhaps there's
> an easier way to move things around based on a config option. If host init is
> moved before device init for everyone, wouldn't there be too many side effects?

Doesn't the host init _already_ come before device init? If host init
were moved _after_ device init, I don't think there would be a lot of
side effects. But it's hard to be sure without trying it.

> Lack of a keyboard makes it impossible to do anything if the module fails to
> load... as I understand it when the HCD init runs, any BIOS emulation stops?

That's right. The host driver kicks the BIOS off the hardware.

> Although if HCD is a module too...
>
> >> If even one device driver and the hcd is compiled in, they'd need to
> >> wait for every USB device to finish init before the usbhid probe could complete.
> >
> > What if usbhid is the one device driver that is compiled in? :-)
>
> That was the situation I was thinking of - surely that would be compiled in
> if the HCDs were?

I noticed in your logs earlier that some of your USB drivers were
compiled in and others weren't. Maybe if _all_ of them were compiled
in you wouldn't experience as much contention and the init delays would
be shorter.

Alan Stern

2008-08-06 22:53:32

by Simon Arlott

[permalink] [raw]
Subject: Re: [PATCH RFC] USB: Add HCD fastboot

On 06/08/08 23:34, Alan Stern wrote:
> On Wed, 6 Aug 2008, Simon Arlott wrote:
>
>> > Doing the HCD initcalls last certainly ought to work.
>>
>> Right - so then all that's needed is to move usb/ before most other things that
>> take a while to init, and it has some time to initialise usb devices in the
>> background.
>
> Not exactly -- you should move most of usb/ forward so that it comes
> before usb/host/. Or alternatively move usb/host/ later so that it
> comes after the rest of usb/.

Well, putting usb/ before net/ etc. requires that usb/host/ is the usb/stuff/ or
they'll all wait until they can probe for devices.

>> > So what exactly do you recommend?
>>
>> I'm not an expert on what can be done with the Makefile process, perhaps there's
>> an easier way to move things around based on a config option. If host init is
>> moved before device init for everyone, wouldn't there be too many side effects?
>
> Doesn't the host init _already_ come before device init? If host init
> were moved _after_ device init, I don't think there would be a lot of
> side effects. But it's hard to be sure without trying it.

Host init is before device init, as that's the Makefile link order. Any device
init causes it to wait for *all* devices, so swapping them around means devices
are going to appear at any time after that - there's no device initcall to make
it block. Presumably it would be possible to have a late_initcall (which would
be early in that list if usb was earlier) that could ensure khubd had finished
[its current queue] before continuing - as if there was a device driver initcall?

If someone currently has HCD init compiled in but nothing else, then the boot
process would block unnecessarily... the initcall would need to be disabled
in that case to maintain existing behaviour - which is why it probably needs
to be a config option, which requires some mess in the Makefile or a new
initcall type.

>> Lack of a keyboard makes it impossible to do anything if the module fails to
>> load... as I understand it when the HCD init runs, any BIOS emulation stops?
>
> That's right. The host driver kicks the BIOS off the hardware.

It seems like there's always going to be a stage where there's no keyboard...

>> Although if HCD is a module too...
>>
>> >> If even one device driver and the hcd is compiled in, they'd need to
>> >> wait for every USB device to finish init before the usbhid probe could complete.
>> >
>> > What if usbhid is the one device driver that is compiled in? :-)
>>
>> That was the situation I was thinking of - surely that would be compiled in
>> if the HCDs were?
>
> I noticed in your logs earlier that some of your USB drivers were
> compiled in and others weren't. Maybe if _all_ of them were compiled
> in you wouldn't experience as much contention and the init delays would
> be shorter.

I don't know where you got that from - they're definitely all compiled in.
Whichever is first to have its initcall run is blocked, and the logs may have
been from a test of that.

--
Simon Arlott

2008-08-07 03:30:16

by David Brownell

[permalink] [raw]
Subject: Re: [PATCH RFC] USB: Add HCD fastboot

> > Lack of a keyboard makes it impossible to do anything if the module fails to
> > load... as I understand it when the HCD init runs, any BIOS emulation stops?
>
> That's right. ?The host driver kicks the BIOS off the hardware.

That's done much earlier ... as part of PCI quirk handling.
See drivers/usb/host/pci-quirks.c ... it's not been part of
the HCDs for about three years now. I don't recall the details
right now, but letting the BIOS hang on to that hardware was
causing a lot of wierd problems that went away when we made
that change.

If a module fails to load, fix the bug in that module. :)

- Dave

2008-08-07 03:34:57

by David Brownell

[permalink] [raw]
Subject: Re: [PATCH RFC] USB: Add HCD fastboot

On Wednesday 06 August 2008, Simon Arlott wrote:
> +ifndef CONFIG_USB_FASTBOOT
> + ?obj-y????????????????????????????????+= $(tmp-y)
> +endif
> ?
> ?obj-$(CONFIG_USB_ACM)??????????+= class/
> ?obj-$(CONFIG_USB_PRINTER)??????+= class/
> @@ -34,3 +40,7 @@ obj-$(CONFIG_USB)?????????????+= misc/
> ?
> ?obj-$(CONFIG_USB_ATM)??????????+= atm/
> ?obj-$(CONFIG_USB_SPEEDTOUCH)???+= atm/
> +
> +ifdef CONFIG_USB_FASTBOOT
> + ?obj-y????????????????????????????????+= $(tmp-y)
> +endif

If you want to do such stuff, don't be conditional.
Just change it so USB is *always* earlier.

We really don't need to make boot sequences become
even harder to understand/predict.

2008-08-07 13:36:59

by deloptes

[permalink] [raw]
Subject: Re: [PATCH RFC] USB: Add HCD fastboot

Hi,

I'm using usb drive to boot from. I also use USB mouse and USB keyboard.

Indeed I have to put a delay in the init script in the initrd to delay and give time for the initialization. I found out by testing that this delay should be >5sec.

I have tried compiling all the usb modules into the kernel, but this doesn't help much, now after following this discussion it might be related.

With 2.6.20 I can use the keyboard. With the same init script in 2.6.24 and 2.6.26 it is not working, but I really coudn't find time to investigate, because as Alan sais there is really not too much to do with keyboard and mouse during the bootprocess unless as I do I unlock crypted partitions.

Don't know if this information is somehow useful for you, but hope so.

I could also look at why the keyboard is not working in boot. Let me know

regards


--- On Wed, 8/6/08, Simon Arlott <[email protected]> wrote:

> From: Simon Arlott <[email protected]>
> Subject: Re: [PATCH RFC] USB: Add HCD fastboot
> To: "Alan Stern" <[email protected]>
> Cc: "Rene Herman" <[email protected]>, "Arjan van de Ven" <[email protected]>, [email protected], [email protected], "Daniel Walker" <[email protected]>, "USB list" <[email protected]>, "Greg Kroah-Hartman" <[email protected]>
> Date: Wednesday, August 6, 2008, 11:49 PM
> On 06/08/08 21:26, Alan Stern wrote:
> > On Wed, 6 Aug 2008, Simon Arlott wrote:
> >
> >> No, by adding a 5 second delay you're
> intending for the device driver initcalls
> >> to complete within that 5 seconds. If they take
> too long then the last one
> >> blocks everything (I realise that's
> ridiculous, these initcalls take <1ms when
> >> there are no devices yet). The best way to do is
> to make the driver initcalls
> >> before the host ones, like you suggested.
> >
> > Doing the HCD initcalls last certainly ought to work.
>
> Right - so then all that's needed is to move usb/
> before most other things that
> take a while to init, and it has some time to initialise
> usb devices in the
> background.
>
> >> > "it'll still have to wait..."
> If by "it" you mean the initcall
> >> > thread, you're wrong. If by
> "it" you mean the user, you still aren't
> >> > necessarily correct; the user can do plenty
> of other things while
> >> > waiting for USB devices to initialize.
> >>
> >> Assuming userspace doesn't wait for all
> devices to settle and appear in /dev etc.
> >> before continuing.
> >
> > Whatever that involves... /dev never truly settles;
> it's always
> > possible to plug in a new device or remove an old one.
>
> Yes... I'm not sure how that part would work.
>
> >> > I suppose you could make the hub_thread delay
> time a module parameter
> >> > for usbcore, defaulting to 0. Then it could
> be set by just the people
> >> > who want to use it -- many (most?) people
> keep their drivers in
> >> > modules, and it wouldn't do them any
> good.
> >>
> >> It really needs to have hcd initcalls done very
> early so that device init
> >
> > Please stop using the word "it" with no
> antecedent! Do you mean "we"?
>
> Yes. I'll try to avoid doing that.
>
> >> has the rest of the (kernel and userspace) boot
> process to complete in the
> >> background. This is negated by having device
> drivers initialised immediately
> >> afterwards. Re-ordering initcalls and doing more
> of the init process
> >> asynchronously is likely to expose bugs and cause
> inconsistent device order
> >> on some systems, so if the makefile mess could be
> reduced then it can be a
> >> Kconfig option.
> >
> > So what exactly do you recommend?
>
> I'm not an expert on what can be done with the Makefile
> process, perhaps there's
> an easier way to move things around based on a config
> option. If host init is
> moved before device init for everyone, wouldn't there
> be too many side effects?
>
> I've not tried to use usb-storage as root but
> presumably that works. If that
> already uses a hack of making the kernel wait X seconds
> before trying to mount
> root then it'll still work.
>
> >> How many people have *all* their USB components
> (hcd, drivers) as modules?
> >
> > All the major distributions do, as far as I know. (I
> haven't actually
> > checked them all to be certain.)
> >
> >> What do they do with their USB keyboards in the
> period between init and module
> >> load?
> >
> > Probably nothing. What do you do on your keyboard
> while waiting for
> > system initialization to complete?
>
> Lack of a keyboard makes it impossible to do anything if
> the module fails to
> load... as I understand it when the HCD init runs, any BIOS
> emulation stops?
> Although if HCD is a module too...
>
> >> If even one device driver and the hcd is compiled
> in, they'd need to
> >> wait for every USB device to finish init before
> the usbhid probe could complete.
> >
> > What if usbhid is the one device driver that is
> compiled in? :-)
>
> That was the situation I was thinking of - surely that
> would be compiled in
> if the HCDs were?
>
> --
> Simon Arlott
> --
> 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


2008-08-07 14:15:02

by Alan Stern

[permalink] [raw]
Subject: Re: [PATCH RFC] USB: Add HCD fastboot

On Wed, 6 Aug 2008, Simon Arlott wrote:

> > Not exactly -- you should move most of usb/ forward so that it comes
> > before usb/host/. Or alternatively move usb/host/ later so that it
> > comes after the rest of usb/.
>
> Well, putting usb/ before net/ etc. requires that usb/host/ is the usb/stuff/ or
> they'll all wait until they can probe for devices.

I don't understand.

I never said you should move usb/. I said you should move usb/host/
after all the rest of usb/*/.

> > Doesn't the host init _already_ come before device init? If host init
> > were moved _after_ device init, I don't think there would be a lot of
> > side effects. But it's hard to be sure without trying it.
>
> Host init is before device init, as that's the Makefile link order. Any device
> init causes it to wait for *all* devices,

"wait" is the wrong word. Each device init (more accurately, each
device driver init) probes all the USB devices. But it doesn't have to
wait if nothing else is probing those devices concurrently and if no
hub activity is going on.

This means that you won't save much time by running multiple USB device
driver initialization threads. Better to do all those inits in a
single thread.

The ideal situation is to have each device driver initcall run when
there are _no_ USB devices to be probed -- i.e., before the host
drivers have been initialized.

> so swapping them around means devices
> are going to appear at any time after that - there's no device initcall to make
> it block.
>
> Presumably it would be possible to have a late_initcall (which would
> be early in that list if usb was earlier) that could ensure khubd had finished
> [its current queue] before continuing - as if there was a device driver initcall?
>
> If someone currently has HCD init compiled in but nothing else, then the boot
> process would block unnecessarily... the initcall would need to be disabled
> in that case to maintain existing behaviour - which is why it probably needs
> to be a config option, which requires some mess in the Makefile or a new
> initcall type.

Again, I don't understand. What's wrong with simply reordering
drivers/usb/Makefile so that all the host/ entries come at the end?

> >> Lack of a keyboard makes it impossible to do anything if the module fails to
> >> load... as I understand it when the HCD init runs, any BIOS emulation stops?
> >
> > That's right. The host driver kicks the BIOS off the hardware.

As David Brownell pointed out, I was wrong about this. The BIOS gets
kicked off even before the host driver starts running, as part of PCI
initialization.

> It seems like there's always going to be a stage where there's no keyboard...

Yes.

> > I noticed in your logs earlier that some of your USB drivers were
> > compiled in and others weren't. Maybe if _all_ of them were compiled
> > in you wouldn't experience as much contention and the init delays would
> > be shorter.
>
> I don't know where you got that from - they're definitely all compiled in.
> Whichever is first to have its initcall run is blocked, and the logs may have
> been from a test of that.

I must have misinterpreted one of the earlier messages in this thread.

Alan Stern

2008-08-07 16:48:23

by Alan Stern

[permalink] [raw]
Subject: Re: [PATCH RFC] USB: Add HCD fastboot

On Thu, 7 Aug 2008, Emanoil Kotsev wrote:

> Hi,
>
> I'm using usb drive to boot from. I also use USB mouse and USB keyboard.
>
> Indeed I have to put a delay in the init script in the initrd to
> delay and give time for the initialization. I found out by testing
> that this delay should be >5sec.

This is controlled by a module parameter. Read the description of
delay_use near the top of drivers/usb/storage/usb.c.

Alan Stern

2008-08-08 09:24:33

by Rene Herman

[permalink] [raw]
Subject: Re: [PATCH RFC] USB: Add HCD fastboot

On 06-08-08 20:40, Simon Arlott wrote:

>>> Ok - so there could be some big improvements to be had by making the
>>> hcd init happen as early as possible and the device initcalls later?
>>
>> Maybe. Perhaps a better approach would be to make the device driver
>> initcalls before there are any devices for their probe routines to
>> block on.
>
> What about this?
>
> The Makefiles become a bit messy, but by moving things around I get the
> desired effect without splitting their initcalls.
>
> [ 7.941890] Write protecting the kernel read-only data: 5656k
> [ 5.437709] Write protecting the kernel read-only data: 5656k
>
> 2.5s faster, which is almost half the boot time.
>
> Signed-off-by: Simon Arlott <[email protected]>

Interesting... I haven't been able to get stable improvements, with all
USB kernel modules built-in, just a USB mouse on OHCI and otherwise a
switched of USB HD on EHCI and usb-storage, but I do get the idea that
with your parch, I "get lucky" more often.

I have knfsd as a module and it loads through the exportfs trigger
during bootup and outputs the last message to my dmesg:

4 boots without your patch: 6.12, 6.41, 6.38, 6.34 seconds
4 boots with : 5.39, 6.33, 5.37, 6.34

Booting with the external HD switched on adds a tiny bit to the actual
kernel startup time -- completely repeatable 2.62 seconds until freeing
init code with the HD off versus 2.73 with it on -- but doesn't seem to
change the picture otherwise...

Would those results make sense you feel? I'm not looking forward to
putting an actual statistical analysis on it ;-)

Arjan: your fastboot repo by the way doesn't pull cleanly into current
upstream anymore.

Rene.

2008-08-08 11:29:54

by Simon Arlott

[permalink] [raw]
Subject: Re: [PATCH RFC] USB: Add HCD fastboot

On Fri, August 8, 2008 10:24, Rene Herman wrote:
> On 06-08-08 20:40, Simon Arlott wrote:
>>>> Ok - so there could be some big improvements to be had by making the
>>>> hcd init happen as early as possible and the device initcalls later?
>>>
>>> Maybe. Perhaps a better approach would be to make the device driver
>>> initcalls before there are any devices for their probe routines to
>>> block on.
>>
>> What about this?
>>
>> The Makefiles become a bit messy, but by moving things around I get the
>> desired effect without splitting their initcalls.
>>
>> [ 7.941890] Write protecting the kernel read-only data: 5656k
>> [ 5.437709] Write protecting the kernel read-only data: 5656k
>>
>> 2.5s faster, which is almost half the boot time.
>>
>> Signed-off-by: Simon Arlott <[email protected]>
>
> Interesting... I haven't been able to get stable improvements, with all
> USB kernel modules built-in, just a USB mouse on OHCI and otherwise a
> switched of USB HD on EHCI and usb-storage, but I do get the idea that
> with your parch, I "get lucky" more often.

It does depend on what else is running... I have e1000 (500ms) and
sata_nv (2200ms) so there's plenty of additional time for usb devices
to finish initialising.

> I have knfsd as a module and it loads through the exportfs trigger
> during bootup and outputs the last message to my dmesg:
>
> 4 boots without your patch: 6.12, 6.41, 6.38, 6.34 seconds
> 4 boots with : 5.39, 6.33, 5.37, 6.34
>
> Booting with the external HD switched on adds a tiny bit to the actual
> kernel startup time -- completely repeatable 2.62 seconds until freeing
> init code with the HD off versus 2.73 with it on -- but doesn't seem to
> change the picture otherwise...

usb-storage by default delays 2 seconds before trying to use the device,
so it won't add much time itself if there's already an driver initcall
trying probe for devices it supports.

> Would those results make sense you feel? I'm not looking forward to
> putting an actual statistical analysis on it ;-)

I suggest adding initcall_debug=1, it'll show how long the usb and other
initcalls take to run. You should see the first usb device driver initcall
take a second or two to run (without the patch). Maybe I'm just badly
affected by having HCDs with 10 ports (of which only 4 are physically
usable...).

dmesg|grep initcall\
|sed -e 's/.*initcall \([^ ]\+\( \(\[[^]]\+\]\)\)\?\) returned [^ ]\+ after \([^ ]\+\) secs/\4 \1/'\
|sort -n

--
Simon Arlott

2008-08-08 14:30:41

by Rene Herman

[permalink] [raw]
Subject: Re: [PATCH RFC] USB: Add HCD fastboot

On 08-08-08 13:29, Simon Arlott wrote:

> I suggest adding initcall_debug=1, it'll show how long the usb and other
> initcalls take to run. You should see the first usb device driver initcall
> take a second or two to run (without the patch). Maybe I'm just badly
> affected by having HCDs with 10 ports (of which only 4 are physically
> usable...).
>
> dmesg|grep initcall\
> |sed -e 's/.*initcall \([^ ]\+\( \(\[[^]]\+\]\)\)\?\) returned [^ ]\+ after \([^ ]\+\) secs/\4 \1/'\
> |sort -n

Thanks, but that specific expression doesn't work work me,since there's
not a single one that takes ' secs', only ' msecs'.

Here's the USB related ones here:

> [ 1.198751] initcall ehci_hcd_init+0x0/0x11 returned 0 after 123 msecs
> [ 1.818688] initcall ohci_hcd_mod_init+0x0/0x31 returned 0 after 591 msecs
> [ 2.537756] initcall usb_stor_init+0x0/0x32 returned 0 after 685 msecs

[ ... ]

> [ 2.619477] initcall hid_init+0x0/0x3 returned 0 after 0 msecs
> [ 2.629667] initcall hid_init+0x0/0x3e returned 0 after 9 msecs

685 isn't nice but better then 2000...

Rene.