[PATCH] Make prepare_namespace() wait for devices

[PATCH] Make prepare_namespace() wait for devices

[Pierre Ossman]

[-- Attachment #1: Type: text/plain, Size: 760 bytes --]

init: wait for asynchronously scanned block devices

Some buses (e.g. USB and MMC) do their scanning of devices in the
background, causing a race between them and prepare_namespace().
In order to be able to use these buses without an initrd, we now
wait for the device specified in root= to actually show up.

If the device never shows up than we will hang in an infinite loop.
In order to not mess with setups that reboot on panic, the feature
must be turned on via the command line option "rootwait".

Signed-off-by: Pierre Ossman <drzeus@drzeus.cx>

-- 
     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  PulseAudio, core developer          http://pulseaudio.org
  rdesktop, core developer          http://www.rdesktop.org

[-- Attachment #2: initwait.patch --]
[-- Type: text/x-patch, Size: 2117 bytes --]

commit bb8c44ee8b4d584295add58a4ea2f03b9938fc3c
Author: Pierre Ossman <drzeus@drzeus.cx>
Date:   Mon May 14 14:15:46 2007 +0200

    init: wait for asynchronously scanned block devices
    
    Some buses (e.g. USB and MMC) do their scanning of devices in the
    background, causing a race between them and prepare_namespace().
    In order to be able to use these buses without an initrd, we now
    wait for the device specified in root= to actually show up.
    
    If the device never shows up than we will hang in an infinite loop.
    In order to not mess with setups that reboot on panic, the feature
    must be turned on via the command line option "rootwait".
    
    Signed-off-by: Pierre Ossman <drzeus@drzeus.cx>

diff --git a/init/do_mounts.c b/init/do_mounts.c
index 46fe407..efc134c 100644
--- a/init/do_mounts.c
+++ b/init/do_mounts.c
@@ -25,6 +25,7 @@ int __initdata rd_doload;	/* 1 = load RAM disk, 0 = don't load */
 int root_mountflags = MS_RDONLY | MS_SILENT;
 char * __initdata root_device_name;
 static char __initdata saved_root_name[64];
+int __initdata root_wait;
 
 dev_t ROOT_DEV;
 
@@ -216,6 +217,14 @@ static int __init root_dev_setup(char *line)
 
 __setup("root=", root_dev_setup);
 
+static int __init rootwait_setup(char *line)
+{
+	root_wait = simple_strtol(line,NULL,0);
+	return 1;
+}
+
+__setup("rootwait=", rootwait_setup);
+
 static char * __initdata root_mount_data;
 static int __init root_data_setup(char *str)
 {
@@ -438,11 +447,24 @@ void __init prepare_namespace(void)
 			root_device_name += 5;
 	}
 
-	is_floppy = MAJOR(ROOT_DEV) == FLOPPY_MAJOR;
-
 	if (initrd_load())
 		goto out;
 
+	/* wait for any asynchronous scanning to complete */
+	if ((ROOT_DEV == 0) && root_wait) {
+		printk(KERN_INFO "Waiting for root device %s...\n",
+			saved_root_name);
+		do {
+			while (driver_probe_done() != 0)
+				msleep(100);
+			ROOT_DEV = name_to_dev_t(saved_root_name);
+			if (ROOT_DEV == 0)
+				msleep(100);
+		} while (ROOT_DEV == 0);
+	}
+
+	is_floppy = MAJOR(ROOT_DEV) == FLOPPY_MAJOR;
+
 	if (is_floppy && rd_doload && rd_load_disk(0))
 		ROOT_DEV = Root_RAM0;
 

Re: [PATCH] Make prepare_namespace() wait for devices

[Olaf Hering]

On Thu, May 24, Pierre Ossman wrote:

> init: wait for asynchronously scanned block devices

init/do_mounts.c:242:__setup("rootdelay=", root_delay_setup);

Why does that not work for you and how does your patch fix it?

Re: [PATCH] Make prepare_namespace() wait for devices

[Pierre Ossman]

Olaf Hering wrote:
> On Thu, May 24, Pierre Ossman wrote:
>
>   
>> init: wait for asynchronously scanned block devices
>>     
>
> init/do_mounts.c:242:__setup("rootdelay=", root_delay_setup);
>
> Why does that not work for you and how does your patch fix it?
>   

The problem isn't really mine, I just got the ball when it came to
fixing it. :)

The problem with rootdelay= is that it's a hammer when the problem
requires a screw driver. In order to get reliability, you need to whack
it hard and give it a long delay. But that has the unpleasant side
effect of having the boot taking way much more time than it really needs.

The original thread is here:

http://marc.info/?t=117879392100022&r=1&w=2

At the end of that thread I suggest a more universal approach, which I
submitted for comments here:

http://marc.info/?t=117914570700004&r=1&w=2

I believe this is a more clean approach than forcing all subsystems to
be synchronous.

Rgds

-- 
     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  PulseAudio, core developer          http://pulseaudio.org
  rdesktop, core developer          http://www.rdesktop.org

Re: [PATCH] Make prepare_namespace() wait for devices

[Andrew Morton]

On Thu, 24 May 2007 14:21:35 +0200
Pierre Ossman <drzeus-list@drzeus.cx> wrote:

> +	/* wait for any asynchronous scanning to complete */
> +	if ((ROOT_DEV == 0) && root_wait) {
> +		printk(KERN_INFO "Waiting for root device %s...\n",
> +			saved_root_name);
> +		do {
> +			while (driver_probe_done() != 0)
> +				msleep(100);
> +			ROOT_DEV = name_to_dev_t(saved_root_name);
> +			if (ROOT_DEV == 0)
> +				msleep(100);
> +		} while (ROOT_DEV == 0);
> +	}

This seems overly complex.  Can't we simply do


	while (driver_probe_done() || ROOT_DEV == 0)
		msleep(100);

?

Re: [PATCH] Make prepare_namespace() wait for devices

[Pierre Ossman]

Andrew Morton wrote:
> On Thu, 24 May 2007 14:21:35 +0200
> Pierre Ossman <drzeus-list@drzeus.cx> wrote:
>
>   
>> +	/* wait for any asynchronous scanning to complete */
>> +	if ((ROOT_DEV == 0) && root_wait) {
>> +		printk(KERN_INFO "Waiting for root device %s...\n",
>> +			saved_root_name);
>> +		do {
>> +			while (driver_probe_done() != 0)
>> +				msleep(100);
>> +			ROOT_DEV = name_to_dev_t(saved_root_name);
>> +			if (ROOT_DEV == 0)
>> +				msleep(100);
>> +		} while (ROOT_DEV == 0);
>> +	}
>>     
>
> This seems overly complex.  Can't we simply do
>
>
> 	while (driver_probe_done() || ROOT_DEV == 0)
> 		msleep(100);
>
> ?
>   

How would ROOT_DEV get updated in that loop?

-- 
     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  PulseAudio, core developer          http://pulseaudio.org
  rdesktop, core developer          http://www.rdesktop.org

Re: [PATCH] Make prepare_namespace() wait for devices

[Andrew Morton]

On Fri, 25 May 2007 06:03:54 +0200 Pierre Ossman <drzeus-list@drzeus.cx> wrote:

> Andrew Morton wrote:
> > On Thu, 24 May 2007 14:21:35 +0200
> > Pierre Ossman <drzeus-list@drzeus.cx> wrote:
> >
> >   
> >> +	/* wait for any asynchronous scanning to complete */
> >> +	if ((ROOT_DEV == 0) && root_wait) {
> >> +		printk(KERN_INFO "Waiting for root device %s...\n",
> >> +			saved_root_name);
> >> +		do {
> >> +			while (driver_probe_done() != 0)
> >> +				msleep(100);
> >> +			ROOT_DEV = name_to_dev_t(saved_root_name);
> >> +			if (ROOT_DEV == 0)
> >> +				msleep(100);
> >> +		} while (ROOT_DEV == 0);
> >> +	}
> >>     
> >
> > This seems overly complex.  Can't we simply do
> >
> >
> > 	while (driver_probe_done() || ROOT_DEV == 0)
> > 		msleep(100);
> >
> > ?
> >   
> 
> How would ROOT_DEV get updated in that loop?
> 

Whatever.  I think you can work it out ;)   

