2000-12-01 17:20:43

by T. Camp

[permalink] [raw]
Subject: [PATCH] mutliple root devs (take II)

A much cleaner patch prompted after right proper chastisement on the
sloppy patch I sent a few days back. This one is against 2.4-pre11 but so
far as I can tell should be good to go against any of the 2.4 series so
far.

I have not implemented a regex-like syntax as was suggested because 1) you
can still do the same stuff just takes more typing 2) would make for a
fairly large re-ordering of code afterall.

basic idea here is that the kernel will search through a list of root
devices passed to the kernel as arguements before giving up on the rootfs.

ie.

(from lilo:)

linux root=/dev/hda1,/dev/hdc,/dev/hda7

will try each device in order. You can also split these up into separate
root= statements just fine.

please be sure to directly address me with any feedback as I'm not on the
list. (I miss linux-kernel-digest).

t. camp


--snip--
diff --recursive --unified --ignore-all-space linux/Documentation/kernel-parameters.txt linux-2.4.0.11/Documentation/kernel-parameters.txt
--- linux/Documentation/kernel-parameters.txt Tue Sep 5 13:51:14 2000
+++ linux-2.4.0.11/Documentation/kernel-parameters.txt Mon Nov 27 17:25:04 2000
@@ -473,7 +473,11 @@

ro [KNL] Mount root device read-only on boot.

- root= [KNL] root filesystem.
+ root= [KNL] root filesystem, comma separated list of up
+ to eight /dev/ entries will be tried in order
+ presented until kernel suceeds in mounting a
+ filesystem, multiple independant 'root=' entries
+ allowed as well.

rw [KNL] Mount root device read-write on boot.

diff --recursive --unified --ignore-all-space linux/fs/super.c linux-2.4.0.11/fs/super.c
--- linux/fs/super.c Mon Oct 16 12:57:59 2000
+++ linux-2.4.0.11/fs/super.c Wed Nov 29 16:56:00 2000
@@ -18,6 +18,8 @@
* Torbj?rn Lindh ([email protected]), April 14, 1996.
* Added devfs support: Richard Gooch <[email protected]>, 13-JAN-1998
* Heavily rewritten for 'one fs - one tree' dcache architecture. AV, Mar 2000
+ * Modifications to mount_root for trying a list of root devices.
+ * Tracy Camp <[email protected]> 11/22/00
*/

#include <linux/config.h>
@@ -1470,6 +1472,7 @@
void *handle;
char path[64];
int path_start = -1;
+ int root_device_index = 0;

#ifdef CONFIG_ROOT_NFS
void *data;
@@ -1528,8 +1531,15 @@
}
#endif

- devfs_make_root (root_device_name);
- handle = devfs_find_handle (NULL, ROOT_DEVICE_NAME,
+ for(root_device_index = 0; root_device_index < number_root_devs; root_device_index++) {
+ printk("VFS: trying root on %s\n",&root_device_name[root_device_index][0]);
+ if(root_device_name[root_device_index][0] != '\0')
+ {
+ ROOT_DEV = name_to_kdev_t(&root_device_name[root_device_index][0]); /* translate */
+ }
+ devfs_make_root (&root_device_name[root_device_index][0]);
+ handle = devfs_find_handle (NULL,
+ &root_device_name[root_device_index][0],
MAJOR (ROOT_DEV), MINOR (ROOT_DEV),
DEVFS_SPECIAL_BLK, 1);
if (handle) /* Sigh: bd*() functions only paper over the cracks */
@@ -1566,10 +1576,10 @@
* and bad superblock on root device.
*/
printk ("VFS: Cannot open root device \"%s\" or %s\n",
- root_device_name, kdevname (ROOT_DEV));
- printk ("Please append a correct \"root=\" boot option\n");
- panic("VFS: Unable to mount root fs on %s",
+ &root_device_name[root_device_index][0], kdevname (ROOT_DEV));
+ printk("VFS: Unable to mount root fs on %s\n",
kdevname(ROOT_DEV));
+ continue;
}

check_disk_change(ROOT_DEV);
@@ -1593,7 +1603,10 @@
put_filesystem(fs_type);
}
read_unlock(&file_systems_lock);
- panic("VFS: Unable to mount root fs on %s", kdevname(ROOT_DEV));
+ printk("VFS: Unable to mount root fs on %s", kdevname(ROOT_DEV));
+ } /* end for */
+ printk("VFS: Please append a corrent \"root=\" boot option\n");
+ panic("VFS: Unable to mount any root fs at all");

