lkml.org 
[lkml]   [2008]   [Oct]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [git pull] fastboot tree for v2.6.28


On Fri, 10 Oct 2008, Ingo Molnar wrote:
>
> Please pull the latest fastboot-v28-for-linus git tree from:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git fastboot-v28-for-linus
>
> the (opt-in) fastboot async bootup feature, as described by Arjan in his
> v2.6.28 announcement:
>
> http://lwn.net/Articles/299591/
>
> tested and maintained in -tip because tip/tracing embedd-merges this
> topic. (for the fastboot tracer)

Ok, so I finally looked at the patch, I quite frankly, I think it's
fundamentally wrong.

This is just an example of the wrongness:

-module_init(uhci_hcd_init);
+module_init_async(uhci_hcd_init);
-module_init(ehci_hcd_init);
+module_init_async(ehci_hcd_init);
-module_init(ohci_hcd_mod_init);
+module_init_async(ohci_hcd_mod_init);

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

because there is absolutely _no_ excuse for doing the PCI part of the
probing asynchronously.

The "ohci_hcd_mod_init" part is _especially_ dangerous, because clearly
nobody looked at the OHCI setup sequence, which includes a lot of odd
devices and not all of them at all PCI-related etc. Making it asynchronous
is not safe, nor is it appropriate.

The fact is, those things all have one thing in common:

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

There are other devices too that may need synchronous core hub discovery
(eg the actual controller), but that want to do the "devices hanging off
this controller" asynchronously. Disks come to mind. That shouldn't mean
that the IDE driver discovery should be asynchronous, it should just mean
that the driver has some simple way to execute something asynchronously.

In other words, I really think it's very wrong to make that
"async_init_wq" be somethign that is internal to do_initcalls. It should
be a workqueue that the initcalls can _choose_ to use at the appropriate
level (which may be deeper down, like 'usb_add_hcd()'), not be forced to
use at the outermost one.

You can try to convince me otherwise, but I really do think this patch is
fundamentally the wrong approach.

Linus


\
 
 \ /
  Last update: 2008-10-10 22:13    [from the cache]
©2003-2014 Jasper Spaans. Advertise on this site