PATCH: md.c - devfs naming fix.
Changes:
Cleaned a few printk's
Removed a meaningless ifndef.
Moved md= name_to_ kdev_t() processing from md_setup() to
md_setup_drive. Rewrote it and added devfs_find_handle() call
to support devfs names for md=.
The devfs_find_handle() code is now redundant in my patch and
fs/super.c::mount_root(). It probably should be moved directly into
name_to_kdev_t(), no? If this was done the md= code would have worked as is,
except for the devfs code choking on the trailing ',' in the device_names
list. (Richard, want to check for this in the future?)
This diff is against md.c in 2.4.4.
Comments/testing please.
Thanks to Neil Brown for the recommendation...
--- md.c.orig Sun Jun 3 13:58:35 2001
+++ md.cSun Jun 3 22:14:52 2001
@@ -3520,7 +3520,7 @@
max_readahead[MAJOR_NR] = md_maxreadahead;
hardsect_size[MAJOR_NR] = md_hardsect_sizes;
- printk("md.c: sizeof(mdp_super_t) = %d\n", (int)sizeof(mdp_super_t));
+ dprintk("md: sizeof(mdp_super_t) = %d\n", (int)sizeof(mdp_super_t));
#ifdef CONFIG_PROC_FS
create_proc_read_entry("mdstat", 0, NULL, md_status_read_proc, NULL);
@@ -3532,7 +3532,7 @@
static char * name = "mdrecoveryd";
int minor;
- printk (KERN_INFO "md driver %d.%d.%d MAX_MD_DEVS=%d, MD_SB_DISKS=%d\n",
+ printk (KERN_INFO "md: md driver %d.%d.%d MAX_MD_DEVS=%d, MD_SB_DISKS=%d\n",
MD_MAJOR_VERSION, MD_MINOR_VERSION,
MD_PATCHLEVEL_VERSION, MAX_MD_DEVS, MD_SB_DISKS);
@@ -3607,7 +3607,7 @@
mdk_rdev_t *rdev;
int i;
- printk(KERN_INFO "autodetecting RAID arrays\n");
+ printk(KERN_INFO "md: Autodetecting RAID arrays.\n");
for (i = 0; i < dev_cnt; i++) {
kdev_t dev = detected_devices[i];
@@ -3641,6 +3641,7 @@
int pers[MAX_MD_DEVS];
int chunk[MAX_MD_DEVS];
kdev_t devices[MAX_MD_DEVS][MD_SB_DISKS];
+ char *device_names[MAX_MD_DEVS];
} md_setup_args md__initdata;
/*
@@ -3659,25 +3660,24 @@
* md=n,device-list reads a RAID superblock from the devices
* elements in device-list are read by name_to_kdev_t so can be
* a hex number or something like /dev/hda1 /dev/sdb
+ * 2001-06-03: Dave Cinege <[EMAIL PROTECTED]>
+ * Shifted name_to_kdev_t() and related operations to md_set_drive()
+ * for later execution. Rewrote section to make devfs compatible.
*/
-#ifndef MODULE
-extern kdev_t name_to_kdev_t(char *line) md__init;
static int md__init md_setup(char *str)
{
int minor, level, factor, fault, i=0;
- kdev_t device;
- char *devnames, *pername = "";
+ char *pername = "";
if (get_option(, ) != 2) {/* MD Number */
printk("md: Too few arguments supplied to md=.\n");
return 0;
}
if (minor >= MAX_MD_DEVS) {
- printk ("md: Minor device number too high.\n");
+ printk ("md: md=%d, Minor device number too high.\n", minor);
return 0;
- } else if (md_setup_args.device_set[minor]) {
- printk ("md: Warning - md=%d,... has been specified twice;\n"
- "will discard the first definition.\n", minor);
+ } else if (md_setup_args.device_names[minor]) {
+ printk ("md: md=%d, Specified more then once. Replacing previous
+definition.\n", minor);
}
switch (get_option(, )) { /* RAID Personality */
case 2: /* could be 0 or -1.. */
@@ -3714,30 +3714,13 @@
md_setup_args.pers[minor] = 0;
pername="super-block";
}
- devnames = str;
- for (; i
PATCH: md.c - devfs naming fix.
Changes:
Cleaned a few printk's
Removed a meaningless ifndef.
Moved md= name_to_ kdev_t() processing from md_setup() to
md_setup_drive. Rewrote it and added devfs_find_handle() call
to support devfs names for md=.
The devfs_find_handle() code is now redundant in my patch and
fs/super.c::mount_root(). It probably should be moved directly into
name_to_kdev_t(), no? If this was done the md= code would have worked as is,
except for the devfs code choking on the trailing ',' in the device_names
list. (Richard, want to check for this in the future?)
This diff is against md.c in 2.4.4.
Comments/testing please.
Thanks to Neil Brown for the recommendation...
--- md.c.orig Sun Jun 3 13:58:35 2001
+++ md.cSun Jun 3 22:14:52 2001
@@ -3520,7 +3520,7 @@
max_readahead[MAJOR_NR] = md_maxreadahead;
hardsect_size[MAJOR_NR] = md_hardsect_sizes;
- printk(md.c: sizeof(mdp_super_t) = %d\n, (int)sizeof(mdp_super_t));
+ dprintk(md: sizeof(mdp_super_t) = %d\n, (int)sizeof(mdp_super_t));
#ifdef CONFIG_PROC_FS
create_proc_read_entry(mdstat, 0, NULL, md_status_read_proc, NULL);
@@ -3532,7 +3532,7 @@
static char * name = mdrecoveryd;
int minor;
- printk (KERN_INFO md driver %d.%d.%d MAX_MD_DEVS=%d, MD_SB_DISKS=%d\n,
+ printk (KERN_INFO md: md driver %d.%d.%d MAX_MD_DEVS=%d, MD_SB_DISKS=%d\n,
MD_MAJOR_VERSION, MD_MINOR_VERSION,
MD_PATCHLEVEL_VERSION, MAX_MD_DEVS, MD_SB_DISKS);
@@ -3607,7 +3607,7 @@
mdk_rdev_t *rdev;
int i;
- printk(KERN_INFO autodetecting RAID arrays\n);
+ printk(KERN_INFO md: Autodetecting RAID arrays.\n);
for (i = 0; i dev_cnt; i++) {
kdev_t dev = detected_devices[i];
@@ -3641,6 +3641,7 @@
int pers[MAX_MD_DEVS];
int chunk[MAX_MD_DEVS];
kdev_t devices[MAX_MD_DEVS][MD_SB_DISKS];
+ char *device_names[MAX_MD_DEVS];
} md_setup_args md__initdata;
/*
@@ -3659,25 +3660,24 @@
* md=n,device-list reads a RAID superblock from the devices
* elements in device-list are read by name_to_kdev_t so can be
* a hex number or something like /dev/hda1 /dev/sdb
+ * 2001-06-03: Dave Cinege [EMAIL PROTECTED]
+ * Shifted name_to_kdev_t() and related operations to md_set_drive()
+ * for later execution. Rewrote section to make devfs compatible.
*/
-#ifndef MODULE
-extern kdev_t name_to_kdev_t(char *line) md__init;
static int md__init md_setup(char *str)
{
int minor, level, factor, fault, i=0;
- kdev_t device;
- char *devnames, *pername = ;
+ char *pername = ;
if (get_option(str, minor) != 2) {/* MD Number */
printk(md: Too few arguments supplied to md=.\n);
return 0;
}
if (minor = MAX_MD_DEVS) {
- printk (md: Minor device number too high.\n);
+ printk (md: md=%d, Minor device number too high.\n, minor);
return 0;
- } else if (md_setup_args.device_set[minor]) {
- printk (md: Warning - md=%d,... has been specified twice;\n
- will discard the first definition.\n, minor);
+ } else if (md_setup_args.device_names[minor]) {
+ printk (md: md=%d, Specified more then once. Replacing previous
+definition.\n, minor);
}
switch (get_option(str, level)) { /* RAID Personality */
case 2: /* could be 0 or -1.. */
@@ -3714,30 +3714,13 @@
md_setup_args.pers[minor] = 0;
pername=super-block;
}
- devnames = str;
- for (; iMD_SB_DISKS str; i++) {
- if ((device = name_to_kdev_t(str))) {
- md_setup_args.devices[minor][i] = device;
- } else {
- printk (md: Unknown device name, %s.\n, str);
- return 0;
- }
- if ((str = strchr(str, ',')) != NULL)
- str++;
- }
- if (!i) {
- printk (md: No devices specified for md%d?\n, minor);
- return 0;
- }
-
- printk (md: Will configure md%d (%s) from %s, below.\n,
- minor, pername, devnames);
- md_setup_args.devices[minor][i] = (kdev_t) 0;
- md_setup_args.device_set[minor] = 1;
+
+ md_setup_args.device_names[minor] = str;
+
return 1;
}
-#endif /* !MODULE */
+extern kdev_t name_to_kdev_t(char *line) md__init;
void md__init md_setup_drive(void)
{
int minor, i;
@@ -3745,16 +3728,50 @@
mddev_t*mddev;
for (minor = 0; minor MAX_MD_DEVS; minor++) {
+ int err = 0;
+ char *devname;
mdu_disk_info_t dinfo;
- int err = 0;
- if (!md_setup_args.device_set[minor
[PATCH] Remove arbitrary md= boot device limit
linux-2.4.1p11-1/drivers/md/md.c
line 3643
-#define MAX_MD_BOOT_DEVS 8
+#define MAX_MD_BOOT_DEVS MAX_MD_DEVS
---
To: Dave Cinege <[EMAIL PROTECTED]>
On Mon, 29 Jan 2001, Dave Cinege wrote:
> -#define MAX_MD_BOOT_DEVS 8
> +#define MAX_MD_BOOT_DEVS MAX_MD_DEVS
sure this is fine.
Ingo
---
To: Ingo Molnar <[EMAIL PROTECTED]>
Devices above md8 will not be initialized when speced with md=.
Error ("md: Minor device number too high.\n");
The limitation is imposed by
#define MAX_MD_BOOT_DEVS8
However it appears arbitray to me. Doesn't make much sence since you can create
/dev/md100 and it may well be the only md device you have...
Is there any reason the next 2.4.1 prepatch should not include this?
-#define MAX_MD_BOOT_DEVS 8
+#define MAX_MD_BOOT_DEVS MAX_MD_DEVS
(If not I assume you will be submitting this to Linus...)
--
"Nobody will ever be safe until the last cop is dead."
NH Rep. Tom Alciere - (My new Hero)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Remove arbitrary md= boot device limit
linux-2.4.1p11-1/drivers/md/md.c
line 3643
-#define MAX_MD_BOOT_DEVS 8
+#define MAX_MD_BOOT_DEVS MAX_MD_DEVS
---
To: Dave Cinege [EMAIL PROTECTED]
On Mon, 29 Jan 2001, Dave Cinege wrote:
-#define MAX_MD_BOOT_DEVS 8
+#define MAX_MD_BOOT_DEVS MAX_MD_DEVS
sure this is fine.
Ingo
---
To: Ingo Molnar [EMAIL PROTECTED]
Devices above md8 will not be initialized when speced with md=.
Error ("md: Minor device number too high.\n");
The limitation is imposed by
#define MAX_MD_BOOT_DEVS8
However it appears arbitray to me. Doesn't make much sence since you can create
/dev/md100 and it may well be the only md device you have...
Is there any reason the next 2.4.1 prepatch should not include this?
-#define MAX_MD_BOOT_DEVS 8
+#define MAX_MD_BOOT_DEVS MAX_MD_DEVS
(If not I assume you will be submitting this to Linus...)
--
"Nobody will ever be safe until the last cop is dead."
NH Rep. Tom Alciere - (My new Hero)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/
Re: md= broken. Found problem. Can't fix it. : (
Douglas Gilbert wrote: > > Dave, > Look at the dmesg output and check that your > "Kernel command line:" is what you think it > is. Some older versions of lilo truncate it. > Here is mine (which is what I expected): > > Kernel command line: auto BOOT_IMAGE=lin240 ro root=803 scsihosts=imm:advansys:a > dvansys:aha1542 No that is not the problem. I'm using GRUB (LILO == poopoo) and have looked at this throughly. I can see from the dmesg via my debugging printk's output that str is properly being passed. -- "Nobody will ever be safe until the last cop is dead." NH Rep. Tom Alciere - (My new Hero) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: md= broken. Found problem. Can't fix it. : (
Sandy Harris wrote:
> Looks to me like this parsing code unnecessarily and rather clumsily
> re-invents strtok
The original parsing code is this:
if ((str = strchr(str, ',')) != NULL)
str++;
Which effectivly steps through
/dev/sda1,/dev/sdab1,/dev/sdc1
like this
str == /dev/sda1,/dev/sdab1,/dev/sdc1
str == /dev/sdab1,/dev/sdc1
str == /dev/sdc1
My code:char *ndevstr;
ndevstr = strchr(str, ',');
if (ndevstr != NULL)*ndevstr++ = 0;
...
str = ndevstr
Works perfectly. I don't find it 'clumsy' or more complex at all. (I don't care
for strtok, nor did I even know the kernel had it)
However I don't see this critique of coding style helping the problem I'm
seeing:
name_to_kdev_t(str);
Returns a bad value. Yet
name_to_kdev_t("/dev/sdd5");
does not. The strange thing is
printk("Checking: '%s'\n", str);
shows str does infact contain a proper string.
It appears str does not get passed to name_to_kdev_t() properly,
and I have no idea why. Both my testing code and the original code
exhibit the same problem.
--
"Nobody will ever be safe until the last cop is dead."
NH Rep. Tom Alciere - (My new Hero)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/
md= broken. Found problem. Can't fix it. : (
I have multiple Linux hosts on a SAN, making autodetect of raid devices
dangerous. This problem should be solved by specing an 'md=' for each device on
the cmdline, but unfortuantly it's broken.
Between a few emails to mingo and several wasted hours, I've managed to figure
out the problem. However I don't know how to fix it; it *should*
be working from what I can see.
My only guess right now is because I'm using gcc 2.95.2, and it's doing
something funky. (And I do not have another version to test with right now.)
The problem is during parsing of the md= line, name_to_kdev_t() is not
returning the proper k_dev_t for the device. (IE /dev/sdd5 returns as
16:45 /dev/sde5 returns as 20:05.)
However if I pass static text to name_to_kdev_t(), it works. I first believed
it was in how the str pointer was sent to name_to_kdev_t(), (running over into
comma's, instead of seperate terminated strings) I
fixed that but the problem persists.
My current test for loop is attached. The hard coded device names
printk out to a proper major:minor. The devicenames obtained from
'str' don't. I don't see the bug in here or name_to_kdev_t()...
I'm testing this with the following cmdline:
root=/dev/md0 raid=noautodetect md=0,/dev/hdd5,/dev/hde5
md=1,/dev/hdd6,/dev/hde6 md=2,/dev/hdd7,/dev/hde7
md=3,/dev/hdd8,/dev/hde8 mem=524288K
--
"Nobody will ever be safe until the last cop is dead."
NH Rep. Tom Alciere - (My new Hero)
2.4.1prepatch 8
drivers/md/md.c line 3722
for (; i < MD_SB_DISKS && str; i++) {
/*
if ((device = name_to_kdev_t(str))) {
md_setup_args.devices[minor][i] = device;
} else {
printk ("md: Unknown device name, %s.\n", str);
return 0;
}
if ((str = strchr(str, ',')) != NULL)
str++;
*/
char *ndevstr;
ndevstr = strchr(str, ','); // Goto ','
if (ndevstr != NULL)
*ndevstr++ = 0; // Zero it for proper string
// DEBUG Print device name
printk("Checking: '%s'\n", str);
// Convert device name to k_dev_t and assign to md_setup_args.devices
// DEBUG As test, hardcode device names for /dev/md0.0 and /dev/md0.1
if (minor == 0 && i == 0)
md_setup_args.devices[minor][i] = name_to_kdev_t("/dev/sdd5");
else if (minor == 0 && i == 1)
md_setup_args.devices[minor][i] = name_to_kdev_t("/dev/sde5");
else
md_setup_args.devices[minor][i] = name_to_kdev_t(str);
// DEBUG Print out kdevname of md_setup_args.devices
printk("\t%s\n", kdevname(md_setup_args.devices[minor][i]));
// DEBUG Print minor and i (insync?)
printk("minor=%d, i=%d\n",minor, i);
// name_to_kdev_t() returned 0. Invalid device
if (md_setup_args.devices[minor][i] == 0) {
printk ("md: Unknown device name, %s.\n", str);
return 0;
}
// Jump to next devname in str
str = ndevstr;
}
md= broken. Found problem. Can't fix it. : (
I have multiple Linux hosts on a SAN, making autodetect of raid devices
dangerous. This problem should be solved by specing an 'md=' for each device on
the cmdline, but unfortuantly it's broken.
Between a few emails to mingo and several wasted hours, I've managed to figure
out the problem. However I don't know how to fix it; it *should*
be working from what I can see.
My only guess right now is because I'm using gcc 2.95.2, and it's doing
something funky. (And I do not have another version to test with right now.)
The problem is during parsing of the md= line, name_to_kdev_t() is not
returning the proper k_dev_t for the device. (IE /dev/sdd5 returns as
16:45 /dev/sde5 returns as 20:05.)
However if I pass static text to name_to_kdev_t(), it works. I first believed
it was in how the str pointer was sent to name_to_kdev_t(), (running over into
comma's, instead of seperate terminated strings) I
fixed that but the problem persists.
My current test for loop is attached. The hard coded device names
printk out to a proper major:minor. The devicenames obtained from
'str' don't. I don't see the bug in here or name_to_kdev_t()...
I'm testing this with the following cmdline:
root=/dev/md0 raid=noautodetect md=0,/dev/hdd5,/dev/hde5
md=1,/dev/hdd6,/dev/hde6 md=2,/dev/hdd7,/dev/hde7
md=3,/dev/hdd8,/dev/hde8 mem=524288K
--
"Nobody will ever be safe until the last cop is dead."
NH Rep. Tom Alciere - (My new Hero)
2.4.1prepatch 8
drivers/md/md.c line 3722
for (; i MD_SB_DISKS str; i++) {
/*
if ((device = name_to_kdev_t(str))) {
md_setup_args.devices[minor][i] = device;
} else {
printk ("md: Unknown device name, %s.\n", str);
return 0;
}
if ((str = strchr(str, ',')) != NULL)
str++;
*/
char *ndevstr;
ndevstr = strchr(str, ','); // Goto ','
if (ndevstr != NULL)
*ndevstr++ = 0; // Zero it for proper string
// DEBUG Print device name
printk("Checking: '%s'\n", str);
// Convert device name to k_dev_t and assign to md_setup_args.devices
// DEBUG As test, hardcode device names for /dev/md0.0 and /dev/md0.1
if (minor == 0 i == 0)
md_setup_args.devices[minor][i] = name_to_kdev_t("/dev/sdd5");
else if (minor == 0 i == 1)
md_setup_args.devices[minor][i] = name_to_kdev_t("/dev/sde5");
else
md_setup_args.devices[minor][i] = name_to_kdev_t(str);
// DEBUG Print out kdevname of md_setup_args.devices
printk("\t%s\n", kdevname(md_setup_args.devices[minor][i]));
// DEBUG Print minor and i (insync?)
printk("minor=%d, i=%d\n",minor, i);
// name_to_kdev_t() returned 0. Invalid device
if (md_setup_args.devices[minor][i] == 0) {
printk ("md: Unknown device name, %s.\n", str);
return 0;
}
// Jump to next devname in str
str = ndevstr;
}
Re: md= broken. Found problem. Can't fix it. : (
Sandy Harris wrote:
Looks to me like this parsing code unnecessarily and rather clumsily
re-invents strtok
The original parsing code is this:
if ((str = strchr(str, ',')) != NULL)
str++;
Which effectivly steps through
/dev/sda1,/dev/sdab1,/dev/sdc1
like this
str == /dev/sda1,/dev/sdab1,/dev/sdc1
str == /dev/sdab1,/dev/sdc1
str == /dev/sdc1
My code:char *ndevstr;
ndevstr = strchr(str, ',');
if (ndevstr != NULL)*ndevstr++ = 0;
...
str = ndevstr
Works perfectly. I don't find it 'clumsy' or more complex at all. (I don't care
for strtok, nor did I even know the kernel had it)
However I don't see this critique of coding style helping the problem I'm
seeing:
name_to_kdev_t(str);
Returns a bad value. Yet
name_to_kdev_t("/dev/sdd5");
does not. The strange thing is
printk("Checking: '%s'\n", str);
shows str does infact contain a proper string.
It appears str does not get passed to name_to_kdev_t() properly,
and I have no idea why. Both my testing code and the original code
exhibit the same problem.
--
"Nobody will ever be safe until the last cop is dead."
NH Rep. Tom Alciere - (My new Hero)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/
Re: md= broken. Found problem. Can't fix it. : (
Douglas Gilbert wrote: Dave, Look at the dmesg output and check that your "Kernel command line:" is what you think it is. Some older versions of lilo truncate it. Here is mine (which is what I expected): Kernel command line: auto BOOT_IMAGE=lin240 ro root=803 scsihosts=imm:advansys:a dvansys:aha1542 No that is not the problem. I'm using GRUB (LILO == poopoo) and have looked at this throughly. I can see from the dmesg via my debugging printk's output that str is properly being passed. -- "Nobody will ever be safe until the last cop is dead." NH Rep. Tom Alciere - (My new Hero) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
