A better way to sequence driver initialization?

A better way to sequence driver initialization?

[Bill Gatliff]

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!

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.

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!  :)


b.g.

-- 
Bill Gatliff
bgat@billgatliff.com

Re: A better way to sequence driver initialization?

[Bill Gatliff]

Bill Gatliff wrote:
> If someone doesn't tell me this is a stupid idea, I might post it to
> lkml.  Now's your chance!  :)
>   

So I went ahead and tried it anyway:


$ git diff init/main.c
diff --git a/init/main.c b/init/main.c
index dac44a9..1461d09 100644
--- a/init/main.c
+++ b/init/main.c
@@ -753,7 +753,11 @@ static void __init do_initcalls(void)
        initcall_t *fn;

        for (fn = __early_initcall_end; fn < __initcall_end; fn++)
+#if 1
+               kthread_run(do_one_initcall, *fn, "do_initcalls");
+#else
                do_one_initcall(*fn);
+#endif

        /* Make sure there is no pending stuff from the initcall sequence */
        flush_scheduled_work();

Interestingly, things didn't blow up as quickly as I had expected they
would.  :)

Booting with initcall_debug=1 both with and without the kthread_run()
mod has proven insightful.  The first OOPS I get with the mod looks like
this:

...
calling  ch_init+0x0/0x18 @ 325
Unable to handle kernel NULL pointer dereference at virtual address
00000000
pgd = c0004000
[00000000] *pgd=00000000 
Internal error: Oops: 5 [#4] PREEMPT 
last sysfs file:
Modules linked in: 
CPU: 0 Tainted: GD  (2.6.33-rc5-csb737-00010-gaba040a-dirty #59)
PC is at kset_find_obj+0x18/0x98
LR is at kset_find_obj+0x18/0x98
pc : [<c018156c>] lr : [<c018156c>] psr: 20000013
sp : c3937f68  ip : c3937f68  fp : 00000000
r10: 00000000  r9 : 00000000  r8 : 00000000
r7 : c0396a6f  r6 : 00000000  r5 : 00000005  r4 : c03f03b0
r3 : 00000003  r2 : c3936000  r1 : 00000000  r0 : 00000001
Flags: nzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
Control: 0005317f  Table: 20004000  DAC: 00000017
Process do_initcalls (pid: 325, stack limit = 0xc3936270) 
Stack: (0xc3937f68 to 0xc3938000) 
7f60: c03fc050 c03f03b0 00000005 c03f03b0 00000000 c01c471c
7f80: c03f03b0 00000005 c001e62c 00000000 00000000 c024007c 00000000
1b58d32c
7fa0: 00000005 c002c39c c381bfac c001e62c 1b58d32c 00000005 00000000
c3937fd4
7fc0: c381bfac c001e62c c002c340 c005cb3c 00000000 00000000 c3937fd8
c3937fd8
7fe0: 00000000 00000000 00000000 00000000 00000000 c002df78 00000000
00000000
[<c018156c>] (kset_find_obj+0x18/0x98) from [<c01c471c>]
(driver_register+0x88/0x150)
[<c01c471c>] (driver_register+0x88/0x150) from [<c024007c>]
(__hid_register_driver+0x38/0x70) 
[<c024007c>] (__hid_register_driver+0x38/0x70) from [<c002c39c>]
(do_one_initcall+0x5c/0x1c4) 
[<c002c39c>] (do_one_initcall+0x5c/0x1c4) from [<c005cb3c>]
(kthread+0x78/0x80)
[<c005cb3c>] (kthread+0x78/0x80) from [<c002df78>]
(kernel_thread_exit+0x0/0x8)
Code: e24dd004 e3a00001 e1a07001 ebfaf76c (e5963000)
---[ end trace f9bb68fd55862436 ]--- 
note: do_initcalls[325] exited with preempt_count 1
...

There is a ch_init() function in drivers/hid/hid-cherry.c.  I think that
function is calling hid_register_driver() before hid_init() has set up
hid_bus_type.  That causes kset_find_obj() to die because the kset
structure it gets passed in ch_init() refers to a bus that doesn't exist
yet.  It's a plain 'ole initialization race.

Maybe there are fewer places that would need wait queues than I
originally thought!  At least for drivers that use the device API,
kset_find_obj() might be the only place that needs to wait until a
device or bus appears.  If I get a second next week, I might hack one in
there and see how much farther I get...


b.g.

-- 
Bill Gatliff
bgat@billgatliff.com

Re: A better way to sequence driver initialization?

[Bill Gatliff]

Bill Gatliff wrote:
> Maybe there are fewer places that would need wait queues than I
> originally thought!  At least for drivers that use the device API,
> kset_find_obj() might be the only place that needs to wait until a
> device or bus appears.

... and kset_register() might be the only place that calls
wake_up_interruptible().  Wow.


b.g.

-- 
Bill Gatliff
bgat@billgatliff.com

Re: A better way to sequence driver initialization?

[Bill Gatliff]

Bill Gatliff wrote:
>
> ... and kset_register() might be the only place that calls
> wake_up_interruptible().  Wow.
>   

Wow, indeed.  With just the attached patch, I get exactly one OOPS
now--- in ssc_free(), which I think is getting called because
atmel_ssc_modinit() is racing with something.

I wasn't able to get all the way to mounting my NFS rootfs, I think
because I'm getting to ip_auto_config() before my ethernet controller
has registered!  :)


$ git diff lib/kobject.c
diff --git a/lib/kobject.c b/lib/kobject.c
index b512b74..e75556c 100644
--- a/lib/kobject.c
+++ b/lib/kobject.c
@@ -18,6 +18,10 @@
 #include <linux/stat.h>
 #include <linux/slab.h>

+#include <linux/sched.h>
+#include <linux/wait.h>
+DECLARE_WAIT_QUEUE_HEAD(kset_wait);
+
 /*
  * populate_dir - populate directory with attributes.
  * @kobj: object we're working on.
@@ -721,6 +725,7 @@ int kset_register(struct kset *k)
        if (err)
                return err;
        kobject_uevent(&k->kobj, KOBJ_ADD);
+       wake_up_interruptible(&kset_wait);
        return 0;
 }

@@ -744,7 +749,7 @@ void kset_unregister(struct kset *k)
  * looking for a matching kobject. If matching object is found
  * take a reference and return the object.
  */
-struct kobject *kset_find_obj(struct kset *kset, const char *name)
+struct kobject *__kset_find_obj(struct kset *kset, const char *name)
 {
        struct kobject *k;
        struct kobject *ret = NULL;
@@ -760,6 +765,14 @@ struct kobject *kset_find_obj(struct kset *kset,
const char *name)
        return ret;
 }

+struct kobject *kset_find_obj(struct kset *kset, const char *name)
+{
+       struct kobject *ret = NULL;
+       wait_event_interruptible(kset_wait, (ret = __kset_find_obj(kset,
name)));
+       return ret;
+}
+
 static void kset_release(struct kobject *kobj)
 {
        struct kset *kset = container_of(kobj, struct kset, kobj);


b.g.

-- 
Bill Gatliff
bgat@billgatliff.com

Re: A better way to sequence driver initialization?

[Grant Likely]

On Fri, Apr 9, 2010 at 1:23 PM, Bill Gatliff <bgat@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.

Re: A better way to sequence driver initialization?

[Bill Gatliff]

Grant Likely wrote:
> On Fri, Apr 9, 2010 at 1:23 PM, Bill Gatliff <bgat@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.

Good point, one that I missed.  Yea, I guess I really want multithreaded
probing.

> Regardless, parallel probe has be attempted and failed before.  There are lots of fiddly bits to get right.
>   

Yep.  I wasnt aware of any specific attempts before, but I did suspect
that there were fiddly bits lurking about.  :)

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

Yep.

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

Yea, but it's the per-driver part that I'm trying to avoid.


> Have you dug into the Arjan's asynchronous function call infrastructure?
>
> http://lwn.net/Articles/314808/
>   

No, but I think I will now.  :)


