Make prepare_namespace() wait for devices

PATCH In review Pierre Ossman 3 participants 12 messages May 24, 2007 – Jun 1, 2007

Subsystems (2)

Documentation VFS

lore

Share & tools

lore

Pierre Ossman

author

2007-05-24 12:21 UTC #0 lore raw

PATCH 1/1 Make prepare_namespace() wait for devices

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 <[email protected]>

Signature · 5 lines

     -- Pierre Ossman

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

commit bb8c44ee8b4d584295add58a4ea2f03b9938fc3c

Author: Pierre Ossman <[email protected]>
Date:   Mon May 14 14:15:46 2007 +0200

init: wait for asynchronously scanned block devices

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 <[email protected]>

init/do_mounts.c +24 -2

index 46fe407..efc134c 100644
@@ -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;

Olaf Hering

2007-05-24 18:52 UTC #1 lore raw

Quoted 1 line · Pierre Ossman

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?

Pierre Ossman

author

2007-05-24 20:46 UTC #2 lore raw

Quoted 10 lines · Olaf Hering

Quoted 2 lines · Pierre Ossman
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

Signature · 5 lines

     -- Pierre Ossman

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

Andrew Morton

2007-05-25 00:07 UTC #3 lore raw

In reply to Pierre Ossman

init/do_mounts.c void __init prepare_namespace(void) · line 464

Quoted 12 lines · On Thu, 24 May 2007 14:21:35 +0200 Pierre Ossman <[email protected]> wrote:

init/do_mounts.c +12 -0

+	/* 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);

Pierre Ossman

author

2007-05-25 04:03 UTC #4 lore raw

Quoted 21 lines · Andrew Morton

init/do_mounts.c void __init prepare_namespace(void) · line 464
Quoted 13 lines · On Thu, 24 May 2007 14:21:35 +0200 Pierre Ossman <[email protected]> wrote:
init/do_mounts.c +12 -0
+	/* 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

Quoted 5 lines

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

How would ROOT_DEV get updated in that loop?

Signature · 5 lines

     -- Pierre Ossman

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

Andrew Morton

2007-05-25 04:17 UTC #5 lore raw

Quoted 22 lines · On Fri, 25 May 2007 06:03:54 +0200 Pierre Ossman <[email protected]> wrote:

Quoted 21 lines · Andrew Morton
init/do_mounts.c void __init prepare_namespace(void) · line 464
Quoted 13 lines · On Thu, 24 May 2007 14:21:35 +0200 Pierre Ossman <[email protected]> wrote:
init/do_mounts.c +12 -0
+	/* 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

Quoted 8 lines

Quoted 5 lines
	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?

Pierre Ossman

author

2007-05-25 04:31 UTC #6 lore raw

Quoted 3 lines · Andrew Morton

Whatever. I think you can work it out ;)

Bare with me, I just woke up ;)

Quoted 6 lines

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

Signature · 5 lines

     -- Pierre Ossman

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

Pierre Ossman

author

2007-05-31 11:20 UTC #7 lore raw

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

Quoted 14 lines · Pierre Ossman

Quoted 3 lines · Andrew Morton
Whatever. I think you can work it out ;)
Bare with me, I just woke up ;)
Quoted 6 lines
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?

Quoted 7 lines

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

Signature · 5 lines

     -- Pierre Ossman

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

Andrew Morton

2007-05-31 15:51 UTC #8 lore raw

Quoted 1 line · On Thu, 31 May 2007 13:20:48 +0200 Pierre Ossman <[email protected]> wrote:

(top-posting reversed)

Quoted 18 lines

Quoted 17 lines · Pierre Ossman
Quoted 3 lines · Andrew Morton
Whatever. I think you can work it out ;)
Bare with me, I just woke up ;)
Quoted 6 lines
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.

Quoted 6 lines

Quoted 4 lines
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?

Pierre Ossman

author

2007-06-01 05:24 UTC #9 lore raw

Quoted 9 lines · Andrew Morton

Quoted 2 lines · On Thu, 31 May 2007 13:20:48 +0200 Pierre Ossman <[email protected]> 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

Signature · 5 lines

     -- Pierre Ossman

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

commit 6354cb955ce1602b7b259f525edac7c8f585574b

Author: Pierre Ossman <[email protected]>
Date:   Mon May 14 14:15:46 2007 +0200

init: wait for asynchronously scanned block devices

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 <[email protected]>

init/do_mounts.c +20 -2

index 46fe407..4e5ef6a 100644
@@ -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;

Andrew Morton

2007-06-01 06:40 UTC #10 lore raw

init/do_mounts.c static int __init root_dev_setup(char *line) · line 226

Quoted 7 lines · On Fri, 01 Jun 2007 07:24:36 +0200 Pierre Ossman <[email protected]> wrote:

init/do_mounts.c +7 -0

+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?

Pierre Ossman

author

2007-06-01 06:50 UTC #11 lore raw

Quoted 2 lines · Andrew Morton

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

Signature · 5 lines

     -- Pierre Ossman

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

commit 1e5e9315f431f54d2a2ee90ed11279271a346764

Author: Pierre Ossman <[email protected]>
Date:   Mon May 14 14:15:46 2007 +0200

init: wait for asynchronously scanned block devices

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 <[email protected]>

Documentation/kernel-parameters.txt +4 -0

index aae2282..685b748 100644
@@ -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

init/do_mounts.c +22 -2

index 46fe407..187dcb5 100644
@@ -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;