Re: [patch v2 1/1] md: Software Raid autodetect dev list not array
On 8/26/07, Kyle Moffett <[EMAIL PROTECTED]> wrote: > On Aug 26, 2007, at 08:20:45, Michael Evans wrote: > > Also, I forgot to mention, the reason I added the counters was > > mostly for debugging. However they're also as useful in the same > > way that listing the partitions when a new disk is added can be (in > > fact this augments that and the existing messages the autodetect > > routines provide). > > > > As for using autodetect or not... the only way to skip it seems to > > be compiling md's raid support as a module. I checked 2.6.22's > > menuconfig and there's no way for me to explicitly turn it on or > > off at compile time. I also feel that forcing the addition of a > > boot parameter to de-activate a broken and deprecated system you > > aren't even aware you are getting is somehow wrong. So if you have > > over 128 devices for it to scan, as I do on one of my PCs, then it > > can bring up > > an array in degraded mode. ... crud. > > Well, you could just change the MSDOS disk label to use a different > "Partition Type" for your raid partitions. Just pick the standard > "Linux" type and you will get exactly the same behavior that > everybody who doesn't use MSDOS partition tables gets. > > Cheers, > Kyle Moffett > > I recall most of the guides I referenced during setup having me change the partition type, additionally parted only calls the flag 'raid' not 'raid autodetect'. However it would still be confusing to anyone not intimately familiar with the subsystem. Also, even though the system has a standard PC BIOS, I liked some of the specifications of the GUID partition table (GPT) provided. Namely a checksum and backup copy, and up to 128 partitions per disk. Parted is the only real tool I could find for editing such a disk label. A problem I experienced that is almost completely unrelated to this patch are other 'magic number' assumptions. It is rather unfortunate that linux allocates a fixed (and very small) number of partitions per scsi disk. A better method might be a complete dis-association of major:minor pair to device name, and instead simply enumerating partitions as they are detected. That way if I choose to have 128 drives but each having at most 2 partitions I still easily fit within one major, or if I choose the opposite (for whatever reason) I still come to the same conclusion. Before anyone mentions using LVM instead, sharing operating systems, and using different partitions for different raid stripe sizes/rebuilding flexibility/restriping flexibility are just two good reasons I can think of for supporting more then 15 partitions per device. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch v2 1/1] md: Software Raid autodetect dev list not array
On 8/26/07, Kyle Moffett [EMAIL PROTECTED] wrote: On Aug 26, 2007, at 08:20:45, Michael Evans wrote: Also, I forgot to mention, the reason I added the counters was mostly for debugging. However they're also as useful in the same way that listing the partitions when a new disk is added can be (in fact this augments that and the existing messages the autodetect routines provide). As for using autodetect or not... the only way to skip it seems to be compiling md's raid support as a module. I checked 2.6.22's menuconfig and there's no way for me to explicitly turn it on or off at compile time. I also feel that forcing the addition of a boot parameter to de-activate a broken and deprecated system you aren't even aware you are getting is somehow wrong. So if you have over 128 devices for it to scan, as I do on one of my PCs, then it can bring up an array in degraded mode. ... crud. Well, you could just change the MSDOS disk label to use a different Partition Type for your raid partitions. Just pick the standard Linux type and you will get exactly the same behavior that everybody who doesn't use MSDOS partition tables gets. Cheers, Kyle Moffett I recall most of the guides I referenced during setup having me change the partition type, additionally parted only calls the flag 'raid' not 'raid autodetect'. However it would still be confusing to anyone not intimately familiar with the subsystem. Also, even though the system has a standard PC BIOS, I liked some of the specifications of the GUID partition table (GPT) provided. Namely a checksum and backup copy, and up to 128 partitions per disk. Parted is the only real tool I could find for editing such a disk label. A problem I experienced that is almost completely unrelated to this patch are other 'magic number' assumptions. It is rather unfortunate that linux allocates a fixed (and very small) number of partitions per scsi disk. A better method might be a complete dis-association of major:minor pair to device name, and instead simply enumerating partitions as they are detected. That way if I choose to have 128 drives but each having at most 2 partitions I still easily fit within one major, or if I choose the opposite (for whatever reason) I still come to the same conclusion. Before anyone mentions using LVM instead, sharing operating systems, and using different partitions for different raid stripe sizes/rebuilding flexibility/restriping flexibility are just two good reasons I can think of for supporting more then 15 partitions per device. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch v2 1/1] md: Software Raid autodetect dev list not array
On Aug 26, 2007, at 08:20:45, Michael Evans wrote: Also, I forgot to mention, the reason I added the counters was mostly for debugging. However they're also as useful in the same way that listing the partitions when a new disk is added can be (in fact this augments that and the existing messages the autodetect routines provide). As for using autodetect or not... the only way to skip it seems to be compiling md's raid support as a module. I checked 2.6.22's menuconfig and there's no way for me to explicitly turn it on or off at compile time. I also feel that forcing the addition of a boot parameter to de-activate a broken and deprecated system you aren't even aware you are getting is somehow wrong. So if you have over 128 devices for it to scan, as I do on one of my PCs, then it can bring up an array in degraded mode. ... crud. Well, you could just change the MSDOS disk label to use a different "Partition Type" for your raid partitions. Just pick the standard "Linux" type and you will get exactly the same behavior that everybody who doesn't use MSDOS partition tables gets. Cheers, Kyle Moffett - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch v2 1/1] md: Software Raid autodetect dev list not array
On 8/26/07, Randy Dunlap <[EMAIL PROTECTED]> wrote: > On Sun, 26 Aug 2007 04:51:24 -0700 Michael J. Evans wrote: > > > From: Michael J. Evans <[EMAIL PROTECTED]> > > > > Is there any way to tell the user what device (or partition?) is > bein skipped? This printk should just print (confirm) that > node_detected_dev is NULL. Shouldn't it just print in > major:minor format? > It would be possible with the MAJOR() and MINOR() macros to do this... however it doesn't really help out much during troubleshooting. I tried using the bdevname function like the function that calls this one uses, however, it wants a struct device_block... which I tried getting with: container_of(dev, struct block_device, bd_dev) Of course this didn't quite work out, I got kernel panics on my two trial attempts. Here's a skip from a dmesg where I added a printk right under the line in question. [ 63.033532] sd 11:0:0:0: [sdk] 976773168 512-byte hardware sectors (500108 MB) [ 63.039842] sd 11:0:0:0: [sdk] Write Protect is off [ 63.046012] sd 11:0:0:0: [sdk] Mode Sense: 00 3a 00 00 [ 63.046025] sd 11:0:0:0: [sdk] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA [ 63.052309] sdk: sdk1 sdk2 sdk3 sdk4 sdk5 sdk6 sdk7 sdk8 sdk9 sdk10 sdk11 sdk12 sdk13 sdk14 sdk15 [ 63.082546] md: Autodetect-buffering the above device. [ 63.088893] md: Autodetect-buffering the above device. [ 63.095053] md: Autodetect-buffering the above device. [ 63.101082] md: Autodetect-buffering the above device. [ 63.106956] md: Autodetect-buffering the above device. [ 63.112596] md: Autodetect-buffering the above device. [ 63.117998] md: Autodetect-buffering the above device. [ 63.123396] md: Autodetect-buffering the above device. [ 63.128789] md: Autodetect-buffering the above device. [ 63.134182] md: Autodetect-buffering the above device. [ 63.139576] md: Autodetect-buffering the above device. [ 63.144970] md: Autodetect-buffering the above device. [ 63.150360] md: Autodetect-buffering the above device. [ 63.155749] md: Autodetect-buffering the above device. [ 63.161498] sd 11:0:0:0: [sdk] Attached SCSI disk > --- > ~Randy > *** Remember to use Documentation/SubmitChecklist when testing your code *** > - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch v2 1/1] md: Software Raid autodetect dev list not array
On Sun, 26 Aug 2007 04:51:24 -0700 Michael J. Evans wrote:
> From: Michael J. Evans <[EMAIL PROTECTED]>
>
> In current release kernels the md module (Software RAID) uses a static array
> (dev_t[128]) to store partition/device info temporarily for autostart.
>
> This patch replaces that static array with a list.
>
> Signed-off-by: Michael J. Evans <[EMAIL PROTECTED]>
> ---
> Version 2: Following Neil Brown's requests...
> using list_add_tail, and corrected missing i_passed++;.
> removed sections of code that would never be reached.
> - -
> The data/structures are only used within md.c, and very close together.
> However I wonder if the structural information shouldn't go in to...
> ../../include/linux/raid/md_k.h instead.
>
>
> I discovered this (and that the devices are added as disks/partitions are
> discovered at boot) while I was debugging why only one of my MD arrays would
> come up whole, while all the others were short a disk.
>
> I eventually discovered that it was enumerating through all of 9 of my 11 hds
> (2 had only 4 partitions apiece) while the other 9 have 15 partitions
> (I wanted 64 per drive...). The last partition of the 8th drive in my 9 drive
> raid 5 sets wasn't added, thus making the final md array short both a parity
> and data disk, and it was started later, elsewhere.
>
> Subject: [patch 1/1] md: Software Raid autodetect dev list not array
>
> SOFTWARE RAID (Multiple Disks) SUPPORT
> P:Ingo Molnar
> M:[EMAIL PROTECTED]
> P:Neil Brown
> M:[EMAIL PROTECTED]
> L:[EMAIL PROTECTED]
> S:Supported
> Unless you have a reason NOT to do so, CC [EMAIL PROTECTED]
>
> 12: Has been tested with CONFIG_PREEMPT, CONFIG_DEBUG_PREEMPT,
> CONFIG_DEBUG_SLAB, CONFIG_DEBUG_PAGEALLOC, CONFIG_DEBUG_MUTEXES,
> CONFIG_DEBUG_SPINLOCK, CONFIG_DEBUG_SPINLOCK_SLEEP all simultaneously
> enabled.
>
> It has been tested with CONFIG_SMP set and unset (Different x86_64 systems).
> It has been tested with CONFIG_PREEMPT set and unset (same system).
> CONFIG_LBD isn't even an option in my .config file.
It's not an option 64_BIT builds.
> Note: between 2.6.22 and 2.6.23-rc3-git5
> rdev = md_import_device(dev,0, 0);
> became
> rdev = md_import_device(dev,0, 90);
> So the patch has been edited to patch around that line. (might be fuzzy)
>
> Signed-off-by: Michael J. Evans <[EMAIL PROTECTED]>
> =
> --- linux/drivers/md/md.c.orig2007-08-21 03:19:42.511576248 -0700
> +++ linux/drivers/md/md.c 2007-08-21 04:30:09.775525710 -0700
> @@ -5752,13 +5754,24 @@ void md_autodetect_dev(dev_t dev)
> * Searches all registered partitions for autorun RAID arrays
> * at boot time.
> */
> -static dev_t detected_devices[128];
> -static int dev_cnt;
> +
> +static LIST_HEAD(all_detected_devices);
> +struct detected_devices_node {
> + struct list_head list;
> + dev_t dev;
> +};
>
> void md_autodetect_dev(dev_t dev)
> {
> - if (dev_cnt >= 0 && dev_cnt < 127)
> - detected_devices[dev_cnt++] = dev;
> + struct detected_devices_node *node_detected_dev;
> + node_detected_dev = kzalloc(sizeof(*node_detected_dev), GFP_KERNEL);\
> + if (node_detected_dev) {
> + node_detected_dev->dev = dev;
> + list_add_tail(_detected_dev->list, _detected_devices);
> + } else {
> + printk(KERN_CRIT "md: kzAlloc node failed, skipping device."
> + " : 0x%p.\n", node_detected_dev);
Is there any way to tell the user what device (or partition?) is
bein skipped? This printk should just print (confirm) that
node_detected_dev is NULL. Shouldn't it just print in
major:minor format?
> + }
> }
>
>
> @@ -5765,7 +5778,12 @@ static void autostart_arrays(int part)
> static void autostart_arrays(int part)
> {
> mdk_rdev_t *rdev;
> - int i;
> + struct detected_devices_node *node_detected_dev;
> + dev_t dev;
> + int i_scanned, i_passed;
> + signed int i_found;
Drop "signed", like the surrounding code.
Leave a blank line between data declarations and beginning of code.
> + i_scanned = 0;
> + i_passed = 0;
>
> printk(KERN_INFO "md: Autodetecting RAID arrays.\n");
>
> @@ -5772,3 +5790,8 @@ static void autostart_arrays(int part)
> - for (i = 0; i < dev_cnt; i++) {
> - dev_t dev = detected_devices[i];
> -
> + /* FIXME: max 'int' #DEFINEd somewhere? not 0x7FFF ? */
include/linux/kernel.h has INT_MAX, UINT_MAX, LONG_MAX, ULONG_MAX,
LLONG_MAX, ULLONG_MAX.
> + while (!list_empty(_detected_devices) && i_scanned < 0x7FFF) {
> + i_scanned++;
> + node_detected_dev = list_entry(all_detected_devices.next,
> + struct detected_devices_node, list);
> + list_del(_detected_dev->list);
> + dev = node_detected_dev->dev;
> +
Re: [patch v2 1/1] md: Software Raid autodetect dev list not array
On 8/26/07, Jan Engelhardt <[EMAIL PROTECTED]> wrote:
>
> On Aug 26 2007 04:51, Michael J. Evans wrote:
> > {
> >- if (dev_cnt >= 0 && dev_cnt < 127)
> >- detected_devices[dev_cnt++] = dev;
> >+ struct detected_devices_node *node_detected_dev;
> >+ node_detected_dev = kzalloc(sizeof(*node_detected_dev), GFP_KERNEL);\
>
> What's the \ good for, besides escaping the newline
> that is ignored as whitespace anyway? :-)
>
I hadn't even noticed that, I suppose I mashed the key above enter at
some time. Removing from my local file.
> >@@ -5772,3 +5790,8 @@ static void autostart_arrays(int part)
> >- for (i = 0; i < dev_cnt; i++) {
> >- dev_t dev = detected_devices[i];
> >-
> >+ /* FIXME: max 'int' #DEFINEd somewhere? not 0x7FFF ? */
> >+ while (!list_empty(_detected_devices) && i_scanned < 0x7FFF) {
>
> I doubt someone really has _that_ many devices. Of course, to be on the
> safer side, make it an unsigned int. That way, people could put in about
> 0xFFFE devs (which is even less likely than 0x7FFF)
>
There is that, but I'm almost expecting someone to ask me to remove
both the ints and kprint statement. (I'd like them as part of some
kind of verbose startup that people would actually think to use
however.) Additionally a though occurred to me earlier, if there are
That many devices, the chance of a UUID namespace collision might
actually be realistic anyway. Though I'm not short sighted enough to
put it past anyone to have more then 32/64K possible block devices.
Anyone with that much cash today is probably buying hardware raid, but
who knows.
> >+ i_scanned++;
> >+ node_detected_dev = list_entry(all_detected_devices.next,
> >+ struct detected_devices_node, list);
> >+ list_del(_detected_dev->list);
> >+ dev = node_detected_dev->dev;
> >+ kfree(node_detected_dev);
>
> Jan
> --
>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch v2 1/1] md: Software Raid autodetect dev list not array
On Aug 26 2007 04:51, Michael J. Evans wrote:
> {
>- if (dev_cnt >= 0 && dev_cnt < 127)
>- detected_devices[dev_cnt++] = dev;
>+ struct detected_devices_node *node_detected_dev;
>+ node_detected_dev = kzalloc(sizeof(*node_detected_dev), GFP_KERNEL);\
What's the \ good for, besides escaping the newline
that is ignored as whitespace anyway? :-)
>@@ -5772,3 +5790,8 @@ static void autostart_arrays(int part)
>- for (i = 0; i < dev_cnt; i++) {
>- dev_t dev = detected_devices[i];
>-
>+ /* FIXME: max 'int' #DEFINEd somewhere? not 0x7FFF ? */
>+ while (!list_empty(_detected_devices) && i_scanned < 0x7FFF) {
I doubt someone really has _that_ many devices. Of course, to be on the
safer side, make it an unsigned int. That way, people could put in about
0xFFFE devs (which is even less likely than 0x7FFF)
>+ i_scanned++;
>+ node_detected_dev = list_entry(all_detected_devices.next,
>+ struct detected_devices_node, list);
>+ list_del(_detected_dev->list);
>+ dev = node_detected_dev->dev;
>+ kfree(node_detected_dev);
Jan
--
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch v2 1/1] md: Software Raid autodetect dev list not array
Also, I forgot to mention, the reason I added the counters was mostly for debugging. However they're also as useful in the same way that listing the partitions when a new disk is added can be (in fact this augments that and the existing messages the autodetect routines provide). As for using autodetect or not... the only way to skip it seems to be compiling md's raid support as a module. I checked 2.6.22's menuconfig and there's no way for me to explicitly turn it on or off at compile time. I also feel that forcing the addition of a boot parameter to de-activate a broken and deprecated system you aren't even aware you are getting is somehow wrong. So if you have over 128 devices for it to scan, as I do on one of my PCs, then it can bring up an array in degraded mode. ... crud. I also just noticed, while looking to see if there was some existing way of detecting if debugging were enabled and to be extra-verbose, that I left in one of my other debugging variables by mistake. i_found. Since it's signed, it must have been the variable I was using to detect where my list matched the existing array in my initial verification runs. Are there any other things you'd like to see changed before I submit a third patch version? - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[patch v2 1/1] md: Software Raid autodetect dev list not array
From: Michael J. Evans <[EMAIL PROTECTED]>
In current release kernels the md module (Software RAID) uses a static array
(dev_t[128]) to store partition/device info temporarily for autostart.
This patch replaces that static array with a list.
Signed-off-by: Michael J. Evans <[EMAIL PROTECTED]>
---
Version 2: Following Neil Brown's requests...
using list_add_tail, and corrected missing i_passed++;.
removed sections of code that would never be reached.
- -
The data/structures are only used within md.c, and very close together.
However I wonder if the structural information shouldn't go in to...
../../include/linux/raid/md_k.h instead.
I discovered this (and that the devices are added as disks/partitions are
discovered at boot) while I was debugging why only one of my MD arrays would
come up whole, while all the others were short a disk.
I eventually discovered that it was enumerating through all of 9 of my 11 hds
(2 had only 4 partitions apiece) while the other 9 have 15 partitions
(I wanted 64 per drive...). The last partition of the 8th drive in my 9 drive
raid 5 sets wasn't added, thus making the final md array short both a parity
and data disk, and it was started later, elsewhere.
Subject: [patch 1/1] md: Software Raid autodetect dev list not array
SOFTWARE RAID (Multiple Disks) SUPPORT
P: Ingo Molnar
M: [EMAIL PROTECTED]
P: Neil Brown
M: [EMAIL PROTECTED]
L: [EMAIL PROTECTED]
S: Supported
Unless you have a reason NOT to do so, CC [EMAIL PROTECTED]
12: Has been tested with CONFIG_PREEMPT, CONFIG_DEBUG_PREEMPT,
CONFIG_DEBUG_SLAB, CONFIG_DEBUG_PAGEALLOC, CONFIG_DEBUG_MUTEXES,
CONFIG_DEBUG_SPINLOCK, CONFIG_DEBUG_SPINLOCK_SLEEP all simultaneously
enabled.
It has been tested with CONFIG_SMP set and unset (Different x86_64 systems).
It has been tested with CONFIG_PREEMPT set and unset (same system).
CONFIG_LBD isn't even an option in my .config file.
Note: between 2.6.22 and 2.6.23-rc3-git5
rdev = md_import_device(dev,0, 0);
became
rdev = md_import_device(dev,0, 90);
So the patch has been edited to patch around that line. (might be fuzzy)
Signed-off-by: Michael J. Evans <[EMAIL PROTECTED]>
=
--- linux/drivers/md/md.c.orig 2007-08-21 03:19:42.511576248 -0700
+++ linux/drivers/md/md.c 2007-08-21 04:30:09.775525710 -0700
@@ -24,4 +24,6 @@
+ - autodetect dev list not array: Michael J. Evans <[EMAIL PROTECTED]>
+
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation; either version 2, or (at your option)
@@ -5752,13 +5754,24 @@ void md_autodetect_dev(dev_t dev)
* Searches all registered partitions for autorun RAID arrays
* at boot time.
*/
-static dev_t detected_devices[128];
-static int dev_cnt;
+
+static LIST_HEAD(all_detected_devices);
+struct detected_devices_node {
+ struct list_head list;
+ dev_t dev;
+};
void md_autodetect_dev(dev_t dev)
{
- if (dev_cnt >= 0 && dev_cnt < 127)
- detected_devices[dev_cnt++] = dev;
+ struct detected_devices_node *node_detected_dev;
+ node_detected_dev = kzalloc(sizeof(*node_detected_dev), GFP_KERNEL);\
+ if (node_detected_dev) {
+ node_detected_dev->dev = dev;
+ list_add_tail(_detected_dev->list, _detected_devices);
+ } else {
+ printk(KERN_CRIT "md: kzAlloc node failed, skipping device."
+" : 0x%p.\n", node_detected_dev);
+ }
}
@@ -5765,7 +5778,12 @@ static void autostart_arrays(int part)
static void autostart_arrays(int part)
{
mdk_rdev_t *rdev;
- int i;
+ struct detected_devices_node *node_detected_dev;
+ dev_t dev;
+ int i_scanned, i_passed;
+ signed int i_found;
+ i_scanned = 0;
+ i_passed = 0;
printk(KERN_INFO "md: Autodetecting RAID arrays.\n");
@@ -5772,3 +5790,8 @@ static void autostart_arrays(int part)
- for (i = 0; i < dev_cnt; i++) {
- dev_t dev = detected_devices[i];
-
+ /* FIXME: max 'int' #DEFINEd somewhere? not 0x7FFF ? */
+ while (!list_empty(_detected_devices) && i_scanned < 0x7FFF) {
+ i_scanned++;
+ node_detected_dev = list_entry(all_detected_devices.next,
+ struct detected_devices_node, list);
+ list_del(_detected_dev->list);
+ dev = node_detected_dev->dev;
+ kfree(node_detected_dev);
@@ -5781,8 +5806,11 @@ static void autostart_arrays(int part)
continue;
}
list_add(>same_set, _raid_disks);
+ i_passed++;
}
- dev_cnt = 0;
+
+ printk(KERN_INFO "md: Scanned %d and added %d devices.\n",
+
[patch v2 1/1] md: Software Raid autodetect dev list not array
From: Michael J. Evans [EMAIL PROTECTED]
In current release kernels the md module (Software RAID) uses a static array
(dev_t[128]) to store partition/device info temporarily for autostart.
This patch replaces that static array with a list.
Signed-off-by: Michael J. Evans [EMAIL PROTECTED]
---
Version 2: Following Neil Brown's requests...
using list_add_tail, and corrected missing i_passed++;.
removed sections of code that would never be reached.
- -
The data/structures are only used within md.c, and very close together.
However I wonder if the structural information shouldn't go in to...
../../include/linux/raid/md_k.h instead.
I discovered this (and that the devices are added as disks/partitions are
discovered at boot) while I was debugging why only one of my MD arrays would
come up whole, while all the others were short a disk.
I eventually discovered that it was enumerating through all of 9 of my 11 hds
(2 had only 4 partitions apiece) while the other 9 have 15 partitions
(I wanted 64 per drive...). The last partition of the 8th drive in my 9 drive
raid 5 sets wasn't added, thus making the final md array short both a parity
and data disk, and it was started later, elsewhere.
Subject: [patch 1/1] md: Software Raid autodetect dev list not array
SOFTWARE RAID (Multiple Disks) SUPPORT
P: Ingo Molnar
M: [EMAIL PROTECTED]
P: Neil Brown
M: [EMAIL PROTECTED]
L: [EMAIL PROTECTED]
S: Supported
Unless you have a reason NOT to do so, CC [EMAIL PROTECTED]
12: Has been tested with CONFIG_PREEMPT, CONFIG_DEBUG_PREEMPT,
CONFIG_DEBUG_SLAB, CONFIG_DEBUG_PAGEALLOC, CONFIG_DEBUG_MUTEXES,
CONFIG_DEBUG_SPINLOCK, CONFIG_DEBUG_SPINLOCK_SLEEP all simultaneously
enabled.
It has been tested with CONFIG_SMP set and unset (Different x86_64 systems).
It has been tested with CONFIG_PREEMPT set and unset (same system).
CONFIG_LBD isn't even an option in my .config file.
Note: between 2.6.22 and 2.6.23-rc3-git5
rdev = md_import_device(dev,0, 0);
became
rdev = md_import_device(dev,0, 90);
So the patch has been edited to patch around that line. (might be fuzzy)
Signed-off-by: Michael J. Evans [EMAIL PROTECTED]
=
--- linux/drivers/md/md.c.orig 2007-08-21 03:19:42.511576248 -0700
+++ linux/drivers/md/md.c 2007-08-21 04:30:09.775525710 -0700
@@ -24,4 +24,6 @@
+ - autodetect dev list not array: Michael J. Evans [EMAIL PROTECTED]
+
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation; either version 2, or (at your option)
@@ -5752,13 +5754,24 @@ void md_autodetect_dev(dev_t dev)
* Searches all registered partitions for autorun RAID arrays
* at boot time.
*/
-static dev_t detected_devices[128];
-static int dev_cnt;
+
+static LIST_HEAD(all_detected_devices);
+struct detected_devices_node {
+ struct list_head list;
+ dev_t dev;
+};
void md_autodetect_dev(dev_t dev)
{
- if (dev_cnt = 0 dev_cnt 127)
- detected_devices[dev_cnt++] = dev;
+ struct detected_devices_node *node_detected_dev;
+ node_detected_dev = kzalloc(sizeof(*node_detected_dev), GFP_KERNEL);\
+ if (node_detected_dev) {
+ node_detected_dev-dev = dev;
+ list_add_tail(node_detected_dev-list, all_detected_devices);
+ } else {
+ printk(KERN_CRIT md: kzAlloc node failed, skipping device.
+ : 0x%p.\n, node_detected_dev);
+ }
}
@@ -5765,7 +5778,12 @@ static void autostart_arrays(int part)
static void autostart_arrays(int part)
{
mdk_rdev_t *rdev;
- int i;
+ struct detected_devices_node *node_detected_dev;
+ dev_t dev;
+ int i_scanned, i_passed;
+ signed int i_found;
+ i_scanned = 0;
+ i_passed = 0;
printk(KERN_INFO md: Autodetecting RAID arrays.\n);
@@ -5772,3 +5790,8 @@ static void autostart_arrays(int part)
- for (i = 0; i dev_cnt; i++) {
- dev_t dev = detected_devices[i];
-
+ /* FIXME: max 'int' #DEFINEd somewhere? not 0x7FFF ? */
+ while (!list_empty(all_detected_devices) i_scanned 0x7FFF) {
+ i_scanned++;
+ node_detected_dev = list_entry(all_detected_devices.next,
+ struct detected_devices_node, list);
+ list_del(node_detected_dev-list);
+ dev = node_detected_dev-dev;
+ kfree(node_detected_dev);
@@ -5781,8 +5806,11 @@ static void autostart_arrays(int part)
continue;
}
list_add(rdev-same_set, pending_raid_disks);
+ i_passed++;
}
- dev_cnt = 0;
+
+ printk(KERN_INFO md: Scanned %d and added %d devices.\n,
+
Re: [patch v2 1/1] md: Software Raid autodetect dev list not array
Also, I forgot to mention, the reason I added the counters was mostly for debugging. However they're also as useful in the same way that listing the partitions when a new disk is added can be (in fact this augments that and the existing messages the autodetect routines provide). As for using autodetect or not... the only way to skip it seems to be compiling md's raid support as a module. I checked 2.6.22's menuconfig and there's no way for me to explicitly turn it on or off at compile time. I also feel that forcing the addition of a boot parameter to de-activate a broken and deprecated system you aren't even aware you are getting is somehow wrong. So if you have over 128 devices for it to scan, as I do on one of my PCs, then it can bring up an array in degraded mode. ... crud. I also just noticed, while looking to see if there was some existing way of detecting if debugging were enabled and to be extra-verbose, that I left in one of my other debugging variables by mistake. i_found. Since it's signed, it must have been the variable I was using to detect where my list matched the existing array in my initial verification runs. Are there any other things you'd like to see changed before I submit a third patch version? - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch v2 1/1] md: Software Raid autodetect dev list not array
On Aug 26 2007 04:51, Michael J. Evans wrote:
{
- if (dev_cnt = 0 dev_cnt 127)
- detected_devices[dev_cnt++] = dev;
+ struct detected_devices_node *node_detected_dev;
+ node_detected_dev = kzalloc(sizeof(*node_detected_dev), GFP_KERNEL);\
What's the \ good for, besides escaping the newline
that is ignored as whitespace anyway? :-)
@@ -5772,3 +5790,8 @@ static void autostart_arrays(int part)
- for (i = 0; i dev_cnt; i++) {
- dev_t dev = detected_devices[i];
-
+ /* FIXME: max 'int' #DEFINEd somewhere? not 0x7FFF ? */
+ while (!list_empty(all_detected_devices) i_scanned 0x7FFF) {
I doubt someone really has _that_ many devices. Of course, to be on the
safer side, make it an unsigned int. That way, people could put in about
0xFFFE devs (which is even less likely than 0x7FFF)
+ i_scanned++;
+ node_detected_dev = list_entry(all_detected_devices.next,
+ struct detected_devices_node, list);
+ list_del(node_detected_dev-list);
+ dev = node_detected_dev-dev;
+ kfree(node_detected_dev);
Jan
--
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch v2 1/1] md: Software Raid autodetect dev list not array
On 8/26/07, Jan Engelhardt [EMAIL PROTECTED] wrote:
On Aug 26 2007 04:51, Michael J. Evans wrote:
{
- if (dev_cnt = 0 dev_cnt 127)
- detected_devices[dev_cnt++] = dev;
+ struct detected_devices_node *node_detected_dev;
+ node_detected_dev = kzalloc(sizeof(*node_detected_dev), GFP_KERNEL);\
What's the \ good for, besides escaping the newline
that is ignored as whitespace anyway? :-)
I hadn't even noticed that, I suppose I mashed the key above enter at
some time. Removing from my local file.
@@ -5772,3 +5790,8 @@ static void autostart_arrays(int part)
- for (i = 0; i dev_cnt; i++) {
- dev_t dev = detected_devices[i];
-
+ /* FIXME: max 'int' #DEFINEd somewhere? not 0x7FFF ? */
+ while (!list_empty(all_detected_devices) i_scanned 0x7FFF) {
I doubt someone really has _that_ many devices. Of course, to be on the
safer side, make it an unsigned int. That way, people could put in about
0xFFFE devs (which is even less likely than 0x7FFF)
There is that, but I'm almost expecting someone to ask me to remove
both the ints and kprint statement. (I'd like them as part of some
kind of verbose startup that people would actually think to use
however.) Additionally a though occurred to me earlier, if there are
That many devices, the chance of a UUID namespace collision might
actually be realistic anyway. Though I'm not short sighted enough to
put it past anyone to have more then 32/64K possible block devices.
Anyone with that much cash today is probably buying hardware raid, but
who knows.
+ i_scanned++;
+ node_detected_dev = list_entry(all_detected_devices.next,
+ struct detected_devices_node, list);
+ list_del(node_detected_dev-list);
+ dev = node_detected_dev-dev;
+ kfree(node_detected_dev);
Jan
--
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch v2 1/1] md: Software Raid autodetect dev list not array
On Sun, 26 Aug 2007 04:51:24 -0700 Michael J. Evans wrote:
From: Michael J. Evans [EMAIL PROTECTED]
In current release kernels the md module (Software RAID) uses a static array
(dev_t[128]) to store partition/device info temporarily for autostart.
This patch replaces that static array with a list.
Signed-off-by: Michael J. Evans [EMAIL PROTECTED]
---
Version 2: Following Neil Brown's requests...
using list_add_tail, and corrected missing i_passed++;.
removed sections of code that would never be reached.
- -
The data/structures are only used within md.c, and very close together.
However I wonder if the structural information shouldn't go in to...
../../include/linux/raid/md_k.h instead.
I discovered this (and that the devices are added as disks/partitions are
discovered at boot) while I was debugging why only one of my MD arrays would
come up whole, while all the others were short a disk.
I eventually discovered that it was enumerating through all of 9 of my 11 hds
(2 had only 4 partitions apiece) while the other 9 have 15 partitions
(I wanted 64 per drive...). The last partition of the 8th drive in my 9 drive
raid 5 sets wasn't added, thus making the final md array short both a parity
and data disk, and it was started later, elsewhere.
Subject: [patch 1/1] md: Software Raid autodetect dev list not array
SOFTWARE RAID (Multiple Disks) SUPPORT
P:Ingo Molnar
M:[EMAIL PROTECTED]
P:Neil Brown
M:[EMAIL PROTECTED]
L:[EMAIL PROTECTED]
S:Supported
Unless you have a reason NOT to do so, CC [EMAIL PROTECTED]
12: Has been tested with CONFIG_PREEMPT, CONFIG_DEBUG_PREEMPT,
CONFIG_DEBUG_SLAB, CONFIG_DEBUG_PAGEALLOC, CONFIG_DEBUG_MUTEXES,
CONFIG_DEBUG_SPINLOCK, CONFIG_DEBUG_SPINLOCK_SLEEP all simultaneously
enabled.
It has been tested with CONFIG_SMP set and unset (Different x86_64 systems).
It has been tested with CONFIG_PREEMPT set and unset (same system).
CONFIG_LBD isn't even an option in my .config file.
It's not an option 64_BIT builds.
Note: between 2.6.22 and 2.6.23-rc3-git5
rdev = md_import_device(dev,0, 0);
became
rdev = md_import_device(dev,0, 90);
So the patch has been edited to patch around that line. (might be fuzzy)
Signed-off-by: Michael J. Evans [EMAIL PROTECTED]
=
--- linux/drivers/md/md.c.orig2007-08-21 03:19:42.511576248 -0700
+++ linux/drivers/md/md.c 2007-08-21 04:30:09.775525710 -0700
@@ -5752,13 +5754,24 @@ void md_autodetect_dev(dev_t dev)
* Searches all registered partitions for autorun RAID arrays
* at boot time.
*/
-static dev_t detected_devices[128];
-static int dev_cnt;
+
+static LIST_HEAD(all_detected_devices);
+struct detected_devices_node {
+ struct list_head list;
+ dev_t dev;
+};
void md_autodetect_dev(dev_t dev)
{
- if (dev_cnt = 0 dev_cnt 127)
- detected_devices[dev_cnt++] = dev;
+ struct detected_devices_node *node_detected_dev;
+ node_detected_dev = kzalloc(sizeof(*node_detected_dev), GFP_KERNEL);\
+ if (node_detected_dev) {
+ node_detected_dev-dev = dev;
+ list_add_tail(node_detected_dev-list, all_detected_devices);
+ } else {
+ printk(KERN_CRIT md: kzAlloc node failed, skipping device.
+ : 0x%p.\n, node_detected_dev);
Is there any way to tell the user what device (or partition?) is
bein skipped? This printk should just print (confirm) that
node_detected_dev is NULL. Shouldn't it just print dev in
major:minor format?
+ }
}
@@ -5765,7 +5778,12 @@ static void autostart_arrays(int part)
static void autostart_arrays(int part)
{
mdk_rdev_t *rdev;
- int i;
+ struct detected_devices_node *node_detected_dev;
+ dev_t dev;
+ int i_scanned, i_passed;
+ signed int i_found;
Drop signed, like the surrounding code.
Leave a blank line between data declarations and beginning of code.
+ i_scanned = 0;
+ i_passed = 0;
printk(KERN_INFO md: Autodetecting RAID arrays.\n);
@@ -5772,3 +5790,8 @@ static void autostart_arrays(int part)
- for (i = 0; i dev_cnt; i++) {
- dev_t dev = detected_devices[i];
-
+ /* FIXME: max 'int' #DEFINEd somewhere? not 0x7FFF ? */
include/linux/kernel.h has INT_MAX, UINT_MAX, LONG_MAX, ULONG_MAX,
LLONG_MAX, ULLONG_MAX.
+ while (!list_empty(all_detected_devices) i_scanned 0x7FFF) {
+ i_scanned++;
+ node_detected_dev = list_entry(all_detected_devices.next,
+ struct detected_devices_node, list);
+ list_del(node_detected_dev-list);
+ dev = node_detected_dev-dev;
+ kfree(node_detected_dev);
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
-
To unsubscribe from
Re: [patch v2 1/1] md: Software Raid autodetect dev list not array
On 8/26/07, Randy Dunlap [EMAIL PROTECTED] wrote: On Sun, 26 Aug 2007 04:51:24 -0700 Michael J. Evans wrote: From: Michael J. Evans [EMAIL PROTECTED] Is there any way to tell the user what device (or partition?) is bein skipped? This printk should just print (confirm) that node_detected_dev is NULL. Shouldn't it just print dev in major:minor format? It would be possible with the MAJOR() and MINOR() macros to do this... however it doesn't really help out much during troubleshooting. I tried using the bdevname function like the function that calls this one uses, however, it wants a struct device_block... which I tried getting with: container_of(dev, struct block_device, bd_dev) Of course this didn't quite work out, I got kernel panics on my two trial attempts. Here's a skip from a dmesg where I added a printk right under the line in question. [ 63.033532] sd 11:0:0:0: [sdk] 976773168 512-byte hardware sectors (500108 MB) [ 63.039842] sd 11:0:0:0: [sdk] Write Protect is off [ 63.046012] sd 11:0:0:0: [sdk] Mode Sense: 00 3a 00 00 [ 63.046025] sd 11:0:0:0: [sdk] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA [ 63.052309] sdk: sdk1 sdk2 sdk3 sdk4 sdk5 sdk6 sdk7 sdk8 sdk9 sdk10 sdk11 sdk12 sdk13 sdk14 sdk15 [ 63.082546] md: Autodetect-buffering the above device. [ 63.088893] md: Autodetect-buffering the above device. [ 63.095053] md: Autodetect-buffering the above device. [ 63.101082] md: Autodetect-buffering the above device. [ 63.106956] md: Autodetect-buffering the above device. [ 63.112596] md: Autodetect-buffering the above device. [ 63.117998] md: Autodetect-buffering the above device. [ 63.123396] md: Autodetect-buffering the above device. [ 63.128789] md: Autodetect-buffering the above device. [ 63.134182] md: Autodetect-buffering the above device. [ 63.139576] md: Autodetect-buffering the above device. [ 63.144970] md: Autodetect-buffering the above device. [ 63.150360] md: Autodetect-buffering the above device. [ 63.155749] md: Autodetect-buffering the above device. [ 63.161498] sd 11:0:0:0: [sdk] Attached SCSI disk --- ~Randy *** Remember to use Documentation/SubmitChecklist when testing your code *** - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch v2 1/1] md: Software Raid autodetect dev list not array
On Aug 26, 2007, at 08:20:45, Michael Evans wrote: Also, I forgot to mention, the reason I added the counters was mostly for debugging. However they're also as useful in the same way that listing the partitions when a new disk is added can be (in fact this augments that and the existing messages the autodetect routines provide). As for using autodetect or not... the only way to skip it seems to be compiling md's raid support as a module. I checked 2.6.22's menuconfig and there's no way for me to explicitly turn it on or off at compile time. I also feel that forcing the addition of a boot parameter to de-activate a broken and deprecated system you aren't even aware you are getting is somehow wrong. So if you have over 128 devices for it to scan, as I do on one of my PCs, then it can bring up an array in degraded mode. ... crud. Well, you could just change the MSDOS disk label to use a different Partition Type for your raid partitions. Just pick the standard Linux type and you will get exactly the same behavior that everybody who doesn't use MSDOS partition tables gets. Cheers, Kyle Moffett - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