b.g.

-- 
Bill Gatliff
bgat@billgatliff.com

Re: A better way to sequence driver initialization?

[Benjamin Herrenschmidt]

On Fri, 2010-04-09 at 14:23 -0500, Bill Gatliff wrote:
> 
> 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. 

I'd rather do things a bit more explicitely and less prone to break
existing stuff... something along the lines of, first defining a variant
of the initcalls to enable that "multithreaded" stuff, along with an
explicit wait_for_service("subsys:hid"); for example.

One could also expose service deps via the module info, thus delaying
module init, or things like that (in fact, initcalls could even come
with a list of dependencies).

Cheers,
Ben.

Re: A better way to sequence driver initialization?

[Bill Gatliff]

Benjamin Herrenschmidt wrote:
> On Fri, 2010-04-09 at 14:23 -0500, Bill Gatliff wrote:
>   
>> 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. 
>>     
>
> I'd rather do things a bit more explicitely and less prone to break
> existing stuff... something along the lines of, first defining a variant
> of the initcalls to enable that "multithreaded" stuff, along with an
> explicit wait_for_service("subsys:hid"); for example.
>
> One could also expose service deps via the module info, thus delaying
> module init, or things like that (in fact, initcalls could even come
> with a list of dependencies).
>   

The general problem with your approach is that the module in question
might not know what services it needs to wait for.