mount_it:
printk ("VFS: Mounted root (%s filesystem)%s.\n",
@@ -1772,6 +1785,15 @@
} else
path_release(&devfs_nd);
}
+/* this function seems to only be used when after an initrd usage,
+* so the actual value of ROOT_DEV hasn't been determined until mount_root
+* is called, because we try a list of root devices before giving up,
+* this ROOT_DEV global is somewhat misleading now, as its only valid after
+* a call to mount_root because mount_root ignores the ROOT_DEV value
+* except for nfs and ram mount attempts going in, but will realise
+* to not mount RAM again so it should be okay to leave this be.
+* - 11/22/00 <[email protected]>
+*/
ROOT_DEV = new_root_dev;
mount_root();
#if 1
diff --recursive --unified --ignore-all-space linux/include/linux/fs.h linux-2.4.0.11/include/linux/fs.h
--- linux/include/linux/fs.h Wed Nov 29 13:15:25 2000
+++ linux-2.4.0.11/include/linux/fs.h Wed Nov 29 15:38:49 2000
@@ -1229,7 +1229,9 @@
unsigned long generate_cluster(kdev_t, int b[], int);
unsigned long generate_cluster_swab32(kdev_t, int b[], int);
extern kdev_t ROOT_DEV;
-extern char root_device_name[];
+extern char root_device_name[8][64];
+extern int number_root_devs;
+extern kdev_t name_to_kdev_t(char *); /* in init/main.c */


extern void show_buffers(void);
diff --recursive --unified --ignore-all-space linux/init/main.c linux-2.4.0.11/init/main.c
--- linux/init/main.c Fri Nov 17 16:45:57 2000
+++ linux-2.4.0.11/init/main.c Wed Nov 29 15:39:27 2000
@@ -130,7 +130,8 @@

int root_mountflags = MS_RDONLY;
char *execute_command;
-char root_device_name[64];
+char root_device_name[8][64];
+int number_root_devs = 0;


static char * argv_init[MAX_INIT_ARGS+2] = { "init", NULL, };
@@ -261,10 +262,11 @@
kdev_t __init name_to_kdev_t(char *line)
{
int base = 0;
+ struct dev_name_struct *dev = root_dev_names;

if (strncmp(line,"/dev/",5) == 0) {
- struct dev_name_struct *dev = root_dev_names;
line += 5;
+ }
do {
int len = strlen(dev->name);
if (strncmp(line,dev->name,len) == 0) {
@@ -274,7 +276,6 @@
}
dev++;
} while (dev->name);
- }
return to_kdev_t(base + simple_strtoul(line,NULL,base?10:16));
}

@@ -283,15 +284,40 @@
int i;
char ch;

- ROOT_DEV = name_to_kdev_t(line);
- memset (root_device_name, 0, sizeof root_device_name);
+/* number_root_devs is initially 0 and not touched except for
+incrementing, so this function should be re-callable with the
+desired results */
if (strncmp (line, "/dev/", 5) == 0) line += 5;
- for (i = 0; i < sizeof root_device_name - 1; ++i)
- {
+ memset (root_device_name[number_root_devs], 0, sizeof root_device_name[number_root_devs]);
+ for (i = 0; i < sizeof root_device_name[number_root_devs] - 1; ++i) {
+ ch = line[i];
+ if ( ch == ',') {
+ if(number_root_devs == 0) {
+ ROOT_DEV = name_to_kdev_t(&root_device_name[number_root_devs][0]);
+ }
+ number_root_devs++;
+ if(number_root_devs >= 8) {
+ break;
+ }
+ i++;
+ line += i;
+ i = 0;
+ if (strncmp (line, "/dev/", 5) == 0) {
+ line += 5;
+ }
ch = line[i];
- if ( isspace (ch) || (ch == ',') || (ch == '\0') ) break;
- root_device_name[i] = ch;
+ memset (root_device_name[number_root_devs], 0, sizeof root_device_name[number_root_devs]);
+ }
+ if ( isspace (ch) || (ch == '\0') ) {
+ if(number_root_devs == 0) {
+ ROOT_DEV = name_to_kdev_t(&root_device_name[number_root_devs][0]);
+ }
+ number_root_devs++;
+ break;
+ }
+ root_device_name[number_root_devs][i] = ch;
}
+printk("root_dev_setup number_root_devs: %d\n",number_root_devs);
return 1;
}



