Re: [patch 3/4] fastboot: make the raid autodetect code wait for all devices to init

Re: [patch 3/4] fastboot: make the raid autodetect code wait for all devices to init

[Arjan van de Ven]

On Mon, 21 Jul 2008 14:30:12 +1000
Neil Brown <neilb@suse.de> wrote:

> 
> > +		while (driver_probe_done())
> >  			msleep(100);
> 
> That's worse.  Now it really looks like a boolean that is being used
> wrongly.  It seems that driver_probe_done either returns 0 or -EBUSY,
> so I'd prefer
> 
> > +		while (driver_probe_done() < 0)
> >  			msleep(100);
> 
> or even

ok I've done this one.

I'll be the first to say that this driver_probe_done() isn't very
elegant.. but it's out of my current scope to rewrite that whole
thing ... I'll leave that to GregKH.


-- 
If you want to reach me at my work email, use arjan@linux.intel.com
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

[patch 1/4] fastboot: make fastboot a config option

[Arjan van de Ven]

From: Arjan van de Ven <arjan@linux.intel.com>
Date: Sun, 20 Jul 2008 10:20:49 -0700
Subject: [PATCH] fastboot: make fastboot a config option

to mitigate the risks of async bootup, make fastboot a configuration
option...

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 init/Kconfig |   11 +++++++++++
 init/main.c  |    4 ++++
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/init/Kconfig b/init/Kconfig
index 6199d11..7545c8b 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -524,6 +524,17 @@ config CC_OPTIMIZE_FOR_SIZE
 
 	  If unsure, say N.
 
+config FASTBOOT
+	bool "Fast boot support"
+	help
+	  The fastboot option will cause the kernel to try to optimize
+	  for faster boot. 
+
+	  This includes doing some of the device initialization asynchronous
+	  as well as opportunistically trying to mount the root fs early.
+
+	  If unsure, say N.
+
 config SYSCTL
 	bool
 
diff --git a/init/main.c b/init/main.c
index 6be1756..bb97add 100644
--- a/init/main.c
+++ b/init/main.c
@@ -776,7 +776,11 @@ static void __init do_initcalls(void)
 	for (call = __initcall_start; call < __initcall_end; call++) {
 		if (phase == 0 && call >= __async_initcall_start) {
 			phase = 1;
+#ifdef CONFIG_FASTBOOT
 			queue_work(async_init_wq, &async_work);
+#else
+			do_async_initcalls(NULL);
+#endif
 		}
 		if (phase == 1 && call >= __async_initcall_end)
 			phase = 2;
-- 
1.5.5.1

[patch 2/4] fastboot: retry mounting the root fs if we can't find init

[Arjan van de Ven]

From: Arjan van de Ven <arjan@linux.intel.com>
Date: Sun, 20 Jul 2008 13:01:28 -0700
Subject: [PATCH] fastboot: retry mounting the root fs if we can't find init

currently we wait until all device init is done before trying to mount
the root fs, and to consequently execute init.

In preparation for relaxing the first delay, this patch adds a retry
attempt in case /sbin/init is not found. Before retrying, the code
will wait for all device init to complete.

While this patch by itself doesn't gain boot time yet (it needs follow on
patches), the alternative already is to panic()...

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---
 init/main.c |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/init/main.c b/init/main.c
index 3575b84..73785a4 100644
--- a/init/main.c
+++ b/init/main.c
@@ -853,6 +853,7 @@ static void run_init_process(char *init_filename)
  */
 static int noinline init_post(void)
 {
+	int retry_count = 1;
 	free_initmem();
 	unlock_kernel();
 	mark_rodata_ro();
@@ -873,6 +874,7 @@ static int noinline init_post(void)
 				ramdisk_execute_command);
 	}
 
+retry:
 	/*
 	 * We try each of these until one succeeds.
 	 *
@@ -885,6 +887,23 @@ static int noinline init_post(void)
 					"defaults...\n", execute_command);
 	}
 	run_init_process("/sbin/init");
+
+	if (retry_count > 0) {
+		retry_count--;
+		/*
+		 * We haven't found init yet... potentially because the device
+		 * is still being probed. We need to
+		 * - flush keventd and friends
+		 * - wait for the known devices to complete their probing
+		 * - try to mount the root fs again
+		 */
+		flush_scheduled_work();
+		while (driver_probe_done() != 0)
+			msleep(100);
+		prepare_namespace();
+		goto retry;
+	}
+
 	run_init_process("/etc/init");
 	run_init_process("/bin/init");
 	run_init_process("/bin/sh");