Specific to my situation, the gpio-led code doesn't have any way of
knowing that it needs to wait until my pca953x i2c devices have all been
installed so that the gpio pin I have specified even exists.  And short
of setting up some kind of table in the board-specific code (or device
tree, actually), I don't know how to communicate such a dependency
without touching the generic gpio-led code--- which I'm trying to avoid.

I just want gpio-led to try again if the gpio pin it has been provided
can't be requested yet.  And I can see the need for similar behavior in
several other of my drivers, hence the desire to generalize things a
bit.  I'm pretty sure my approach is only half-baked, but it does seem
to avoid the need to touch a bunch of existing code--- especially to add
board-specific dependencies.  It just give the system a tool to sort
things out on its own.

I do think your overall criticism is valid, though.


b.g.

-- 
Bill Gatliff
bgat@billgatliff.com

Re: A better way to sequence driver initialization?

[Paul Mundt]

On Sat, Apr 10, 2010 at 08:35:41AM -0500, Bill Gatliff wrote:
> Benjamin Herrenschmidt wrote:
> > On Fri, 2010-04-09 at 14:23 -0500, Bill Gatliff wrote:
> >   
> >> 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. 
> >>     
> >
> > I'd rather do things a bit more explicitely and less prone to break
> > existing stuff... something along the lines of, first defining a variant
> > of the initcalls to enable that "multithreaded" stuff, along with an
> > explicit wait_for_service("subsys:hid"); for example.
> >
> > One could also expose service deps via the module info, thus delaying
> > module init, or things like that (in fact, initcalls could even come
> > with a list of dependencies).
> >   
> 
> The general problem with your approach is that the module in question
> might not know what services it needs to wait for.
> 
> Specific to my situation, the gpio-led code doesn't have any way of
> knowing that it needs to wait until my pca953x i2c devices have all been
> installed so that the gpio pin I have specified even exists.  And short
> of setting up some kind of table in the board-specific code (or device
> tree, actually), I don't know how to communicate such a dependency
> without touching the generic gpio-led code--- which I'm trying to avoid.
> 
This is a common problem for drivers that are all stuck on the same
initcall level and as a result are entirely dependent on link order. Some
more intelligent logic via the bus notifiers would help with some of
this, but it wouldn't help with drivers that are effectively unable to
probe for devices and that are entirely dependent on registration timing
to make sure that their backing device has become available.

You could also look at doing multi-threaded probing at the bus level
where the semantics and implications are better understood, but that
still doesn't necessarily help with ordering. (ie, registering a GPIO
expander on a PCI MFD before being able to bring up gpio-leds or
so). There has been past work for both multi-threading the probe routines
as well as constraining it and only doing it for things like PCI.

