A better way to sequence driver initialization?

[Grant Likely]

On Fri, Apr 9, 2010 at 1:23 PM, Bill Gatliff <bgat at billgatliff.com> wrote:
> Guys:
>
>
> My recent post, "Requesting a GPIO that hasn't been registered yet", and
> Anton's reply thereto (thanks, Anton!) on linuxppc-dev got me thinking
> about the problem of dependencies between devices in different  classes,
> and/or between drivers/devices in general.  I'd like to float an idea to
> see if it's worth pursuing.
>
> Changing the link order to get drivers to initialize in the right order
> will always be a problem for someone--- the order will be right for some
> people, but exactly wrong for others.  And the problem is made worse for
> Device Tree-based systems, where just about everything including IRQ
> descriptors are created on a demand and/or as-available basis.  What if
> we let the kernel sort those dependencies out for us, at runtime?  I
> think it's possible, and I can't be the only one who would like to see
> this happen.
>
> There are two parts to my idea for a solution.  First part is to modify
> do_initcalls() so that it launches each initcall function in its own
> kernel thread.  Wait, don't panic yet!

Is initcall the right granularity?  Shouldn't it be that each .probe()
hook gets its own thread?  Otherwise one device missing its resources
could block another device using the same driver (whose resources are
available) from probing.  Regardless, parallel probe has be attempted
and failed before.  There are lots of fiddly bits to get right.

In fact, there *used* to be code in the kernel that does exactly that.
 It was put in 2.6.20, but removed in 2.6.21-rc1.  Here's the relevant
commits, and a very interesting thread discussing the issues:

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=21c7f30b1d3f8a3de3128478daca3ce203fc8733
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=5adc55da4a7758021bcc374904b0f8b076508a11
http://lkml.indiana.edu/hypermail/linux/kernel/0705.1/0205.html

It is pointed out in that thread that a big part of the problem is
that a large number of drivers in the tree just aren't safe for
multithreaded probing which is kind of a showstopper.  Now, maybe
doing it at the initcall level makes it less scary and more sane, but
I suspect that it will still expose a lot of broken code that assumes
things are already set up because it has always been that way.

> Second, we create/modify functions like gpio_request() so that if they
> fail, they can optionally stop in a wait queue until the call could
> succeed.  Then gpiochip_add() would signal that queue each time a new
> gpiochip was added. Functions like request_irq() and clk_get()--- and
> probably many others--- would do the same thing, but with their own wait
> queues.

You could take a look at the bus notifier code too.  I made an attempt
at solving a similar problem between Ethernet MAC drivers and binding
against the correct phy.  I ended up solving the problem in a
different way, but the experimenting I did convince me that it is a
viable concept.

Basically, if the driver depends on another resource/device then it
can register a callback to be notified when a new device becomes
available.  Once the needed device shows up (or immediately if the
device is already there), the driver gets called back and it can
complete registration.

Not quite the same model that you are talking about here, but it would
solve the problem on a per-driver basis.

> Something like this:
>
>    static wait_queue_head_t requestor_wq;
>
>    int gpiochip_add(struct gpio_chip *chip)
>    {
>        ...
>        wake_up_interruptible(&requestor_wq);
>        return status;
>    }
>
>    int gpio_request_wait(unsigned gpio, const char *label)
>    {
>        return wait_event_interruptible(&requestor_wq,
> !gpio_request(gpio, label));
>    }
>
>
> Or we might want to make the above code be the actual gpio_request(),
> and bury the wait_event_interruptible() inside of that.  Doing so would
> prevent having to touch a lot of code, but would definitely change the
> behavior of gpio_request().  Not sure which approach is best.
>
> Finally, I think that do_initcalls() would turn into something like this:
>
>    static void __init do_initcalls(void)
>    {
>            initcall_t *fn;
>
>            for (fn = __early_initcall_end; fn < __initcall_end; fn++)
>                    kthread_create(do_one_initcall, *fn, "do_initcalls");
>
>            flush_scheduled_work();
>    }
>
>
> If someone doesn't tell me this is a stupid idea, I might post it to
> lkml.  Now's your chance!  :)

Have you dug into the Arjan's asynchronous function call infrastructure?

http://lwn.net/Articles/314808/

g.