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/