Re: [RFC v1 0/3] driver-core: add asynch module loading support

Re: [RFC v1 0/3] driver-core: add asynch module loading support

[Arjan van de Ven]

On 8/31/2014 1:06 PM, 吴章金 wrote:
> Hi, folks
>
> I'm back to this discussion,
>
> The original requirement of my first RFC patchset is mainly for Android Smartphone use case:
>
> 1. We want light on LCD and draw a logo immediately after power key press(don't consider uboot or lk biotloader here).
> 2. We want the whole kernel boot fast to give user the Android Launch deaktop
> 3. The modem initialization/reset is slow
> 4. The Touchpad firmware upgrade is slow
> 5. We have many cpu cores(up to 8 in latest exynos 5430 and MT6595...)
> 6. We have few schedulable/parallellizable threads
> 7. We compiled all of the modules in the kernel(stupid? avoid modprobe...but lose parallelization in userspace)
>
> So, I think about is that possible to async most of the probes, but still reserve the requred dependencies to let them still work as expected.

you can boot a whole kernel including all graphics in less than 0.5 seconds, even without this patchset.

Re: [RFC v1 0/3] driver-core: add asynch module loading support

[Dmitry Torokhov]

On August 31, 2014 3:32:19 PM PDT, Arjan van de Ven <arjan@linux.intel.com> wrote:
>On 8/31/2014 1:06 PM, 吴章金 wrote:
>> Hi, folks
>>
>> I'm back to this discussion,
>>
>> The original requirement of my first RFC patchset is mainly for
>Android Smartphone use case:
>>
>> 1. We want light on LCD and draw a logo immediately after power key
>press(don't consider uboot or lk biotloader here).
>> 2. We want the whole kernel boot fast to give user the Android Launch
>deaktop
>> 3. The modem initialization/reset is slow
>> 4. The Touchpad firmware upgrade is slow
>> 5. We have many cpu cores(up to 8 in latest exynos 5430 and
>MT6595...)
>> 6. We have few schedulable/parallellizable threads
>> 7. We compiled all of the modules in the kernel(stupid? avoid
>modprobe...but lose parallelization in userspace)
>>
>> So, I think about is that possible to async most of the probes, but
>still reserve the requred dependencies to let them still work as
>expected.
>
>you can boot a whole kernel including all graphics in less than 0.5
>seconds, even without this patchset.


You forgot to add "on certain subset of hardware and configuration".

Thanks.

-- 
Dmitry

Re: [RFC v1 0/3] driver-core: add asynch module loading support

[Arjan van de Ven]

On 8/31/2014 3:45 PM, Dmitry Torokhov wrote:
> On August 31, 2014 3:32:19 PM PDT, Arjan van de Ven <arjan@linux.intel.com> wrote:
>> On 8/31/2014 1:06 PM, 吴章金 wrote:
>>> Hi, folks
>>>
>>> I'm back to this discussion,
>>>
>>> The original requirement of my first RFC patchset is mainly for
>> Android Smartphone use case:
>>>
>>> 1. We want light on LCD and draw a logo immediately after power key
>> press(don't consider uboot or lk biotloader here).
>>> 2. We want the whole kernel boot fast to give user the Android Launch
>> deaktop
>>> 3. The modem initialization/reset is slow
>>> 4. The Touchpad firmware upgrade is slow
>>> 5. We have many cpu cores(up to 8 in latest exynos 5430 and
>> MT6595...)
>>> 6. We have few schedulable/parallellizable threads
>>> 7. We compiled all of the modules in the kernel(stupid? avoid
>> modprobe...but lose parallelization in userspace)
>>>
>>> So, I think about is that possible to async most of the probes, but
>> still reserve the requred dependencies to let them still work as
>> expected.
>>
>> you can boot a whole kernel including all graphics in less than 0.5
>> seconds, even without this patchset.
>
>
> You forgot to add "on certain subset of hardware and configuration".

certainly.

for hardware that takes longer than 0.5s you certainly won't get that.

asynchronous/parallel init helps, but it is not a miracle. Most devices I've worked on
(wide range of hardware from PC to tablets to phones) there tend to be 4 to 5 big guys and a long tail of negliable ones.
the big guys tend to be, even after going in parallel, so large that people who wanted instant boot have to accept
that more miracles are needed.
Like actual work in the driver or device firmware to split work up, rather than an "from the outside" make it asynchronous.

[RFC v1 0/3] driver-core: add asynch module loading support

[Luis R. Rodriguez]

From: "Luis R. Rodriguez" <mcgrof@suse.com>

While reviewing Wu Zhangjin's solution to async probe [0] and his
ideas on creating async groups I decided to try following the init
levels on the kernel to try to help with synchronization at least
on some level. This borrows ideas discussed with the kthread_create()
solution [1] and also simplifies the idea of how we should be grouping
async calls between drivers.

A few things worth mentioning. I decided to go down a generic solution as
if we ever wanted to bring asynchrnonization behaviour below modules
this would allow folks to start testing this without much effort. It
allows asynchronous calls to upkeep the order already set in place
for built-in code. If one wanted to eventually venture down below
that path we'd need to add respective exit calls for each level, as
right now we just assume that every level should use module_exit().
SmPL grammer could be used to easily tidy this up, for example on
the subsys_init():

@ subsys_found @
expression fn_init;
declarer name subsys_initcall;
@@

  subsys_initcall(fn_init);                                                     

@ subsys_exit_found depends on subsys_found @
expression fn_exit;
declarer name module_exit;
declarer name subsys_exitcall;
@@                                                                              
                                                                                
- module_exit(fn_exit);
+ subsys_exitcall(fn_exit);

And so on. The second thing to note is that when an asynchronous
call is used we don't want to free the init data as otherwise
async_run_entry_fn() will run into a missing init routine. SmPL
could still be used to convert basic __init data out if this is
needed, however if the init routine also used other init data
we'd also have to remove the other __init data used. It could
in theory be possible to do a witch hunt to write this in grammar
but for now only a simple conversion on the init side is recommended
as reflected in the SmPL below and manual inspection after that.
Another option as suggested by Julia to me was to consider an
__init_asynch which we could reap later. This is of course if
we go down this generic path.

@ module_init_found @
identifier f;
declarer name module_init;
declarer name module_init_async;
@@

-   module_init(f);
+   module_init_async(f);

@ modify_decl depends on module_init_found @
identifier module_init_found.f;
@@

- int f
+ int f
    (...) { ... }


@ module_exit_found depends on module_init_found @
identifier fn_exit;
declarer name module_exit;
declarer name module_exit_async;
@@

- module_exit(fn_exit);
+ module_exit_async(fn_exit);

Although not visible the above int f does remove the __init...

If generalizing an async solution is not desirable in the
long run for things other than modules than a bool should
be easy to use to figure if probe should run async'd or not.
Grouping however still becomes a question then, and this is
why I went with this approach as it aligns itself more closely
to the kernel init levels and that should be well tested. In
theory it could even be possible to use a similar strategy to
asynch on per init level when built-in using a similar strategy
but these would have to be separate.

Let me know which way to swing, and if we do a probe bool on
the module, please consider the grouping question.

[0] https://lkml.org/lkml/2014/8/14/11
[1] http://lists.freedesktop.org/archives/systemd-devel/2014-August/022156.html

Luis R. Rodriguez (3):
  driver-core: split module_init() and module_exit()
  async: move synchronous caller into a helper
  async: add driver asynch levels

 include/linux/async.h |  4 +++
 include/linux/init.h  | 77 ++++++++++++++++++++++++++++++++++++++-------------
 kernel/async.c        | 55 +++++++++++++++++++++++++++++++-----
 3 files changed, 109 insertions(+), 27 deletions(-)

-- 
2.0.3

Re: [RFC v1 0/3] driver-core: add asynch module loading support

[Tejun Heo]

Hello, Luis.

I haven't followed the previous discussions so please let me know if
this has been discussed before.  It looks like you're trying to extend
the async mechanism and applying them to init functions themselves.
That sounds kinda weird to me.  Isn't the root cause of the problem
doing device probings along with driver initilaization on module load?

Wouldn't it be more logical to simply make bus_add_driver() ->
driver_attach() invocation asynchronous?  There's no reason to make
them parallel either.  We can use an ordered queue for it so that we
don't lose the probing order we used to have.  Making things go
parallel is the responsibility of each probing function after all and
there isn't much to gain by making attach calls go parallel.

Thanks.

-- 
tejun

Re: [RFC v1 0/3] driver-core: add asynch module loading support

[Tejun Heo]

So, something like the following.  A couple things to note

* driver_attach() can never fail but is marked with __must_check.  We
  prolly should change it to void.

* Old/weird userspace which depends on insmod to wait for device
  probing might choke and the new behavior might need to be switched
  somehow (sysctl, insmod param or whatever).

Lightly tested and seems to work fine.

Thanks.
---
 drivers/base/bus.c |   39 ++++++++++++++++++++++++++++++++++++---
 1 file changed, 36 insertions(+), 3 deletions(-)

diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index 83e910a..e4fc9bc 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -23,6 +23,7 @@
 
 /* /sys/devices/system */
 static struct kset *system_kset;
+static struct workqueue_struct *driver_attach_wq;
 
 #define to_bus_attr(_attr) container_of(_attr, struct bus_attribute, attr)
 
@@ -657,6 +658,20 @@ static ssize_t uevent_store(struct device_driver *drv, const char *buf,
 }
 static DRIVER_ATTR_WO(uevent);
 
+struct driver_attach_work {
+	struct work_struct		work;
+	struct device_driver		*driver;
+};
+
+static void driver_attach_workfn(struct work_struct *work)
+{
+	struct driver_attach_work *daw =
+		container_of(work, struct driver_attach_work, work);
+
+	driver_attach(daw->driver);
+	kfree(daw);
+}
+
 /**
  * bus_add_driver - Add a driver to the bus.
  * @drv: driver.
@@ -689,9 +704,23 @@ int bus_add_driver(struct device_driver *drv)
 
 	klist_add_tail(&priv->knode_bus, &bus->p->klist_drivers);
 	if (drv->bus->p->drivers_autoprobe) {
-		error = driver_attach(drv);
-		if (error)
-			goto out_unregister;
+		struct driver_attach_work *daw;
+
+		if (drv->owner) {
+			daw = kzalloc(sizeof(*daw), GFP_KERNEL);
+			if (!daw) {
+				error = -ENOMEM;
+				goto out_unregister;
+			}
+
+			INIT_WORK(&daw->work, driver_attach_workfn);
+			daw->driver = drv;
+			queue_work(driver_attach_wq, &daw->work);
+		} else {
+			error = driver_attach(drv);
+			if (error)
+				goto out_unregister;
+		}
 	}
 	module_add_driver(drv->owner, drv);
 
@@ -1268,5 +1297,9 @@ int __init buses_init(void)
 	if (!system_kset)
 		return -ENOMEM;
 
+	driver_attach_wq = alloc_ordered_workqueue("driver_attach", 0);
+	if (!driver_attach_wq)
+		return -ENOMEM;
+
 	return 0;
 }

Re: [RFC v1 0/3] driver-core: add asynch module loading support

[Tejun Heo]

On Sun, Aug 31, 2014 at 07:02:00AM -0400, Tejun Heo wrote:
> So, something like the following.  A couple things to note
> 
> * driver_attach() can never fail but is marked with __must_check.  We
>   prolly should change it to void.
> 
> * Old/weird userspace which depends on insmod to wait for device
>   probing might choke and the new behavior might need to be switched
>   somehow (sysctl, insmod param or whatever).

One more thing.

* Use of ordered workqueue probably isn't necessary and using
  system_unbound_wq should be fine.

Thanks.

-- 
tejun

Re: [RFC v1 0/3] driver-core: add asynch module loading support

[Dmitry Torokhov]

On Sun, Aug 31, 2014 at 07:05:26AM -0400, Tejun Heo wrote:
> On Sun, Aug 31, 2014 at 07:02:00AM -0400, Tejun Heo wrote:
> > So, something like the following.  A couple things to note
> > 
> > * driver_attach() can never fail but is marked with __must_check.  We
> >   prolly should change it to void.
> > 
> > * Old/weird userspace which depends on insmod to wait for device
> >   probing might choke and the new behavior might need to be switched
> >   somehow (sysctl, insmod param or whatever).
> 
> One more thing.
> 
> * Use of ordered workqueue probably isn't necessary and using
>   system_unbound_wq should be fine.
> 

For my use case (touchpad taking long time to initialize) I explicitly do not
want it to keep order of initialization. I want to make sure the rest of the
kernel continues initialization while touchpad device resets.

Thanks.

-- 
Dmitry

Re: [RFC v1 0/3] driver-core: add asynch module loading support

[Arjan van de Ven]

On 8/31/2014 10:52 AM, Dmitry Torokhov wrote:
> On Sun, Aug 31, 2014 at 07:05:26AM -0400, Tejun Heo wrote:
>> On Sun, Aug 31, 2014 at 07:02:00AM -0400, Tejun Heo wrote:
>>> So, something like the following.  A couple things to note
>>>
>>> * driver_attach() can never fail but is marked with __must_check.  We
>>>    prolly should change it to void.
>>>
>>> * Old/weird userspace which depends on insmod to wait for device
>>>    probing might choke and the new behavior might need to be switched
>>>    somehow (sysctl, insmod param or whatever).
>>
>> One more thing.
>>
>> * Use of ordered workqueue probably isn't necessary and using
>>    system_unbound_wq should be fine.
>>
>
> For my use case (touchpad taking long time to initialize) I explicitly do not
> want it to keep order of initialization. I want to make sure the rest of the
> kernel continues initialization while touchpad device resets.

but we do that right now

you know its there synchronous
and do the heavy stuff async


now sadly  we also wait for the touchpad to finish init if you don't use an initramfs
(but we dont wait if you have an initramfs) but that's a different thing from all of this

Re: [RFC v1 0/3] driver-core: add asynch module loading support

[Dmitry Torokhov]

On Sun, Aug 31, 2014 at 12:26:06PM -0700, Arjan van de Ven wrote:
> On 8/31/2014 10:52 AM, Dmitry Torokhov wrote:
> >On Sun, Aug 31, 2014 at 07:05:26AM -0400, Tejun Heo wrote:
> >>On Sun, Aug 31, 2014 at 07:02:00AM -0400, Tejun Heo wrote:
> >>>So, something like the following.  A couple things to note
> >>>
> >>>* driver_attach() can never fail but is marked with __must_check.  We
> >>>   prolly should change it to void.
> >>>
> >>>* Old/weird userspace which depends on insmod to wait for device
> >>>   probing might choke and the new behavior might need to be switched
> >>>   somehow (sysctl, insmod param or whatever).
> >>
> >>One more thing.
> >>
> >>* Use of ordered workqueue probably isn't necessary and using
> >>   system_unbound_wq should be fine.
> >>
> >
> >For my use case (touchpad taking long time to initialize) I explicitly do not
> >want it to keep order of initialization. I want to make sure the rest of the
> >kernel continues initialization while touchpad device resets.
> 
> but we do that right now
> 
> you know its there synchronous
> and do the heavy stuff async

No, in general we do not. I have hacked in serio (i.e. PS/2 support to do
registration/probing in a separate thread - and I'd love to drop that code),
but PS/2 is in its way out and we need the same for devices on I2C. SPI and
other slow buses.

Thanks.

-- 
Dmitry

Re: [RFC v1 0/3] driver-core: add asynch module loading support

[David Herrmann]

Hi

On Sun, Aug 31, 2014 at 1:02 PM, Tejun Heo <tj@kernel.org> wrote:
> So, something like the following.  A couple things to note
>
> * driver_attach() can never fail but is marked with __must_check.  We
>   prolly should change it to void.
>
> * Old/weird userspace which depends on insmod to wait for device
>   probing might choke and the new behavior might need to be switched
>   somehow (sysctl, insmod param or whatever).
>
> Lightly tested and seems to work fine.
>
> Thanks.
> ---
>  drivers/base/bus.c |   39 ++++++++++++++++++++++++++++++++++++---
>  1 file changed, 36 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/base/bus.c b/drivers/base/bus.c
> index 83e910a..e4fc9bc 100644
> --- a/drivers/base/bus.c
> +++ b/drivers/base/bus.c
> @@ -23,6 +23,7 @@
>
>  /* /sys/devices/system */
>  static struct kset *system_kset;
> +static struct workqueue_struct *driver_attach_wq;
>
>  #define to_bus_attr(_attr) container_of(_attr, struct bus_attribute, attr)
>
> @@ -657,6 +658,20 @@ static ssize_t uevent_store(struct device_driver *drv, const char *buf,
>  }
>  static DRIVER_ATTR_WO(uevent);
>
> +struct driver_attach_work {
> +       struct work_struct              work;
> +       struct device_driver            *driver;
> +};
> +
> +static void driver_attach_workfn(struct work_struct *work)
> +{
> +       struct driver_attach_work *daw =
> +               container_of(work, struct driver_attach_work, work);
> +
> +       driver_attach(daw->driver);
> +       kfree(daw);
> +}
> +
>  /**
>   * bus_add_driver - Add a driver to the bus.
>   * @drv: driver.
> @@ -689,9 +704,23 @@ int bus_add_driver(struct device_driver *drv)
>
>         klist_add_tail(&priv->knode_bus, &bus->p->klist_drivers);
>         if (drv->bus->p->drivers_autoprobe) {
> -               error = driver_attach(drv);
> -               if (error)
> -                       goto out_unregister;
> +               struct driver_attach_work *daw;
> +
> +               if (drv->owner) {
> +                       daw = kzalloc(sizeof(*daw), GFP_KERNEL);
> +                       if (!daw) {
> +                               error = -ENOMEM;
> +                               goto out_unregister;
> +                       }
> +
> +                       INIT_WORK(&daw->work, driver_attach_workfn);
> +                       daw->driver = drv;
> +                       queue_work(driver_attach_wq, &daw->work);

Doesn't this break on-demand cdev initialization? We currently call
request_module() on open() for unclaimed major/minor combinations. If
driver_attach() is no longer part of module_init(), there is no
guarantee the driver created the cdev before request_module() returns.

I actually like this "deferred attach" approach, so this is not meant
as counter-argument. We just need to make sure to have a notion of
"settled modules" so we know how long to wait after loading a module.

Thanks
David

> +               } else {
> +                       error = driver_attach(drv);
> +                       if (error)
> +                               goto out_unregister;
> +               }
>         }
>         module_add_driver(drv->owner, drv);
>
> @@ -1268,5 +1297,9 @@ int __init buses_init(void)
>         if (!system_kset)
>                 return -ENOMEM;
>
> +       driver_attach_wq = alloc_ordered_workqueue("driver_attach", 0);
> +       if (!driver_attach_wq)
> +               return -ENOMEM;
> +
>         return 0;
>  }
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC v1 0/3] driver-core: add asynch module loading support

[Tejun Heo]

(cc'ing Rusty for module loading)

Hello,

On Sun, Aug 31, 2014 at 01:25:03PM +0200, David Herrmann wrote:
> On Sun, Aug 31, 2014 at 1:02 PM, Tejun Heo <tj@kernel.org> wrote:
> > @@ -689,9 +704,23 @@ int bus_add_driver(struct device_driver *drv)
> >
> >         klist_add_tail(&priv->knode_bus, &bus->p->klist_drivers);
> >         if (drv->bus->p->drivers_autoprobe) {
> > -               error = driver_attach(drv);
> > -               if (error)
> > -                       goto out_unregister;
> > +               struct driver_attach_work *daw;

Oops, this probably should go inside the below if block.

> > +
> > +               if (drv->owner) {
> > +                       daw = kzalloc(sizeof(*daw), GFP_KERNEL);
> > +                       if (!daw) {
> > +                               error = -ENOMEM;
> > +                               goto out_unregister;
> > +                       }
> > +
> > +                       INIT_WORK(&daw->work, driver_attach_workfn);
> > +                       daw->driver = drv;
> > +                       queue_work(driver_attach_wq, &daw->work);
> 
> Doesn't this break on-demand cdev initialization? We currently call
> request_module() on open() for unclaimed major/minor combinations. If
> driver_attach() is no longer part of module_init(), there is no
> guarantee the driver created the cdev before request_module() returns.

Right, yeah, this really looks like something we'd need to switch per
insmod instance.  It looks like driver core needs a generic parameter
to tell it to whether drive probing asynchronously to module loading
or not.  Maybe we can add a generic driver param like
"driver_async_probe"?

> I actually like this "deferred attach" approach, so this is not meant
> as counter-argument. We just need to make sure to have a notion of
> "settled modules" so we know how long to wait after loading a module.

Another way could be making module-generic pollable file in the
module's sysfs dir to indicate "full init completion", which would map
to probing completion for drivers; however, given that we need to keep
the synchronous behavior by default for compatibility, I don't think
that buys us much.

Thanks.

-- 
tejun

Re: [RFC v1 0/3] driver-core: add asynch module loading support

[Dmitry Torokhov]

HI Tejun,

On Sun, Aug 31, 2014 at 06:13:58AM -0400, Tejun Heo wrote:
> Hello, Luis.
> 
> I haven't followed the previous discussions so please let me know if
> this has been discussed before.  It looks like you're trying to extend
> the async mechanism and applying them to init functions themselves.
> That sounds kinda weird to me.  Isn't the root cause of the problem
> doing device probings along with driver initilaization on module load?

For my use case it is driver initialization itself (because most of the
relevant drivers is compiled in). Although, come to think, if we could do
something about resume that would be nice too: then I'd be able to drop all
stuff in serio that lies about device state and marks it as resumed even though
mouse/touchpad will be actually reset and operable much later.

> 
> Wouldn't it be more logical to simply make bus_add_driver() ->
> driver_attach() invocation asynchronous?  There's no reason to make
> them parallel either.  We can use an ordered queue for it so that we
> don't lose the probing order we used to have.

Sometimes losing probing order is the desired outcome though. Like with my
beloved touchpad :)

> Making things go
> parallel is the responsibility of each probing function after all and
> there isn't much to gain by making attach calls go parallel.

If we make probe function schedule stuff asynchronously, then, in case of
failures, we'll end up with half-bound driver. Also drivers would have to have
additional code on removal to make sure probe full done before removing. PM
methods need to be ready to be called on half-initialized device. It is a mess.

Thanks.

-- 
Dmitry

Re: [RFC v1 0/3] driver-core: add asynch module loading support

[Tejun Heo]

Hello, Dmitry.

On Sun, Aug 31, 2014 at 11:28:51AM -0700, Dmitry Torokhov wrote:
> HI Tejun,
> 
> On Sun, Aug 31, 2014 at 06:13:58AM -0400, Tejun Heo wrote:
> > I haven't followed the previous discussions so please let me know if
> > this has been discussed before.  It looks like you're trying to extend
> > the async mechanism and applying them to init functions themselves.
> > That sounds kinda weird to me.  Isn't the root cause of the problem
> > doing device probings along with driver initilaization on module load?
> 
> For my use case it is driver initialization itself (because most of the
> relevant drivers is compiled in). Although, come to think, if we could do

Hmmm... that is a different case from the one I'm aware of - userland
timing out on module probing and sending SIGKILL aborting device
probing, which is taking long but still making progress properly.

> something about resume that would be nice too: then I'd be able to drop all
> stuff in serio that lies about device state and marks it as resumed even though
> mouse/touchpad will be actually reset and operable much later.
> 
> > Wouldn't it be more logical to simply make bus_add_driver() ->
> > driver_attach() invocation asynchronous?  There's no reason to make
> > them parallel either.  We can use an ordered queue for it so that we
> > don't lose the probing order we used to have.
> 
> Sometimes losing probing order is the desired outcome though. Like with my
> beloved touchpad :)

Yeah, at first I was applying the async thing to built-in drivers too
so the ordering was kinda necessary but I don't think it matters at
all for modules.

> > Making things go
> > parallel is the responsibility of each probing function after all and
> > there isn't much to gain by making attach calls go parallel.
> 
> If we make probe function schedule stuff asynchronously, then, in case of
> failures, we'll end up with half-bound driver. Also drivers would have to have
> additional code on removal to make sure probe full done before removing. PM
> methods need to be ready to be called on half-initialized device. It is a mess.

I don't get this.  The relationship between the driver and its devices
is 1:N.  If a driver fails to bind to a device, it should properly
unbind from that device but still remain available so that it can be
used when another device becomes available.  The two have completely
different life cycles.  If you take a look at
drivers/base/dd.c::__driver_attach(), the function ignores error
return from probing.  The return types are pretty misleading due to
historical reasons but we already do not fail module loading after
probing failure, so making probing asynchronous shouldn't really
change anything.

Thanks.

-- 
tejun

Re: [RFC v1 0/3] driver-core: add asynch module loading support

[Dmitry Torokhov]

On Sun, Aug 31, 2014 at 06:02:38PM -0400, Tejun Heo wrote:
> Hello, Dmitry.
> 
> On Sun, Aug 31, 2014 at 11:28:51AM -0700, Dmitry Torokhov wrote:
> > HI Tejun,
> > 
> > On Sun, Aug 31, 2014 at 06:13:58AM -0400, Tejun Heo wrote:
> > > I haven't followed the previous discussions so please let me know if
> > > this has been discussed before.  It looks like you're trying to extend
> > > the async mechanism and applying them to init functions themselves.
> > > That sounds kinda weird to me.  Isn't the root cause of the problem
> > > doing device probings along with driver initilaization on module load?
> > 
> > For my use case it is driver initialization itself (because most of the
> > relevant drivers is compiled in). Although, come to think, if we could do
> 
> Hmmm... that is a different case from the one I'm aware of - userland
> timing out on module probing and sending SIGKILL aborting device
> probing, which is taking long but still making progress properly.

Right, it is definitely different use case. Here we just have a touchpad taking
1.5-2 secs to initialize and thus stalling the overall boot progress. 

> 
> > something about resume that would be nice too: then I'd be able to drop all
> > stuff in serio that lies about device state and marks it as resumed even though
> > mouse/touchpad will be actually reset and operable much later.
> > 
> > > Wouldn't it be more logical to simply make bus_add_driver() ->
> > > driver_attach() invocation asynchronous?  There's no reason to make
> > > them parallel either.  We can use an ordered queue for it so that we
> > > don't lose the probing order we used to have.
> > 
> > Sometimes losing probing order is the desired outcome though. Like with my
> > beloved touchpad :)
> 
> Yeah, at first I was applying the async thing to built-in drivers too
> so the ordering was kinda necessary but I don't think it matters at
> all for modules.

Why are we talking about modules? The touchpad driver is built-in in my case.

> 
> > > Making things go
> > > parallel is the responsibility of each probing function after all and
> > > there isn't much to gain by making attach calls go parallel.
> > 
> > If we make probe function schedule stuff asynchronously, then, in case of
> > failures, we'll end up with half-bound driver. Also drivers would have to have
> > additional code on removal to make sure probe full done before removing. PM
> > methods need to be ready to be called on half-initialized device. It is a mess.
> 
> I don't get this.  The relationship between the driver and its devices
> is 1:N.  If a driver fails to bind to a device, it should properly
> unbind from that device but still remain available so that it can be
> used when another device becomes available.  The two have completely
> different life cycles.  If you take a look at
> drivers/base/dd.c::__driver_attach(), the function ignores error
> return from probing.  The return types are pretty misleading due to
> historical reasons but we already do not fail module loading after
> probing failure, so making probing asynchronous shouldn't really
> change anything.

I think we are talking about 2 different things. I am talking about probe()
taking too long and stalling overall boot process. Yes, we can change
individual drivers to use async_schedule() to do longer operations
asynchronously, but if something that was scheduled asynchronously fails we do
not have a convenient way to signal driver core that bind operation failed. We
would end up with driver being bound to the device but device would not be
operational. This is different from synchronous probing where any failure would
cause probe() to return error and bind operation to fail.

Thanks.

-- 
Dmitry

Re: [RFC v1 0/3] driver-core: add asynch module loading support

[Tejun Heo]

Hello, Dmitry.

On Sun, Aug 31, 2014 at 04:06:51PM -0700, Dmitry Torokhov wrote:
> On Sun, Aug 31, 2014 at 06:02:38PM -0400, Tejun Heo wrote:
> > Hmmm... that is a different case from the one I'm aware of - userland
> > timing out on module probing and sending SIGKILL aborting device
> > probing, which is taking long but still making progress properly.
> 
> Right, it is definitely different use case. Here we just have a touchpad taking
> 1.5-2 secs to initialize and thus stalling the overall boot progress. 

Ah, yeah, that's a completely different issue and what other drivers
solve by using async_schedule() mechanism.

> > Yeah, at first I was applying the async thing to built-in drivers too
> > so the ordering was kinda necessary but I don't think it matters at
> > all for modules.
> 
> Why are we talking about modules? The touchpad driver is built-in in my case.

Because that was the issue which started this patch series.  It wasn't
about speeding up boot process but about probe failure from SIGKILL
from userland after timing out module loading.

> > I don't get this.  The relationship between the driver and its devices
> > is 1:N.  If a driver fails to bind to a device, it should properly
> > unbind from that device but still remain available so that it can be
> > used when another device becomes available.  The two have completely
> > different life cycles.  If you take a look at
> > drivers/base/dd.c::__driver_attach(), the function ignores error
> > return from probing.  The return types are pretty misleading due to
> > historical reasons but we already do not fail module loading after
> > probing failure, so making probing asynchronous shouldn't really
> > change anything.
> 
> I think we are talking about 2 different things. I am talking about probe()

Heh, yeah, we sure are.

> taking too long and stalling overall boot process. Yes, we can change
> individual drivers to use async_schedule() to do longer operations

That's the course we took for all other drivers.

> asynchronously, but if something that was scheduled asynchronously fails we do
> not have a convenient way to signal driver core that bind operation failed. We
> would end up with driver being bound to the device but device would not be
> operational. This is different from synchronous probing where any failure would
> cause probe() to return error and bind operation to fail.

Hmmm... I guess there's genuine difference in probing sequence between
input and scsi / libata.  For scsi / libata, the probing of actual
devices takes place outside the probe function of the controller
itself.  The controller probing is still synchronous but probing of
devices attached to that controller is asynchronous.  I guess the
problem you have is that the controller probe function itself takes
long time and there's no good way to make that asynchronous.  Yeah, I
suppose that can be solved together with the module probing timeout
issue.

Thanks.

-- 
tejun

Re: [RFC v1 0/3] driver-core: add asynch module loading support

[Arjan van de Ven]

On 8/31/2014 2:03 AM, Luis R. Rodriguez wrote:
> From: "Luis R. Rodriguez" <mcgrof@suse.com>
>
> While reviewing Wu Zhangjin's solution to async probe [0] and his
> ideas on creating async groups I decided to try following the init
> levels on the kernel to try to help with synchronization at least
> on some level. This borrows ideas discussed with the kthread_create()
> solution [1] and also simplifies the idea of how we should be grouping
> async calls between drivers.
>
> A few things worth mentioning. I decided to go down a generic solution as
> if we ever wanted to bring asynchrnonization behaviour below modules
> this would allow folks to start testing this without much effort. It
> allows asynchronous calls to upkeep the order already set in place
> for built-in code. If one wanted to eventually venture down below
> that path we'd need to add respective exit calls for each level, as
> right now we just assume that every level should use module_exit().
> SmPL grammer could be used to easily tidy this up, for example on
> the subsys_init():

before we added the current async approach the approach of async init calls was tried
At the time, Linus hated it and he was right, it was not the right thing.

What is different this time to make this the right thing to do ?

Re: [RFC v1 0/3] driver-core: add asynch module loading support

[Dmitry Torokhov]

On Sun, Aug 31, 2014 at 07:44:02AM -0700, Arjan van de Ven wrote:
> On 8/31/2014 2:03 AM, Luis R. Rodriguez wrote:
> >From: "Luis R. Rodriguez" <mcgrof@suse.com>
> >
> >While reviewing Wu Zhangjin's solution to async probe [0] and his
> >ideas on creating async groups I decided to try following the init
> >levels on the kernel to try to help with synchronization at least
> >on some level. This borrows ideas discussed with the kthread_create()
> >solution [1] and also simplifies the idea of how we should be grouping
> >async calls between drivers.
> >
> >A few things worth mentioning. I decided to go down a generic solution as
> >if we ever wanted to bring asynchrnonization behaviour below modules
> >this would allow folks to start testing this without much effort. It
> >allows asynchronous calls to upkeep the order already set in place
> >for built-in code. If one wanted to eventually venture down below
> >that path we'd need to add respective exit calls for each level, as
> >right now we just assume that every level should use module_exit().
> >SmPL grammer could be used to easily tidy this up, for example on
> >the subsys_init():
> 
> before we added the current async approach the approach of async init calls was tried
> At the time, Linus hated it and he was right, it was not the right thing.
> 
> What is different this time to make this the right thing to do ?

Because otherwise drivers still have to do this, but open code it. Let's say I
have a long operations (i.e. for some touchpads it takes about 2 secs to reset
and configure it). I can offload that part into async_schedule() so it does not
stop initialization of the rest of the system (why would I want to delay
initializing of USB or storage system until touchpad is ready?) but if that
initialization fails we end up with partially bound driver and device that is
not really operable. I would very much prefer async and sync cases be the same
- if probe() fails the driver is not bound to the device.

I think it is wrong to make async probing system-wide, but driver opt-in shoudl
be fine and right thing to do.

Thanks.

-- 
Dmitry

Re: [RFC v1 0/3] driver-core: add asynch module loading support

[Arjan van de Ven]

>> before we added the current async approach the approach of async init calls was tried
>> At the time, Linus hated it and he was right, it was not the right thing.
>>
>> What is different this time to make this the right thing to do ?
>
> Because otherwise drivers still have to do this, but open code it. Let's say I
> have a long operations (i.e. for some touchpads it takes about 2 secs to reset
> and configure it). I can offload that part into async_schedule() so it does not
> stop initialization of the rest of the system (why would I want to delay
> initializing of USB or storage system until touchpad is ready?) but if that
> initialization fails we end up with partially bound driver and device that is
> not really operable. I would very much prefer async and sync cases be the same
> - if probe() fails the driver is not bound to the device.
>
> I think it is wrong to make async probing system-wide, but driver opt-in shoudl
> be fine and right thing to do.
>

I am completely fine if we make basically an async wrapper for
pci_register_driver() and friends.. that would be convenient I suppose.

(but then again, in reality very few drivers take real time to init... most already
do the heavy work in open(). Not all can, sure, but if you look at a bootgraph.pl
graph of a typical boot it's only a few that matter).
And many drivers need to register with a subsystem, and there's some ordering around that,
and that's why we ended up with the async cookie stuff, so that you can do the
heavy work in parallel, but order near the end at registeration-with-the-subsystem time.

But doing this on an initcall level was wrong back then, and I have yet to hear
a reason why it would be right this time.

Re: [RFC v1 0/3] driver-core: add asynch module loading support

[Greg KH]

On Sun, Aug 31, 2014 at 12:24:46PM -0700, Arjan van de Ven wrote:
> >>before we added the current async approach the approach of async init calls was tried
> >>At the time, Linus hated it and he was right, it was not the right thing.
> >>
> >>What is different this time to make this the right thing to do ?
> >
> >Because otherwise drivers still have to do this, but open code it. Let's say I
> >have a long operations (i.e. for some touchpads it takes about 2 secs to reset
> >and configure it). I can offload that part into async_schedule() so it does not
> >stop initialization of the rest of the system (why would I want to delay
> >initializing of USB or storage system until touchpad is ready?) but if that
> >initialization fails we end up with partially bound driver and device that is
> >not really operable. I would very much prefer async and sync cases be the same
> >- if probe() fails the driver is not bound to the device.
> >
> >I think it is wrong to make async probing system-wide, but driver opt-in shoudl
> >be fine and right thing to do.
> >
> 
> I am completely fine if we make basically an async wrapper for
> pci_register_driver() and friends.. that would be convenient I suppose.
> 
> (but then again, in reality very few drivers take real time to init... most already
> do the heavy work in open(). Not all can, sure, but if you look at a bootgraph.pl
> graph of a typical boot it's only a few that matter).
> And many drivers need to register with a subsystem, and there's some ordering around that,
> and that's why we ended up with the async cookie stuff, so that you can do the
> heavy work in parallel, but order near the end at registeration-with-the-subsystem time.
> 
> But doing this on an initcall level was wrong back then, and I have yet to hear
> a reason why it would be right this time.

It's still wrong, it's not what I was thinking about when talking this
over with Luis and Dmitry, I think something got lost in the
translation...

thanks,

greg k-h

Re: [RFC v1 0/3] driver-core: add asynch module loading support

[Dmitry Torokhov]

On Sun, Aug 31, 2014 at 12:31:40PM -0700, Greg KH wrote:
> On Sun, Aug 31, 2014 at 12:24:46PM -0700, Arjan van de Ven wrote:
> > >>before we added the current async approach the approach of async init calls was tried
> > >>At the time, Linus hated it and he was right, it was not the right thing.
> > >>
> > >>What is different this time to make this the right thing to do ?
> > >
> > >Because otherwise drivers still have to do this, but open code it. Let's say I
> > >have a long operations (i.e. for some touchpads it takes about 2 secs to reset
> > >and configure it). I can offload that part into async_schedule() so it does not
> > >stop initialization of the rest of the system (why would I want to delay
> > >initializing of USB or storage system until touchpad is ready?) but if that
> > >initialization fails we end up with partially bound driver and device that is
> > >not really operable. I would very much prefer async and sync cases be the same
> > >- if probe() fails the driver is not bound to the device.
> > >
> > >I think it is wrong to make async probing system-wide, but driver opt-in shoudl
> > >be fine and right thing to do.
> > >
> > 
> > I am completely fine if we make basically an async wrapper for
> > pci_register_driver() and friends.. that would be convenient I suppose.
> > 
> > (but then again, in reality very few drivers take real time to init... most already
> > do the heavy work in open(). Not all can, sure, but if you look at a bootgraph.pl
> > graph of a typical boot it's only a few that matter).

Input devices normally can't as we need to publish their capabilities before
users start opening them.

> > And many drivers need to register with a subsystem, and there's some ordering around that,
> > and that's why we ended up with the async cookie stuff, so that you can do the
> > heavy work in parallel, but order near the end at registeration-with-the-subsystem time.
> > 
> > But doing this on an initcall level was wrong back then, and I have yet to hear
> > a reason why it would be right this time.
> 
> It's still wrong, it's not what I was thinking about when talking this
> over with Luis and Dmitry, I think something got lost in the
> translation...

Right, all (well almost all) I wanted is for individual drivers to declare
their probe() functions asynchronous and driver core scheduling async attach
and properly handle failures from it.

As I mentioned, resume has similar issues...

Thanks.

-- 
Dmitry

Re: [RFC v1 0/3] driver-core: add asynch module loading support

[Greg KH]

On Sun, Aug 31, 2014 at 01:14:07PM -0700, Dmitry Torokhov wrote:
> On Sun, Aug 31, 2014 at 12:31:40PM -0700, Greg KH wrote:
> > On Sun, Aug 31, 2014 at 12:24:46PM -0700, Arjan van de Ven wrote:
> > > >>before we added the current async approach the approach of async init calls was tried
> > > >>At the time, Linus hated it and he was right, it was not the right thing.
> > > >>
> > > >>What is different this time to make this the right thing to do ?
> > > >
> > > >Because otherwise drivers still have to do this, but open code it. Let's say I
> > > >have a long operations (i.e. for some touchpads it takes about 2 secs to reset
> > > >and configure it). I can offload that part into async_schedule() so it does not
> > > >stop initialization of the rest of the system (why would I want to delay
> > > >initializing of USB or storage system until touchpad is ready?) but if that
> > > >initialization fails we end up with partially bound driver and device that is
> > > >not really operable. I would very much prefer async and sync cases be the same
> > > >- if probe() fails the driver is not bound to the device.
> > > >
> > > >I think it is wrong to make async probing system-wide, but driver opt-in shoudl
> > > >be fine and right thing to do.
> > > >
> > > 
> > > I am completely fine if we make basically an async wrapper for
> > > pci_register_driver() and friends.. that would be convenient I suppose.
> > > 
> > > (but then again, in reality very few drivers take real time to init... most already
> > > do the heavy work in open(). Not all can, sure, but if you look at a bootgraph.pl
> > > graph of a typical boot it's only a few that matter).
> 
> Input devices normally can't as we need to publish their capabilities before
> users start opening them.
> 
> > > And many drivers need to register with a subsystem, and there's some ordering around that,
> > > and that's why we ended up with the async cookie stuff, so that you can do the
> > > heavy work in parallel, but order near the end at registeration-with-the-subsystem time.
> > > 
> > > But doing this on an initcall level was wrong back then, and I have yet to hear
> > > a reason why it would be right this time.
> > 
> > It's still wrong, it's not what I was thinking about when talking this
> > over with Luis and Dmitry, I think something got lost in the
> > translation...
> 
> Right, all (well almost all) I wanted is for individual drivers to declare
> their probe() functions asynchronous and driver core scheduling async attach
> and properly handle failures from it.

Yes, that's what I want as well.

Luis, care to redo the patches in this way?  It should be a lot simpler
(no messing around with init levels and linker fun...)

thanks,

greg k-h

Re: [RFC v1 0/3] driver-core: add asynch module loading support

[Tejun Heo]

Hello, Greg.

On Sun, Aug 31, 2014 at 01:40:35PM -0700, Greg KH wrote:
> > Right, all (well almost all) I wanted is for individual drivers to declare
> > their probe() functions asynchronous and driver core scheduling async attach
> > and properly handle failures from it.
> 
> Yes, that's what I want as well.
> 
> Luis, care to redo the patches in this way?  It should be a lot simpler
> (no messing around with init levels and linker fun...)

I don't think binding that switch to the driver is gonna work.  This
is mainly about the behavior expected by the entity which initiated
module loading not about specific drivers.  I'm fairly certain that
there are userland scripts which depend on synchronous device probing
on module loading, especially for storage devices, so we can't simply
mark, say, libata as needing async probing and do it always
asynchronously.

Thanks.

-- 
tejun

Re: [RFC v1 0/3] driver-core: add asynch module loading support

[Greg KH]

On Sun, Aug 31, 2014 at 05:53:13PM -0400, Tejun Heo wrote:
> Hello, Greg.
> 
> On Sun, Aug 31, 2014 at 01:40:35PM -0700, Greg KH wrote:
> > > Right, all (well almost all) I wanted is for individual drivers to declare
> > > their probe() functions asynchronous and driver core scheduling async attach
> > > and properly handle failures from it.
> > 
> > Yes, that's what I want as well.
> > 
> > Luis, care to redo the patches in this way?  It should be a lot simpler
> > (no messing around with init levels and linker fun...)
> 
> I don't think binding that switch to the driver is gonna work.  This
> is mainly about the behavior expected by the entity which initiated
> module loading not about specific drivers.  I'm fairly certain that
> there are userland scripts which depend on synchronous device probing
> on module loading, especially for storage devices, so we can't simply
> mark, say, libata as needing async probing and do it always
> asynchronously.

For the use cases we have today, it would work.  We have a few drivers
that take a _long_ time in their probe callback, and they need to be
made async for various reasons (modprobe timeout killer, touchscreen
init sequence stalling boot, etc.)

I'm not saying to mark drivers that require synchronous probing with
this flag, that would be broken and wrong.

thanks,

greg k-h

Re: [RFC v1 0/3] driver-core: add asynch module loading support

[Tejun Heo]

Hello,

On Sun, Aug 31, 2014 at 03:15:34PM -0700, Greg KH wrote:
> For the use cases we have today, it would work.  We have a few drivers
> that take a _long_ time in their probe callback, and they need to be
> made async for various reasons (modprobe timeout killer, touchscreen
> init sequence stalling boot, etc.)

Oh, but there already is a reported case which fails.  Hard drives
with a lot of platters take > 10secs to spin up and there are
configurations which regularly fail the initial reset and it's quite
rare but probing time taking over 30s does happen in the wild.  We
can't mark libata as requiring async probing on module load
unconditionally but at the same time we need them to be asynchronous
for use cases which don't depend on synchronous behavior has has
timeout.

> I'm not saying to mark drivers that require synchronous probing with
> this flag, that would be broken and wrong.

I don't think we can distinguish the two sets by drivers.

Thanks.

-- 
tejun

Re: [RFC v1 0/3] driver-core: add asynch module loading support

[Arjan van de Ven]

On 8/31/2014 3:53 PM, Tejun Heo wrote:
> Hello,
>
> On Sun, Aug 31, 2014 at 03:15:34PM -0700, Greg KH wrote:
>> For the use cases we have today, it would work.  We have a few drivers
>> that take a _long_ time in their probe callback, and they need to be
>> made async for various reasons (modprobe timeout killer, touchscreen
>> init sequence stalling boot, etc.)
>
> Oh, but there already is a reported case which fails.  Hard drives
> with a lot of platters take > 10secs to spin up and there are
> configurations which regularly fail the initial reset and it's quite
> rare but probing time taking over 30s does happen in the wild.  We
> can't mark libata as requiring async probing on module load
> unconditionally but at the same time we need them to be asynchronous
> for use cases which don't depend on synchronous behavior has has
> timeout.

libata probing is already async thankfully.

Re: [RFC v1 0/3] driver-core: add asynch module loading support

[Tejun Heo]

Hello,

On Sun, Aug 31, 2014 at 04:20:08PM -0700, Arjan van de Ven wrote:
> >Oh, but there already is a reported case which fails.  Hard drives
> >with a lot of platters take > 10secs to spin up and there are
> >configurations which regularly fail the initial reset and it's quite
> >rare but probing time taking over 30s does happen in the wild.  We
> >can't mark libata as requiring async probing on module load
> >unconditionally but at the same time we need them to be asynchronous
> >for use cases which don't depend on synchronous behavior has has
> >timeout.
> 
> libata probing is already async thankfully.

Heh, why is there so much confusion on what the problem is in this
thread?

libata probing is fully parallelized but is still synchronous against
module probing.  module probing performs async_synchronize_full() if
the the module loading thread did any async_schedule() calls and we
need that behavior for backward compatibility; however, the exact same
behavior is causing problem because it can take longer than 30s, so we
need a way to separate out probing from loading so that probing
actually becomes asynchronous against module loading.

Thanks.

-- 
tejun

Re: [RFC v1 0/3] driver-core: add asynch module loading support

[Dmitry Torokhov]

On Sun, Aug 31, 2014 at 05:53:13PM -0400, Tejun Heo wrote:
> Hello, Greg.
> 
> On Sun, Aug 31, 2014 at 01:40:35PM -0700, Greg KH wrote:
> > > Right, all (well almost all) I wanted is for individual drivers to declare
> > > their probe() functions asynchronous and driver core scheduling async attach
> > > and properly handle failures from it.
> > 
> > Yes, that's what I want as well.
> > 
> > Luis, care to redo the patches in this way?  It should be a lot simpler
> > (no messing around with init levels and linker fun...)
> 
> I don't think binding that switch to the driver is gonna work.  This
> is mainly about the behavior expected by the entity which initiated
> module loading not about specific drivers.

Why would you say that? In my particular userspace we do not have modules.

>  I'm fairly certain that
> there are userland scripts which depend on synchronous device probing
> on module loading, especially for storage devices, so we can't simply
> mark, say, libata as needing async probing and do it always
> asynchronously.

Right. But for many devices (input ones for example, USB as well, etc) that are
essentially hot-pluggable userspace has been long ready to handle async order.

Thanks.

-- 
Dmitry

Re: [RFC v1 0/3] driver-core: add asynch module loading support

[Tejun Heo]

Hello, Dmitry.

On Sun, Aug 31, 2014 at 03:51:40PM -0700, Dmitry Torokhov wrote:
> > > Luis, care to redo the patches in this way?  It should be a lot simpler
> > > (no messing around with init levels and linker fun...)
> > 
> > I don't think binding that switch to the driver is gonna work.  This
> > is mainly about the behavior expected by the entity which initiated
> > module loading not about specific drivers.
> 
> Why would you say that? In my particular userspace we do not have modules.

Heh, I think we're just thinking about different problems.  For me,
this is all from the bug report that I was cc'd on where systemd times
out module load after 30s and sends SIG_KILL to the module loading
thread which aborts module loading which in turn leads to nasty
things.  The logical way to address that is separating out module
loading and device probing.  Given the way we associate drivers with
devices these days, it makes a lot of sense to separate them out
anyway.

> >  I'm fairly certain that
> > there are userland scripts which depend on synchronous device probing
> > on module loading, especially for storage devices, so we can't simply
> > mark, say, libata as needing async probing and do it always
> > asynchronously.
> 
> Right. But for many devices (input ones for example, USB as well, etc) that are
> essentially hot-pluggable userspace has been long ready to handle async order.

Sure, all modern userspace should be completely fine without ordering
but there are niche userspaces out in the wild which don't do much of
the dynamic things and just expect devices to be assigned consistent
nodes across multiple boots, so keeping the order is mostly about
backward compatibility.

Thanks.

-- 
tejun

Re: [RFC v1 0/3] driver-core: add asynch module loading support

[Luis R. Rodriguez]

On Sun, Aug 31, 2014 at 01:40:35PM -0700, Greg KH wrote:
> On Sun, Aug 31, 2014 at 01:14:07PM -0700, Dmitry Torokhov wrote:
> > On Sun, Aug 31, 2014 at 12:31:40PM -0700, Greg KH wrote:
> > > On Sun, Aug 31, 2014 at 12:24:46PM -0700, Arjan van de Ven wrote:
> > > > >>before we added the current async approach the approach of async init calls was tried
> > > > >>At the time, Linus hated it and he was right, it was not the right thing.
> > > > >>
> > > > >>What is different this time to make this the right thing to do ?
> > > > >
> > > > >Because otherwise drivers still have to do this, but open code it. Let's say I
> > > > >have a long operations (i.e. for some touchpads it takes about 2 secs to reset
> > > > >and configure it). I can offload that part into async_schedule() so it does not
> > > > >stop initialization of the rest of the system (why would I want to delay
> > > > >initializing of USB or storage system until touchpad is ready?) but if that
> > > > >initialization fails we end up with partially bound driver and device that is
> > > > >not really operable. I would very much prefer async and sync cases be the same
> > > > >- if probe() fails the driver is not bound to the device.
> > > > >
> > > > >I think it is wrong to make async probing system-wide, but driver opt-in shoudl
> > > > >be fine and right thing to do.
> > > > >
> > > > 
> > > > I am completely fine if we make basically an async wrapper for
> > > > pci_register_driver() and friends.. that would be convenient I suppose.
> > > > 
> > > > (but then again, in reality very few drivers take real time to init... most already
> > > > do the heavy work in open(). Not all can, sure, but if you look at a bootgraph.pl
> > > > graph of a typical boot it's only a few that matter).
> > 
> > Input devices normally can't as we need to publish their capabilities before
> > users start opening them.
> > 
> > > > And many drivers need to register with a subsystem, and there's some ordering around that,
> > > > and that's why we ended up with the async cookie stuff, so that you can do the
> > > > heavy work in parallel, but order near the end at registeration-with-the-subsystem time.
> > > > 
> > > > But doing this on an initcall level was wrong back then, and I have yet to hear
> > > > a reason why it would be right this time.
> > > 
> > > It's still wrong, it's not what I was thinking about when talking this
> > > over with Luis and Dmitry, I think something got lost in the
> > > translation...
> > 
> > Right, all (well almost all) I wanted is for individual drivers to declare
> > their probe() functions asynchronous and driver core scheduling async attach
> > and properly handle failures from it.
> 
> Yes, that's what I want as well.
> 
> Luis, care to redo the patches in this way?  It should be a lot simpler
> (no messing around with init levels and linker fun...)

Sure, when we spoke the requirement was indeed clear but as I noted on
the description on this cover letter as I reviewed Wu Zhangjin's RFC
which used async_schedule() he ran into the issue of eventually grouping
things. I went with this approach as a first shot given that we have a long
tested set of groupings already done by the kernel so this tried to take
advantage of that should we try to scale on asynchronous probing.

I still believe its a good approach if we wanted to scale it but that
would require the desire to do so, I obviously considered it worthwhile
as I shaved off at least ~1 second off kernel boot time when doing an
original proof of concept and only doing drivers, and taking into
consideration my kenrel takes ~5 seconds before userspace is reached.
I'll toss this in the bin for now though and send something based on
Tejun's approach shortly!

  Luis

Re: [RFC v1 0/3] driver-core: add asynch module loading support

[Greg KH]

On Thu, Sep 04, 2014 at 11:21:18PM +0200, Luis R. Rodriguez wrote:
> I still believe its a good approach if we wanted to scale it but that
> would require the desire to do so, I obviously considered it worthwhile
> as I shaved off at least ~1 second off kernel boot time when doing an
> original proof of concept and only doing drivers, and taking into
> consideration my kenrel takes ~5 seconds before userspace is reached.

Sidenote, something is really wrong with your hardware or configuration
if it takes 5 seconds to get to userspace on a modern system these days.
Everything[1] I run is in the sub-second boot time before userspace is
hit, and everything boots to full graphics mode in less than 5 seconds.
The long-pole is by far, the bios/uefi these days.

I think you need new hardware, or a better kernel .config file :)

> I'll toss this in the bin for now though and send something based on
> Tejun's approach shortly!

That would be great, thanks for doing this.

greg k-h

[1] A wide range of laptops and server/build boxes, some 3+ years old,
this isn't just new hardware by any means.

Re: [RFC v1 0/3] driver-core: add asynch module loading support

[Greg KH]

On Sun, Aug 31, 2014 at 02:03:17AM -0700, Luis R. Rodriguez wrote:
> From: "Luis R. Rodriguez" <mcgrof@suse.com>
> 
> While reviewing Wu Zhangjin's solution to async probe [0] and his
> ideas on creating async groups I decided to try following the init
> levels on the kernel to try to help with synchronization at least
> on some level. This borrows ideas discussed with the kthread_create()
> solution [1] and also simplifies the idea of how we should be grouping
> async calls between drivers.
> 
> A few things worth mentioning. I decided to go down a generic solution as
> if we ever wanted to bring asynchrnonization behaviour below modules
> this would allow folks to start testing this without much effort. It
> allows asynchronous calls to upkeep the order already set in place
> for built-in code. If one wanted to eventually venture down below
> that path we'd need to add respective exit calls for each level, as
> right now we just assume that every level should use module_exit().
> SmPL grammer could be used to easily tidy this up, for example on
> the subsys_init():
> 
> @ subsys_found @
> expression fn_init;
> declarer name subsys_initcall;
> @@
> 
>   subsys_initcall(fn_init);                                                     
> 
> @ subsys_exit_found depends on subsys_found @
> expression fn_exit;
> declarer name module_exit;
> declarer name subsys_exitcall;
> @@                                                                              
>                                                                                 
> - module_exit(fn_exit);
> + subsys_exitcall(fn_exit);
> 
> And so on. The second thing to note is that when an asynchronous
> call is used we don't want to free the init data as otherwise
> async_run_entry_fn() will run into a missing init routine. SmPL
> could still be used to convert basic __init data out if this is
> needed, however if the init routine also used other init data
> we'd also have to remove the other __init data used. It could
> in theory be possible to do a witch hunt to write this in grammar
> but for now only a simple conversion on the init side is recommended
> as reflected in the SmPL below and manual inspection after that.
> Another option as suggested by Julia to me was to consider an
> __init_asynch which we could reap later. This is of course if
> we go down this generic path.
> 
> @ module_init_found @
> identifier f;
> declarer name module_init;
> declarer name module_init_async;
> @@
> 
> -   module_init(f);
> +   module_init_async(f);
> 
> @ modify_decl depends on module_init_found @
> identifier module_init_found.f;
> @@
> 
> - int f
> + int f
>     (...) { ... }
> 
> 
> @ module_exit_found depends on module_init_found @
> identifier fn_exit;
> declarer name module_exit;
> declarer name module_exit_async;
> @@
> 
> - module_exit(fn_exit);
> + module_exit_async(fn_exit);
> 
> Although not visible the above int f does remove the __init...
> 
> If generalizing an async solution is not desirable in the
> long run for things other than modules than a bool should
> be easy to use to figure if probe should run async'd or not.
> Grouping however still becomes a question then, and this is
> why I went with this approach as it aligns itself more closely
> to the kernel init levels and that should be well tested. In
> theory it could even be possible to use a similar strategy to
> asynch on per init level when built-in using a similar strategy
> but these would have to be separate.

This is a much larger change than I was wanting, I just wanted a change
to 'struct driver' to add a new flag that a driver could then use to say
if it can be async probed or not by the driver core.

Closer to Tejun's patch in this email series, but toggleable on a
per-driver basis, if for no other reason than David's response about
some drivers _having_ to be bound to their device by the time module
init returns.

thanks,

greg k-h