-- 
1.5.5.1

[patch 3/4] fastboot: make the raid autodetect code wait for all devices to init

[Arjan van de Ven]

From: Arjan van de Ven <arjan@linux.intel.com>
Date: Sun, 20 Jul 2008 13:07:09 -0700
Subject: [PATCH] fastboot: make the raid autodetect code wait for all devices to init

The raid autodetect code really needs to have all devices probed before
it can detect raid arrays; not doing so would give rather messy situations
where arrays would get detected as degraded while they shouldn't be etc.

This is in preparation of removing the "wait for everything to init"
code that makes everyone pay, not just raid users.

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---
 init/do_mounts_md.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/init/do_mounts_md.c b/init/do_mounts_md.c
index 693d246..c0412a9 100644
--- a/init/do_mounts_md.c
+++ b/init/do_mounts_md.c
@@ -267,9 +267,16 @@ __setup("md=", md_setup);
 void __init md_run_setup(void)
 {
 	create_dev("/dev/md0", MKDEV(MD_MAJOR, 0));
+
 	if (raid_noautodetect)
 		printk(KERN_INFO "md: Skipping autodetection of RAID arrays. (raid=noautodetect)\n");
 	else {
+		/*
+		 * Since we don't want to detect and use half a raid array, we
+		 * need to wait for the known devices to complete their probing
+		 */
+		while (driver_probe_done() != 0)
+			msleep(100);
 		int fd = sys_open("/dev/md0", 0, 0);
 		if (fd >= 0) {
 			sys_ioctl(fd, RAID_AUTORUN, raid_autopart);
-- 
1.5.5.1

[patch 4/4] fastboot: remove "wait for all devices before mounting root" delay

[Arjan van de Ven]

From: Arjan van de Ven <arjan@linux.intel.com>
Date: Sun, 20 Jul 2008 13:12:16 -0700
Subject: [PATCH] fastboot: remove "wait for all devices before mounting root" delay

In the non-initrd case, we wait for all devices to finish their
probing before we try to mount the rootfs.
In practice, this means that we end up waiting 2 extra seconds for
the PS/2 mouse probing even though the root holding device has been
ready since a long time.

The previous two patches in this series made the RAID autodetect code
do it's own "wait for probing to be done" code, and added
"wait and retry" functionality in case the root device isn't actually
available.

These two changes should make it safe to remove the delay itself,
and this patch does this. On my test laptop, this reduces the boot time
by 2 seconds (kernel time goes from 3.9 to 1.9 seconds).

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---
 init/do_mounts.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/init/do_mounts.c b/init/do_mounts.c
index a1de1bf..c984fab 100644
--- a/init/do_mounts.c
+++ b/init/do_mounts.c
@@ -364,9 +364,11 @@ void __init prepare_namespace(void)
 		ssleep(root_delay);
 	}
 
+#ifndef CONFIG_FASTBOOT
 	/* wait for the known devices to complete their probing */
 	while (driver_probe_done() != 0)
 		msleep(100);
+#endif
 
 	md_run_setup();
 
-- 
1.5.5.1

Re: [patch 3/4] fastboot: make the raid autodetect code wait for all devices to init

[Neil Brown]

(wondering why I wasn't Cc:ed on this...)

On Sunday July 20, arjan@infradead.org wrote:
> 
> From: Arjan van de Ven <arjan@linux.intel.com>
> Date: Sun, 20 Jul 2008 13:07:09 -0700
> Subject: [PATCH] fastboot: make the raid autodetect code wait for all devices to init
> 
> The raid autodetect code really needs to have all devices probed before
> it can detect raid arrays; not doing so would give rather messy situations
> where arrays would get detected as degraded while they shouldn't be etc.
> 
> This is in preparation of removing the "wait for everything to init"
> code that makes everyone pay, not just raid users.
> 
> Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
> ---
>  init/do_mounts_md.c |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/init/do_mounts_md.c b/init/do_mounts_md.c
> index 693d246..c0412a9 100644
> --- a/init/do_mounts_md.c
> +++ b/init/do_mounts_md.c
> @@ -267,9 +267,16 @@ __setup("md=", md_setup);
>  void __init md_run_setup(void)
>  {
>  	create_dev("/dev/md0", MKDEV(MD_MAJOR, 0));
> +
>  	if (raid_noautodetect)
>  		printk(KERN_INFO "md: Skipping autodetection of RAID arrays. (raid=noautodetect)\n");
>  	else {
> +		/*
> +		 * Since we don't want to detect and use half a raid array, we
> +		 * need to wait for the known devices to complete their probing
> +		 */
> +		while (driver_probe_done() != 0)
> +			msleep(100);
>  		int fd = sys_open("/dev/md0", 0, 0);
>  		if (fd >= 0) {
>  			sys_ioctl(fd, RAID_AUTORUN, raid_autopart);

I must say that I think this is pretty horrible.   But then it is a
pretty horrible problem and I don't think there is a clean solution.

If md in a module, this code won't run so there will be no change.  If
md is compiled in, this code will silently slow down boot even if
there are no raid arrays to assemble.  I think the "silently" is a
problem.  I'm not looking forward to "my computer boots slower if I
compile md into the kernel" reports on linux-raid@vger.

What would you think of

    if (driver_probe_done() != 0) {
	printk("md: Waiting for all devices to be available before autodetect\n"
               "md:  If you don't boot off raid, use raid=noautodetect\n");
        do
           msleep(100);
	while (driver_probe_done() != 0);
    }

??

Also, the "driver_probe_done() != 0" bothers me.

If driver_probe_done is a boolean, it should be
	while (! drive_probe_done() ) msleep ....

and if it returns a negative error, then it should be
        while ( drive_probe_done() < 0) msleep ....

The != 0 confuses me about the expected return type.


The "real" solution here involves assembling arrays in userspace using
"mdadm --incremental" from udevd, and using write-intent-bitmaps so
that writing to an array before all the component devices are
available can be done without requiring a full resync.  There is still
a bit more code needed to make that work really smoothly.

NeilBrown

Re: [patch 3/4] fastboot: make the raid autodetect code wait for all devices to init

[david]

On Mon, 21 Jul 2008, Neil Brown wrote:

> I must say that I think this is pretty horrible.   But then it is a
> pretty horrible problem and I don't think there is a clean solution.
>
> If md in a module, this code won't run so there will be no change.  If
> md is compiled in, this code will silently slow down boot even if
> there are no raid arrays to assemble.  I think the "silently" is a
> problem.  I'm not looking forward to "my computer boots slower if I
> compile md into the kernel" reports on linux-raid@vger.
>
> What would you think of
>
>    if (driver_probe_done() != 0) {
> 	printk("md: Waiting for all devices to be available before autodetect\n"
>               "md:  If you don't boot off raid, use raid=noautodetect\n");
>        do
>           msleep(100);
> 	while (driver_probe_done() != 0);
>    }
>
> ??

even if you don't boot off of raid, if the boot process will mount a raid 
that's autodetected this will be a problem. so I think the message needs 
to allow for this.

David Lang

Re: [patch 3/4] fastboot: make the raid autodetect code wait for all devices to init

[Arjan van de Ven]

On Mon, 21 Jul 2008 09:06:36 +1000
Neil Brown <neilb@suse.de> wrote:

> > +		/*
> > +		 * Since we don't want to detect and use half a
> > raid array, we
> > +		 * need to wait for the known devices to complete
> > their probing
> > +		 */
> > +		while (driver_probe_done() != 0)
> > +			msleep(100);
> >  		int fd = sys_open("/dev/md0", 0, 0);
> >  		if (fd >= 0) {
> >  			sys_ioctl(fd, RAID_AUTORUN, raid_autopart);
> 
> I must say that I think this is pretty horrible.   But then it is a
> pretty horrible problem and I don't think there is a clean solution.

it's also the existing solution, just moved around to the only place
that needs it.

> 
> If md in a module, this code won't run so there will be no change.  If
> md is compiled in, this code will silently slow down boot even if
> there are no raid arrays to assemble. 

it's not slower than it is is today for *everyone*. All that this does
is move it to the MD code rather than having it 1 line before calling
the MD code (the removal of the global wait is in the next patch)
Waiting twice is the same cost as waiting once; the moment everything
is done it'll stay done.

> I think the "silently" is a
> problem.  I'm not looking forward to "my computer boots slower if I
> compile md into the kernel" reports on linux-raid@vger.

I *am* looking forward to "my computer boots faster" reports though ;-)

> 
> What would you think of
> 
>     if (driver_probe_done() != 0) {
> 	printk("md: Waiting for all devices to be available before
> autodetect\n" "md:  If you don't boot off raid, use
> raid=noautodetect\n"); do
>            msleep(100);
> 	while (driver_probe_done() != 0);
>     }

not sure we need the outer if for this (it'll basically always hit, and
even if by some miracle, everything is done, it's no big deal, you
tried to wait and were done immediately).
Adding a printk like this makes total sense to me; I'll add a patch for
that.

Maybe it's worth introducing a config option for the raid autodetect
thing; but I'll leave that up to you.


> Also, the "driver_probe_done() != 0" bothers me.

it's the existing form just moved; I wanted to make the change to be as
simple as possible.

> 
> The "real" solution here involves assembling arrays in userspace using
> "mdadm --incremental" from udevd, and using write-intent-bitmaps so
> that writing to an array before all the component devices are
> available can be done without requiring a full resync.  There is still
> a bit more code needed to make that work really smoothly.

if you use an initrd, the code above doesn't run...
totally different method of booting.

-- 
If you want to reach me at my work email, use arjan@linux.intel.com
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [patch 3/4] fastboot: make the raid autodetect code wait for all devices to init

[Arjan van de Ven]

On Mon, 21 Jul 2008 09:06:36 +1000
Neil Brown <neilb@suse.de> wrote:

>     if (driver_probe_done() != 0) {
> 	printk("md: Waiting for all devices to be available before
> autodetect\n" "md:  If you don't boot off raid, use
> raid=noautodetect\n"); do


how about this patch?

From: Arjan van de Ven <arjan@linux.intel.com>
Date: Sun, 20 Jul 2008 16:30:29 -0700
Subject: [PATCH] fastboot: make the RAID autostart code print a message just before waiting

As requested/suggested by Neil Brown: make the raid code print that it's
about to wait for probing to be done as well as give a suggestion on how
to disable the probing if the user doesn't use raid.

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com
---
 init/do_mounts_md.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/init/do_mounts_md.c b/init/do_mounts_md.c
index c0412a9..2ab122a 100644
--- a/init/do_mounts_md.c
+++ b/init/do_mounts_md.c
@@ -275,7 +275,9 @@ void __init md_run_setup(void)
 		 * Since we don't want to detect and use half a raid array, we need to
 		 * wait for the known devices to complete their probing
 		 */
-		while (driver_probe_done() != 0)
+		printk(KERN_INFO "md: Waiting for all devices to be available before autodetect\n");
+		printk(KERN_INFO "md: If you don't use raid, use raid=noautodetect\n");
+		while (driver_probe_done())
 			msleep(100);
 		int fd = sys_open("/dev/md0", 0, 0);
 		if (fd >= 0) {
-- 
1.5.5.1

Re: [patch 2/4] fastboot: retry mounting the root fs if we can't find init

[Daniel Walker]

On Sun, 2008-07-20 at 15:13 -0700, Arjan van de Ven wrote:
> +               /*
> +                * We haven't found init yet... potentially because the device
> +                * is still being probed. We need to
> +                * - flush keventd and friends
> +                * - wait for the known devices to complete their probing
> +                * - try to mount the root fs again
> +                */
> +               flush_scheduled_work();
> +               while (driver_probe_done() != 0)
> +                       msleep(100);

I realize the while() loop above is just code shifted from
init/do_mounts_md.c , but I'm wondering if that could be a complete
instead of a while then sleep 100ms .. That would reduce the overshoot .

Daniel

Re: [patch 3/4] fastboot: make the raid autodetect code wait for all devices to init

[Neil Brown]

On Sunday July 20, arjan@infradead.org wrote:
> On Mon, 21 Jul 2008 09:06:36 +1000
> Neil Brown <neilb@suse.de> wrote:
> 
> >     if (driver_probe_done() != 0) {
> > 	printk("md: Waiting for all devices to be available before
> > autodetect\n" "md:  If you don't boot off raid, use
> > raid=noautodetect\n"); do
> 
> 
> how about this patch?

Well,

> -		while (driver_probe_done() != 0)
> +		printk(KERN_INFO "md: Waiting for all devices to be available before autodetect\n");
> +		printk(KERN_INFO "md: If you don't use raid, use raid=noautodetect\n");

that's better thanks.  I'm quite happy with that.


> +		while (driver_probe_done())
>  			msleep(100);

That's worse.  Now it really looks like a boolean that is being used
wrongly.  It seems that driver_probe_done either returns 0 or -EBUSY,
so I'd prefer

> +		while (driver_probe_done() < 0)
>  			msleep(100);

or even

> +		while (driver_probe_done() == -EBUSY)
>  			msleep(100);

though that is less robust.

Thanks,
NeilBrown

Re: [patch 1/4] fastboot: make fastboot a config option

[Randy Dunlap]

On Sun, 20 Jul 2008 15:12:34 -0700 Arjan van de Ven wrote:

> 
> From: Arjan van de Ven <arjan@linux.intel.com>
> Date: Sun, 20 Jul 2008 10:20:49 -0700
> Subject: [PATCH] fastboot: make fastboot a config option
> 
> to mitigate the risks of async bootup, make fastboot a configuration
> option...
> 
> Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
> Signed-off-by: Ingo Molnar <mingo@elte.hu>
> ---
>  init/Kconfig |   11 +++++++++++
>  init/main.c  |    4 ++++
>  2 files changed, 15 insertions(+), 0 deletions(-)
> 
> diff --git a/init/Kconfig b/init/Kconfig
> index 6199d11..7545c8b 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -524,6 +524,17 @@ config CC_OPTIMIZE_FOR_SIZE
>  
>  	  If unsure, say N.
>  
> +config FASTBOOT
> +	bool "Fast boot support"
> +	help
> +	  The fastboot option will cause the kernel to try to optimize
> +	  for faster boot. 
> +
> +	  This includes doing some of the device initialization asynchronous

	                                                        asynchronously

> +	  as well as opportunistically trying to mount the root fs early.
> +
> +	  If unsure, say N.
> +
>  config SYSCTL
>  	bool
>  
> diff --git a/init/main.c b/init/main.c
> index 6be1756..bb97add 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -776,7 +776,11 @@ static void __init do_initcalls(void)
>  	for (call = __initcall_start; call < __initcall_end; call++) {
>  		if (phase == 0 && call >= __async_initcall_start) {
>  			phase = 1;
> +#ifdef CONFIG_FASTBOOT
>  			queue_work(async_init_wq, &async_work);
> +#else
> +			do_async_initcalls(NULL);
> +#endif
>  		}
>  		if (phase == 1 && call >= __async_initcall_end)
>  			phase = 2;
> -- 

---
~Randy
Linux Plumbers Conference, 17-19 September 2008, Portland, Oregon USA
http://linuxplumbersconf.org/

Re: [patch 1/4] fastboot: make fastboot a config option

[Ingo Molnar]

* Randy Dunlap <randy.dunlap@oracle.com> wrote:

> > +	  This includes doing some of the device initialization asynchronous
> 
> 	                                                        asynchronously
> 

fixed in tip/fastboot via the patch below - thanks Randy.

	Ingo

------------->
>From 1a23ed42e1baf0481cc70c2f71d97b0bf0f1be70 Mon Sep 17 00:00:00 2001
From: Ingo Molnar <mingo@elte.hu>
Date: Thu, 31 Jul 2008 12:52:29 +0200
Subject: [PATCH] fastboot: fix typo in init/Kconfig text

noticed by Randy Dunlap.

Reported-by: Randy Dunlap <randy.dunlap@oracle.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 init/Kconfig |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/init/Kconfig b/init/Kconfig
index 4f73780..6b4de4d 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -530,7 +530,7 @@ config FASTBOOT
 	  The fastboot option will cause the kernel to try to optimize
 	  for faster boot. 
 
-	  This includes doing some of the device initialization asynchronous
+	  This includes doing some of the device initialization asynchronously
 	  as well as opportunistically trying to mount the root fs early.
 
 	  If unsure, say N.