Messages in this thread Patch in this message |  | | From | Neil Brown <> | Date | Thu, 7 Dec 2000 07:54:45 +1100 (EST) | Subject | Re: kernel panic in SoftwareRAID autodetection |
| |
On Wednesday December 6, kressb@fsc-usa.com wrote: > Peter Samuelson wrote: > > > > [Roberto Ragusa] > > > BTW, here is a little patch regarding a silly problem I found > > > about RAID partitions naming (/proc/partitions). > > > No more "md8" "md9" "md:" "md;" ... but "md8" "md9" "md10" "md11" ... > > > Well, this patch should work up to "md99". > > > > This stuff *really* should be split out into the drivers. Brian Kress > > had a patch against test11 for this. Brian? You want to fold in this > > fix? > > Sure. I got resounding silence to posting the patch last time, > so I'm not sure if anyone actually wants this patch, but here it is > again with this fix (the non ugly version) folded in. >
Have you ever noticed that bad patches get a lot more response than good patches? I think people like having something useful to say and a simple "I like it" often doesn't seem worth it, though in reality is very valuable.
Mayge the thing to do is include a few obvious but not very significant errors like: - initialise a static variable to 0 - use /** to introduce a comment that isn't in the right format for the auto-documentation stuff - uses lines wider than 80 characters - use unnecessary extra parentheses - use non-standard indenting
that way people will respond because they think they have something to say, and will hopefully comment further... :-)
But I see that you have done that .... a simple compiler error (I think).
> diff -u --recursive linux-2.4.0-test11/fs/partitions/check.c > linux-2.4.0-test11-ppfix/fs/partitions/check.c > --- linux-2.4.0-test11/fs/partitions/check.c Mon Nov 20 15:17:27 2000 > +++ linux-2.4.0-test11-ppfix/fs/partitions/check.c Thu Nov 23 14:30:45 > 2000 > @@ -83,11 +83,10 @@ > */ > char *disk_name (struct gendisk *hd, int minor, char *buf) > { > - unsigned int part; > const char *maj = hd->major_name; > int unit = (minor >> hd->minor_shift) + 'a'; > + unsigned int part = minor & ((1 << hd->minor_shift) - 1); > > - part = minor & ((1 << hd->minor_shift) - 1);
here we have lost the "part" automatic variable in disk_name but ....
(stuff deleted) > - else > - sprintf(buf, "%s/c%dd%dp%d", maj, ctlr, disk, part); > - return buf; > - } > + if (hd->hd_name) return hd->hd_name(hd, minor, buf); > + > if (part) > sprintf(buf, "%s%c%d", maj, unit, part);
here we use it. Does this compile?
But apart from this, I like it, and I think that it would be good if it went into 2.4.
Maybe do one more iteration (or convince me that the above isn't a bug), and ask for comment. I promise to test and respond. Then send it to Linus, and explain what it does, and mention that it fixes a real issue in lvm (I assume it does as someone said so. I haven't actually looked into that) and leave it to him.
NeilBrown - 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/
|  |