while (driver_probe_done() || (ROOT_DEV = name_to_dev_t(...)) == 0)

perhaps?

The loop-which-sleeps within a loop-which-sleeps seems poorly thought out?

Re: [PATCH] Make prepare_namespace() wait for devices

[Pierre Ossman]

Andrew Morton wrote:
> Whatever.  I think you can work it out ;)   
>
>   

Bare with me, I just woke up ;)

> while (driver_probe_done() || (ROOT_DEV = name_to_dev_t(...)) == 0)
>
> perhaps?
>
> The loop-which-sleeps within a loop-which-sleeps seems poorly thought out?
>   

I'd say a matter of taste. I'm not a big fan och cramming things into
the while() clause.

The idea with the double loops was to keep this thread asleep when we
could detect meaningful work elsewhere in the kernel. You could just
remove the inner-most loop if it offends you. :)

Rgds

-- 
     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  PulseAudio, core developer          http://pulseaudio.org
  rdesktop, core developer          http://www.rdesktop.org

Re: [PATCH] Make prepare_namespace() wait for devices

[Pierre Ossman]

What was the verdict here? Were you satisfied with this or do you need a change?

Pierre Ossman wrote:
> Andrew Morton wrote:
>> Whatever.  I think you can work it out ;)   
>>
>>   
> 
> Bare with me, I just woke up ;)
> 
>> while (driver_probe_done() || (ROOT_DEV = name_to_dev_t(...)) == 0)
>>
>> perhaps?
>>
>> The loop-which-sleeps within a loop-which-sleeps seems poorly thought out?
>>   
> 
> I'd say a matter of taste. I'm not a big fan och cramming things into
> the while() clause.
> 
> The idea with the double loops was to keep this thread asleep when we
> could detect meaningful work elsewhere in the kernel. You could just
> remove the inner-most loop if it offends you. :)
> 