2000-12-01 17:26:33

by Tigran Aivazian

[permalink] [raw]
Subject: Re: [PATCH] mutliple root devs (take II)

Hello,

On Fri, 1 Dec 2000, T. Camp wrote:

> A much cleaner patch prompted after right proper chastisement on the

indeed, much cleaner. But still not perfect.

> + int root_device_index = 0;

this initialisation is not needed. Just make it 'int root_device_index;'
The kernel will do the right thing for you on boot, trust me.

> +int number_root_devs = 0;

this is not needed either.

Regards,
Tigran

2000-12-01 17:36:24

by T. Camp

[permalink] [raw]
Subject: Re: [PATCH] mutliple root devs (take II)

> indeed, much cleaner. But still not perfect.
>
> > + int root_device_index = 0;
>
> this initialisation is not needed. Just make it 'int root_device_index;'
> The kernel will do the right thing for you on boot, trust me.
>
> > +int number_root_devs = 0;
>
> this is not needed either.
Hmm didn't know that, from the user-land portable C perspective I'm in the
habit of zero'ing everything. - thanks.

t.

Fear the future? Change the past.
This message has resulted in an increase in the entropy of the universe

2000-12-01 17:45:35

by Olivier Galibert

[permalink] [raw]
Subject: Re: [PATCH] mutliple root devs (take II)

On Fri, Dec 01, 2000 at 09:05:23AM -0800, T. Camp wrote:
> Hmm didn't know that, from the user-land portable C perspective I'm in the
> habit of zero'ing everything. - thanks.

It's a requirement of the ISO C standard that all global/static (not
local) variables are initialized to 0 is not initialized explicitely.
It tends to be true, too.

OG.

2000-12-01 17:46:15

by Tigran Aivazian

[permalink] [raw]
Subject: Re: [PATCH] mutliple root devs (take II)

On Fri, 1 Dec 2000, T. Camp wrote:
> Hmm didn't know that, from the user-land portable C perspective I'm in the
> habit of zero'ing everything. - thanks.

Yes, sorry, I should have explained a bit more, perhaps. The point is that
when you have an unitialized variable like this:

int x;

the C compiler does not reserve any space for it in the final object file
but rather puts special information in a section called BSS which tells
how much stuff is there, so an appropriate memory image is created for
that section during loading. Subsequently, the kernel initialisation code
(see arch/i386/kernel/head.S, grep for BSS) explicitly initializes the
content of that segment to 0 to comply with various ANSI C standards that
demand this (which presumably demand this for the very reason of making
such optimization possible).

So, not initializing things explicitly saves space in the final disk image
and is a common technique used (by far!) by the majority of Linux kernel
code. In the past, I remember, some arguments against it such as 'having
explicit 0 initialization better documents the code' but, of course, they
are not valid because that is what we have /* */ for -- for documentation.
Comments should not waste space or time.

Anyway, I said too much on such trivial and obvious issue :) -- just
making sure it is clear to you, that is all.

Regards,
Tigran

2000-12-01 23:06:51

by Peter Samuelson

[permalink] [raw]
Subject: Re: [PATCH] mutliple root devs (take II)


[Tracy Camp]
> A much cleaner patch prompted after right proper chastisement on the
> sloppy patch I sent a few days back. This one is against 2.4-pre11

Hmmm, I don't like your array thing (also in v.I of the patch),
limiting us to <n> possible root devices, where n==8. A better
approach might be to iterate over the root= arguments when mounting. I
know why you used the array -- easier to code.

One potential problem with the patch is that you have changed behavior
some people are relying on. If you use 'syslinux' to boot, for
example, the SYSLINUX.CFG file can define a default command line
including root=. Then you can augment that line at runtime by typing
in your own command line. Your patch makes it impossible, in this
situation, to override the default root device from the syslinux
command line. A kludge to make it work again would be to process the
root devices in reverse. That would be ugly and unintuitive, though.