In cases where you can specifically note that dependencies, doing so will
save you a world of pain. Despite that, it's simply not possible to do
this as a free-for-all. Devices or busses that can tolerate multi-threaded
probing need to be converted over one at a time, but even then you still
need the dependency tracking for those that depend on link order today.

Vendors often end up doing this sort of work for device-specific kernels
where the underlying set of drivers is fixed, so there are also some
alternative approaches you can investigate there (OLPC comes to mind).

Re: A better way to sequence driver initialization?

[Grant Likely]

On Sat, Apr 10, 2010 at 5:39 PM, Paul Mundt <lethal@linux-sh.org> wrote:
> In cases where you can specifically note that dependencies, doing so will
> save you a world of pain. Despite that, it's simply not possible to do
> this as a free-for-all. Devices or busses that can tolerate multi-threaded
> probing need to be converted over one at a time, but even then you still
> need the dependency tracking for those that depend on link order today.

Well stated.

g.

Re: A better way to sequence driver initialization?

[Bill Gatliff]

Grant Likely wrote:
> On Sat, Apr 10, 2010 at 5:39 PM, Paul Mundt <lethal@linux-sh.org> wrote:
>   
>> In cases where you can specifically note that dependencies, doing so will
>> save you a world of pain. Despite that, it's simply not possible to do
>> this as a free-for-all. Devices or busses that can tolerate multi-threaded
>> probing need to be converted over one at a time, but even then you still
>> need the dependency tracking for those that depend on link order today.
>>     

Who's to say a function like gpio_request_wait_for_it(GPIO_NUMBER,
"dependent-driver") isn't the way to do the dependency tracking?  I
can't even implement that without a context that can sleep...

b.g.

-- 
Bill Gatliff
bgat@billgatliff.com

Re: A better way to sequence driver initialization?

[Paul Mundt]

On Sat, Apr 10, 2010 at 08:33:53PM -0500, Bill Gatliff wrote:
> Grant Likely wrote:
> > On Sat, Apr 10, 2010 at 5:39 PM, Paul Mundt <lethal@linux-sh.org> wrote:
> >   
> >> In cases where you can specifically note that dependencies, doing so will
> >> save you a world of pain. Despite that, it's simply not possible to do
> >> this as a free-for-all. Devices or busses that can tolerate multi-threaded
> >> probing need to be converted over one at a time, but even then you still
> >> need the dependency tracking for those that depend on link order today.
> >>     
> 
> Who's to say a function like gpio_request_wait_for_it(GPIO_NUMBER,
> "dependent-driver") isn't the way to do the dependency tracking?  I
> can't even implement that without a context that can sleep...
> 
In some cases that might be valid, but there are many cases where drivers
can reconfigure their capability sets based on which GPIOs are and aren't
available. Just because a pin isn't available doesn't make it a
show-stopper for the probe path..

Re: A better way to sequence driver initialization?

[Bill Gatliff]

Paul Mundt wrote:
> In some cases that might be valid, but there are many cases where drivers
> can reconfigure their capability sets based on which GPIOs are and aren't
> available. Just because a pin isn't available doesn't make it a
> show-stopper for the probe path..
>   

Understood.

I just tweaked my kernel to run all probe()s  in their own kthreads. 
The results were a mixed bag, as expected.

On the one hand, it's pretty cool to see everything running in parallel!

On the other hand, I can see now yet another big reason why a probe
free-for-all won't work.  Busses are also devices, so there are plenty
of places where a device for a particular bus type won't be able to
probe because the target bus simply doesn't exist yet.  That's yet
another dependency that I hadn't thought about.

Were I to rewrite Linux :), I would make probing parallel from the
beginning.  But I think I'm going to have to settle for now with
kthreading my probes that might want to sleep, and adding a wait queue
to gpio_request() and a few others.  It ain't perfect, but it is
achieveable.  *sigh*


b.g.

-- 
Bill Gatliff
bgat@billgatliff.com

Re: A better way to sequence driver initialization?