Rgds
-- 
     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  PulseAudio, core developer          http://pulseaudio.org
  rdesktop, core developer          http://www.rdesktop.org

Re: [PATCH] Make prepare_namespace() wait for devices

[Andrew Morton]

On Thu, 31 May 2007 13:20:48 +0200 Pierre Ossman <drzeus-list@drzeus.cx> wrote:
>

(top-posting reversed)

> Pierre Ossman wrote:
> > Andrew Morton wrote:
> >> Whatever.  I think you can work it out ;)   
> >>
> >>   
> > 
> > Bare with me, I just woke up ;)
> > 
> >> while (driver_probe_done() || (ROOT_DEV = name_to_dev_t(...)) == 0)
> >>
> >> perhaps?
> >>
> >> The loop-which-sleeps within a loop-which-sleeps seems poorly thought out?
> >>   
> > 
> > I'd say a matter of taste. I'm not a big fan och cramming things into
> > the while() clause.
> > 
> > The idea with the double loops was to keep this thread asleep when we
> > could detect meaningful work elsewhere in the kernel. You could just
> > remove the inner-most loop if it offends you. :)
> > 
>
> What was the verdict here? Were you satisfied with this or do you need a change?


I was kinda hoing to see version #2 with that funny loop cleaned up a bit?

Re: [PATCH] Make prepare_namespace() wait for devices

[Pierre Ossman]

