* md= broken. Found problem. Can't fix it. : (
@ 2001-01-20 21:34 Dave Cinege
2001-01-20 21:58 ` Sandy Harris
0 siblings, 1 reply; 6+ messages in thread
From: Dave Cinege @ 2001-01-20 21:34 UTC (permalink / raw)
To: linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1431 bytes --]
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)
[-- Attachment #2: md-test.c --]
[-- Type: text/x-csrc, Size: 1335 bytes --]
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;
}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: md= broken. Found problem. Can't fix it. : (
2001-01-20 21:34 md= broken. Found problem. Can't fix it. : ( Dave Cinege
@ 2001-01-20 21:58 ` Sandy Harris
2001-01-20 22:28 ` Andi Kleen
2001-01-20 22:39 ` Dave Cinege
0 siblings, 2 replies; 6+ messages in thread
From: Sandy Harris @ 2001-01-20 21:58 UTC (permalink / raw)
To: linux-kernel
Dave Cinege wrote:
>
> ... '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.)
Looks to me like this parsing code unnecessarily and rather clumsily re-invents
strtok() and should be rewritten to use that function. It takes two nested loops,
along the general lines of:
/*
outer loop, parse into space-separted strings
*/
for( p = strtok(str, " ") ; p != NULL ; p = strtok(NULL, " ") {
/*
inner loop using comma separator
*
for( q = strtok( p, ",") ; q != NULL ; q = strtok(NULL, ",") {
}
}
I suspect that I've misunderstood some constraint here. Perhaps the more complex
code you posted is necessary, but I'd like to know why.
> 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;
> }
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: md= broken. Found problem. Can't fix it. : (
2001-01-20 21:58 ` Sandy Harris
@ 2001-01-20 22:28 ` Andi Kleen
2001-01-22 14:09 ` Ingo Oeser
2001-01-20 22:39 ` Dave Cinege
1 sibling, 1 reply; 6+ messages in thread
From: Andi Kleen @ 2001-01-20 22:28 UTC (permalink / raw)
To: Sandy Harris; +Cc: linux-kernel
On Sat, Jan 20, 2001 at 04:58:56PM -0500, Sandy Harris wrote:
> I suspect that I've misunderstood some constraint here. Perhaps the more complex
> code you posted is necessary, but I'd like to know why.
strtok is not reentrant and cannot be nested this way without
saving __strtok. strsep would work.
-Andi
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: md= broken. Found problem. Can't fix it. : (
2001-01-20 21:58 ` Sandy Harris
2001-01-20 22:28 ` Andi Kleen
@ 2001-01-20 22:39 ` Dave Cinege
1 sibling, 0 replies; 6+ messages in thread
From: Dave Cinege @ 2001-01-20 22:39 UTC (permalink / raw)
To: Sandy Harris; +Cc: linux-kernel
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 majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: md= broken. Found problem. Can't fix it. : (
2001-01-20 22:28 ` Andi Kleen
@ 2001-01-22 14:09 ` Ingo Oeser
0 siblings, 0 replies; 6+ messages in thread
From: Ingo Oeser @ 2001-01-22 14:09 UTC (permalink / raw)
To: Andi Kleen; +Cc: Sandy Harris, linux-kernel
On Sat, Jan 20, 2001 at 11:28:51PM +0100, Andi Kleen wrote:
> On Sat, Jan 20, 2001 at 04:58:56PM -0500, Sandy Harris wrote:
> > I suspect that I've misunderstood some constraint here. Perhaps the more complex
> > code you posted is necessary, but I'd like to know why.
>
> strtok is not reentrant and cannot be nested this way without
> saving __strtok. strsep would work.
But be careful:
strsep() in kernel skips zero length strings, but strsep
glibc wouldn't do.
Regards
Ingo Oeser
Note: I implemented it to replace strtok and even did a patch to
replace all occourences of it, but got no response and so stopped
working on this issue. If there is still interest, I would do
it again.
--
10.+11.03.2001 - 3. Chemnitzer LinuxTag <http://www.tu-chemnitz.de/linux/tag>
<<<<<<<<<<<< come and join the fun >>>>>>>>>>>>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: md= broken. Found problem. Can't fix it. : (
[not found] <3A6A1512.C2684CCC@interlog.com>
@ 2001-01-20 23:21 ` Dave Cinege
0 siblings, 0 replies; 6+ messages in thread
From: Dave Cinege @ 2001-01-20 23:21 UTC (permalink / raw)
To: Douglas Gilbert; +Cc: linux-kernel
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 majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2001-01-22 14:09 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2001-01-20 21:34 md= broken. Found problem. Can't fix it. : ( Dave Cinege
2001-01-20 21:58 ` Sandy Harris
2001-01-20 22:28 ` Andi Kleen
2001-01-22 14:09 ` Ingo Oeser
2001-01-20 22:39 ` Dave Cinege
[not found] <3A6A1512.C2684CCC@interlog.com>
2001-01-20 23:21 ` Dave Cinege
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®