[Bill Gatliff]

Paul Mundt wrote:
>
> In cases where you can specifically note that dependencies, doing so will
> save you a world of pain. Despite that, it's simply not possible to do
> this as a free-for-all. Devices or busses that can tolerate multi-threaded
> probing need to be converted over one at a time, but even then you still
> need the dependency tracking for those that depend on link order today.
>   

Right now I'm thinking mostly about GPIO devices which need to register
before things like gpio-leds can use them.  Sometimes the GPIO expansion
chip of interest is on i2c, other times it's spi, and still others it's
a platform or USB device.  Linking the gpio-led driver late helps that
specific situation.

But what about when I want to use a pin on a GPIO expansion device as
the card-detect for an MMC slot?  Or, as I've just recently noticed in
one hardware platform I'm working on, using a pin on a SPI GPIO expander
as a chip-select for another SPI device, but through gpiolib? 
Eventually, the dependencies don't become circular but changing the link
order will break some and fix others.  Ugh.

Definitely, a free-for-all isn't going to work.  But I don't think that
having every driver do its own kthread_run() is a solution, either. 
I'll keep tinkering.  :)


b.g.

-- 
Bill Gatliff
bgat@billgatliff.com

Re: A better way to sequence driver initialization?

[Benjamin Herrenschmidt]

On Sat, 2010-04-10 at 20:31 -0500, Bill Gatliff wrote:
> Right now I'm thinking mostly about GPIO devices which need to register
> before things like gpio-leds can use them.  Sometimes the GPIO expansion
> chip of interest is on i2c, other times it's spi, and still others it's
> a platform or USB device.  Linking the gpio-led driver late helps that
> specific situation.

What it boils down to is that driver A depends on a service provided by
driver B. At some stage, your dependency -has- to be expressed. Either
using the device-tree, or by hard wiring a GPIO number, maybe by having
the platform code figure out the GPIO number and shoving it into a
platform_data structure, etc... but at -some- point, driver "A" needs
some kind of "identifier" to the service provided by driver "B". In our
case, let's say it's a GPIO number.

That's when you can start kicking in a simple mechanism for exporting
services. Let's say the GPIO layer does

	register_service("gpio", gpio_service_query);

With something like

	int gpio_service_query(const char *query_string);

Which can parse whatever arguments are passed after the service name and
return success or failure (or even return a struct device *, whatever,
this is just a rough sketch).

From there, you can have for the drivers something like:

	request_service(const char *name, struct device *myself);

Which takes a service name of the form "service:args", for example
"gpio:5" for GPIO #5.

That returns 0 if found, -EAGAIN if not found yet, -EINVAL if rejected
(the service "gpio" can decide that there cannot be a GPIO 5 on the
system, whatever) for example. In addition, it would attach "myself" (if
non-NULL) to an internal list so that later, it's probe() routine can be
called again.

We then add a way for probe() to return -EAGAIN to put the driver "on
hold" silently.

And finally, maybe a way for services to re-run their query (new GPIOs
were added for example) for all "pending" drivers.

I haven't looked at the flip side here which is service -removal- of
course. We may get away without dealing with it... tbd.

> But what about when I want to use a pin on a GPIO expansion device as
> the card-detect for an MMC slot?  Or, as I've just recently noticed in
> one hardware platform I'm working on, using a pin on a SPI GPIO expander
> as a chip-select for another SPI device, but through gpiolib? 
> Eventually, the dependencies don't become circular but changing the link
> order will break some and fix others.  Ugh.

As I said above, at some stage, -somebody- can identify the dependency,
which ends up being translated into a GPIO number, an OF device path, or
something .... That should be enough for the above to be made to fit
no ?

> Definitely, a free-for-all isn't going to work.  But I don't think that
> having every driver do its own kthread_run() is a solution, either. 
> I'll keep tinkering.  :) 

I'd rather keep threads out of the picture for now. IE. Leave the
decision as to do threaded probing or not at a different level.

Cheers,
Ben.

Re: A better way to sequence driver initialization?

[Benjamin Herrenschmidt]