[-- Attachment #1: Type: text/plain, Size: 614 bytes --]

Andrew Morton wrote:
> On Thu, 31 May 2007 13:20:48 +0200 Pierre Ossman <drzeus-list@drzeus.cx> wrote:
>   
>> What was the verdict here? Were you satisfied with this or do you need a change?
>>     
>
>
> I was kinda hoing to see version #2 with that funny loop cleaned up a bit?
>
>   

New patch with the layout you suggested. It may be fewer lines, but I
still say my version was more readable.

Rgds

-- 
     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  PulseAudio, core developer          http://pulseaudio.org
  rdesktop, core developer          http://www.rdesktop.org


[-- Attachment #2: initwait.patch --]
[-- Type: text/x-patch, Size: 2047 bytes --]

commit 6354cb955ce1602b7b259f525edac7c8f585574b
Author: Pierre Ossman <drzeus@drzeus.cx>
Date:   Mon May 14 14:15:46 2007 +0200

    init: wait for asynchronously scanned block devices
    
    Some buses (e.g. USB and MMC) do their scanning of devices in the
    background, causing a race between them and prepare_namespace().
    In order to be able to use these buses without an initrd, we now
    wait for the device specified in root= to actually show up.
    
    If the device never shows up than we will hang in an infinite loop.
    In order to not mess with setups that reboot on panic, the feature
    must be turned on via the command line option "rootwait".
    
    Signed-off-by: Pierre Ossman <drzeus@drzeus.cx>

diff --git a/init/do_mounts.c b/init/do_mounts.c
index 46fe407..4e5ef6a 100644
--- a/init/do_mounts.c
+++ b/init/do_mounts.c
@@ -25,6 +25,7 @@ int __initdata rd_doload;	/* 1 = load RAM disk, 0 = don't load */
 int root_mountflags = MS_RDONLY | MS_SILENT;
 char * __initdata root_device_name;
 static char __initdata saved_root_name[64];
+int __initdata root_wait;
 
 dev_t ROOT_DEV;
 
@@ -216,6 +217,14 @@ static int __init root_dev_setup(char *line)
 
 __setup("root=", root_dev_setup);
 
+static int __init rootwait_setup(char *line)
+{
+	root_wait = simple_strtol(line,NULL,0);
+	return 1;
+}
+
+__setup("rootwait=", rootwait_setup);
+
 static char * __initdata root_mount_data;
 static int __init root_data_setup(char *str)
 {
@@ -438,11 +447,20 @@ void __init prepare_namespace(void)
 			root_device_name += 5;
 	}
 
-	is_floppy = MAJOR(ROOT_DEV) == FLOPPY_MAJOR;
-
 	if (initrd_load())
 		goto out;
 
+	/* wait for any asynchronous scanning to complete */
+	if ((ROOT_DEV == 0) && root_wait) {
+		printk(KERN_INFO "Waiting for root device %s...\n",
+			saved_root_name);
+		while (driver_probe_done() != 0 ||
+			(ROOT_DEV = name_to_dev_t(saved_root_name)) == 0)
+			msleep(100);
+	}
+
+	is_floppy = MAJOR(ROOT_DEV) == FLOPPY_MAJOR;
+
 	if (is_floppy && rd_doload && rd_load_disk(0))
 		ROOT_DEV = Root_RAM0;
 

Re: [PATCH] Make prepare_namespace() wait for devices

[Andrew Morton]

On Fri, 01 Jun 2007 07:24:36 +0200 Pierre Ossman <drzeus-list@drzeus.cx> wrote:

> +static int __init rootwait_setup(char *line)
> +{
> +	root_wait = simple_strtol(line,NULL,0);
> +	return 1;
> +}
> +
> +__setup("rootwait=", rootwait_setup);

Could we have an update for Documentation/kernel-parameters.txt, please?

Re: [PATCH] Make prepare_namespace() wait for devices

[Pierre Ossman]

[-- Attachment #1: Type: text/plain, Size: 427 bytes --]

Andrew Morton wrote:
> Could we have an update for Documentation/kernel-parameters.txt, please?
>   

Sorry, of course.

New patch included. "rootwait" is also just a boolean, so make sure it
is treated as such.

Rgds

-- 
     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  PulseAudio, core developer          http://pulseaudio.org
  rdesktop, core developer          http://www.rdesktop.org


[-- Attachment #2: initwait.patch --]
[-- Type: text/x-patch, Size: 2625 bytes --]

commit 1e5e9315f431f54d2a2ee90ed11279271a346764
Author: Pierre Ossman <drzeus@drzeus.cx>
Date:   Mon May 14 14:15:46 2007 +0200

    init: wait for asynchronously scanned block devices
    
    Some buses (e.g. USB and MMC) do their scanning of devices in the
    background, causing a race between them and prepare_namespace().
    In order to be able to use these buses without an initrd, we now
    wait for the device specified in root= to actually show up.
    
    If the device never shows up than we will hang in an infinite loop.
    In order to not mess with setups that reboot on panic, the feature
    must be turned on via the command line option "rootwait".
    
    Signed-off-by: Pierre Ossman <drzeus@drzeus.cx>

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index aae2282..685b748 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1538,6 +1538,10 @@ and is between 256 and 4096 characters. It is defined in the file
 
 	rootfstype=	[KNL] Set root filesystem type
 
+	rootwait	[KNL] Wait (indefinitely) for root device to show up.
+			Useful for devices that are detected asynchronously
+			(e.g. USB and MMC devices).
+
 	rw		[KNL] Mount root device read-write on boot
 
 	S		[KNL] Run init in single mode
diff --git a/init/do_mounts.c b/init/do_mounts.c
index 46fe407..187dcb5 100644
--- a/init/do_mounts.c
+++ b/init/do_mounts.c
@@ -25,6 +25,7 @@ int __initdata rd_doload;	/* 1 = load RAM disk, 0 = don't load */
 int root_mountflags = MS_RDONLY | MS_SILENT;
 char * __initdata root_device_name;
 static char __initdata saved_root_name[64];
+int __initdata root_wait;
 
 dev_t ROOT_DEV;
 
@@ -216,6 +217,16 @@ static int __init root_dev_setup(char *line)
 
 __setup("root=", root_dev_setup);
 
+static int __init rootwait_setup(char *str)
+{
+	if (*str)
+		return 0;
+	root_wait = 1;
+	return 1;
+}
+
+__setup("rootwait", rootwait_setup);
+
 static char * __initdata root_mount_data;
 static int __init root_data_setup(char *str)
 {
@@ -438,11 +449,20 @@ void __init prepare_namespace(void)
 			root_device_name += 5;
 	}
 
-	is_floppy = MAJOR(ROOT_DEV) == FLOPPY_MAJOR;
-
 	if (initrd_load())
 		goto out;
 
+	/* wait for any asynchronous scanning to complete */
+	if ((ROOT_DEV == 0) && root_wait) {
+		printk(KERN_INFO "Waiting for root device %s...\n",
+			saved_root_name);
+		while (driver_probe_done() != 0 ||
+			(ROOT_DEV = name_to_dev_t(saved_root_name)) == 0)
+			msleep(100);
+	}
+
+	is_floppy = MAJOR(ROOT_DEV) == FLOPPY_MAJOR;
+
 	if (is_floppy && rd_doload && rd_load_disk(0))
 		ROOT_DEV = Root_RAM0;