Peter

2000-12-01 23:15:03

by Peter Samuelson

[permalink] [raw]
Subject: Re: [PATCH] mutliple root devs (take II)


[I wrote]
> Your patch makes it impossible, in this situation, to override the
> default root device from the syslinux command line. A kludge to make
> it work again would be to process the root devices in reverse.

Better would be to reset the list of root devices with every 'root='
statement, rather than trying all of them as you do now. I.e.

... root=/dev/hda1 ... root=/dev/hda2,/dev/hda3 ...

would *not* try hda1.

Peter

2000-12-02 00:06:02

by T. Camp

[permalink] [raw]
Subject: Re: [PATCH] mutliple root devs (take II)

> Hmmm, I don't like your array thing (also in v.I of the patch),
> limiting us to <n> possible root devices, where n==8. A better
> approach might be to iterate over the root= arguments when mounting. I
> know why you used the array -- easier to code.
I was unsure if it was okay to be using kmalloc during early stages of
init/main.c so I decided to follow the example allready set and just use a
static array - can anyone advise on being able to do this dynamically?

> One potential problem with the patch is that you have changed behavior
> some people are relying on. If you use 'syslinux' to boot, for
> example, the SYSLINUX.CFG file can define a default command line
> including root=. Then you can augment that line at runtime by typing
> in your own command line. Your patch makes it impossible, in this
> situation, to override the default root device from the syslinux
> command line. A kludge to make it work again would be to process the
> root devices in reverse. That would be ugly and unintuitive, though.
Yeah you would need to patch lilo as well to handle the new syntax amongst
other things. I use grub and have no troubles along these lines.
Refrencing the idea in the follow on message about just using the last
root= only and not allowing multiple root= would work around this. I
guess I can't think of any really good reason why having multiple root= is
a necissary feature.

t.

Fear the future? Change the past.
This message has resulted in an increase in the entropy of the universe

2000-12-02 00:19:57

by Peter Samuelson

[permalink] [raw]
Subject: Re: [PATCH] mutliple root devs (take II)


[Tracy Camp]
> I was unsure if it was okay to be using kmalloc during early stages
> of init/main.c so I decided to follow the example allready set and
> just use a static array - can anyone advise on being able to do this
> dynamically?

Have a static 'char *' somewhere. In the "root=" callback function,
just set this variable. Do not parse it until you are ready to
actually mount root, then just parse one dev at a time. No allocation
needed.

Note that this approach doesn't support the "multiple root=" feature,
which brings us to...

> I guess I can't think of any really good reason why having multiple
> root= is a necissary feature.

Agreed, and there *is* good reason not to support this, since it is
useful to be able to override a root= given in a config file.


> Yeah you would need to patch lilo as well to handle the new syntax
> amongst other things.

Hmm. LILO shouldn't care, but it does, because it has a 'root='
parameter which it handles specially, by patching the 16-bit device
number into the kernel image at runtime. Your patch should be fully
functional, though, as long as people just use 'append="root=..."'
instead of simply 'root=...'. The append= forces LILO not to treat the
root dev specially. (This tip brought to you by the devfs docs.)

Peter

2000-12-02 00:26:58

by Jeff Dike

[permalink] [raw]
Subject: Re: [PATCH] mutliple root devs (take II)

[email protected] said:
> I was unsure if it was okay to be using kmalloc during early stages of
> init/main.c so I decided to follow the example allready set and just
> use a static array - can anyone advise on being able to do this
> dynamically?

kmalloc is usable after mem_init(), I think. Before that, you can use the
boot memory allocator (see mm/bootmem.c). In the arch that I'm most familiar
with (arch/um), that is usable from the beginning of start_kernel. I don't
know about the other arches.

Jeff


2000-12-02 02:48:04

by Philipp Rumpf

[permalink] [raw]
Subject: Re: [PATCH] mutliple root devs (take II)

On Fri, Dec 01, 2000 at 08:04:52PM -0500, Jeff Dike wrote:
> boot memory allocator (see mm/bootmem.c). In the arch that I'm most familiar
> with (arch/um), that is usable from the beginning of start_kernel. I don't
> know about the other arches.

setup_arch does the necessary initialization on most architectures - you
might want to consider fixing UML to be consistent with them.