On Sun, 2010-04-11 at 08:39 +0900, Paul Mundt wrote:
> This is a common problem for drivers that are all stuck on the same
> initcall level and as a result are entirely dependent on link order. Some
> more intelligent logic via the bus notifiers would help with some of
> this, but it wouldn't help with drivers that are effectively unable to
> probe for devices and that are entirely dependent on registration timing
> to make sure that their backing device has become available.

Right. Exactly my problem with EMAC for example.

> You could also look at doing multi-threaded probing at the bus level
> where the semantics and implications are better understood, but that
> still doesn't necessarily help with ordering. (ie, registering a GPIO
> expander on a PCI MFD before being able to bring up gpio-leds or
> so). There has been past work for both multi-threading the probe routines
> as well as constraining it and only doing it for things like PCI.

I'd like to detatch the ordering problem from the threading feature,
those are mostly orthogonal. We could solve the ordering problem with
either some creative way of defining dependencies statically (but I
personally think that's doomed) or a way for probe() to return -EAGAIN
after the driver got a chance to attach itself to some kind of mechanism
to re-trigger the probing when some condition is satisfied (bus
notifier ? exported service names as in my previous example ? some combo
of these ?).

Then for archs like powerpc, we can add a layer to automate maybe the
dependency resolution by using the device-tree, though the underlying
mechanism doesn't need to be device-tree based.

> In cases where you can specifically note that dependencies, doing so will
> save you a world of pain. Despite that, it's simply not possible to do
> this as a free-for-all. Devices or busses that can tolerate multi-threaded
> probing need to be converted over one at a time, but even then you still
> need the dependency tracking for those that depend on link order today.
> 
> Vendors often end up doing this sort of work for device-specific kernels
> where the underlying set of drivers is fixed, so there are also some
> alternative approaches you can investigate there (OLPC comes to mind).

As I said above, I think we should keep the dependency handling from the
threading two separate things :-)

Re: A better way to sequence driver initialization?

[Benjamin Herrenschmidt]

> The general problem with your approach is that the module in question
> might not know what services it needs to wait for.

The device-tree could provide a lot of help here, by providing a way to
wait for a service attached to a given device-node since -that- at least
contains the linkage we need.

In fact, I could use something like that with the ibm_newemac driver
where I have various bits and pieces probed separately (MAL - aka DMA
engine, MII interface, core MAC, etc...) and right now I go through
horrible hacks to ensure all the drivers have called home. I do have
explicit links in the device-tree, so what I really want is for a driver
attached to a device node to be able to say "I'm now operational" and
for other drivers to wait on that.

In fact, we don't necessarily need to use threads for that. We could
have a way for a device to return something like -EAGAIN from probe(),
maybe having called some kind of request_service(node,...) on all the
dependencies, if any of them isn't satisfied. The core would then be
able to finally call probe() again when they are.

That has the advantage of leaving the amount of threading required to
the core, rather than making it a driver decision, since we don't want
-too- many probing threads in parallel.

> Specific to my situation, the gpio-led code doesn't have any way of
> knowing that it needs to wait until my pca953x i2c devices have all been
> installed so that the gpio pin I have specified even exists.

It could know that via the device-tree. Or on more paleolithic
architectures, via a GPIO number though that sucks. Note that my
wait_for_service() example takes a string. That string in my mind can be
a compound, such as "gpio:#5" for example.

>  And short
> of setting up some kind of table in the board-specific code (or device
> tree, actually), I don't know how to communicate such a dependency
> without touching the generic gpio-led code--- which I'm trying to avoid.
> 
> I just want gpio-led to try again if the gpio pin it has been provided
> can't be requested yet.  And I can see the need for similar behavior in
> several other of my drivers, hence the desire to generalize things a
> bit.  I'm pretty sure my approach is only half-baked, but it does seem
> to avoid the need to touch a bunch of existing code--- especially to add
> board-specific dependencies.  It just give the system a tool to sort
> things out on its own.
> 
> I do think your overall criticism is valid, though.

Cheers,
Ben.