LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* Re: [patch 1/1] md: Software Raid autodetect dev list not array
       [not found] <200708222058.45480.mjevans1983@comcast.net>
@ 2007-08-24  3:37 ` Neil Brown
  2007-08-24  5:50   ` Michael Evans
  2007-08-26 11:51   ` [patch v2 " Michael J. Evans
  0 siblings, 2 replies; 28+ messages in thread
From: Neil Brown @ 2007-08-24  3:37 UTC (permalink / raw)
  To: Michael J. Evans; +Cc: Ingo Molnar, linux-raid, linux-kernel, Michael J . Evans

On Wednesday August 22, mjevans1983@comcast.net wrote:
> From: Michael J. Evans <mjevans1983@gmail.com>
> 
> 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.

I must admit that I'm not very keen on this.
I would much rather that in-kernel autodetect were deprecated rather
than enhanced.

Just use 'mdadm' in an initrd, or during normal boot, to assemble all
your arrays.

However.....

> =============================================================
> --- 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 <mjevans1983@gmail.com>
> +
>     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,25 @@ 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);
> +/* FIXME : Should these 4 lines instead go in to include/linux/raid/md_k.h ? 

No.  No-one outside this file uses them, so they are fine where they
are.

> */
> +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(&node_detected_dev->list, &all_detected_devices);

Probably list_add_tail would be better so the ordering is the same as
it was before?


> +	} else {
> +		printk(KERN_CRIT "md: kzAlloc node failed, skipping device."
> +				 " : 0x%p.\n", node_detected_dev);
> +	}



>  }
>  
>  
> @@ -5765,7 +5779,13 @@ 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, i_loops;
> +	signed int i_found;
> +	i_scanned = 0;
> +	i_passed = 0;
> +	i_loops = 0;

i_passed is never used.

And what is the point of i_loops (and i_scanned)?  The comments
at the top of the patch should explain this if there is a good reason.

>  
>  	printk(KERN_INFO "md: Autodetecting RAID arrays.\n");
>  
> @@ -5772,3 +5792,11 @@ 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   0x7FFFFFFF ? */
> +	while (!list_empty(&all_detected_devices) && i_loops < 0x7FFFFFFF) {
> +		i_scanned++;
> +		node_detected_dev = NULL;
> +		node_detected_dev = list_entry(all_detected_devices.next,
> +					struct detected_devices_node, list);
> +		if (node_detected_dev) {

list_entry will *never* return NULL.   It simply doesn't pointer
arithmetic.  (Well, I guess it could return NULL if it was passed
NULL, and the struct-offset were 0, but that isn't the case here).

So you don't need this 'if' at all.

> +			list_del(&node_detected_dev->list);
> +			dev = node_detected_dev->dev;
> +			kfree(node_detected_dev);
> +		/* Indent is now off by one, but the old code is after this. */

so you don't need to worrying about indents.


> @@ -5781,8 +5809,16 @@ static void autostart_arrays(int part)
>  			continue;
>  		}
>  		list_add(&rdev->same_set, &pending_raid_disks);
> +		/* Indent is now off by one, but the old code is above this. */
> +
> +		} else {
> +			printk(KERN_CRIT "md: Invalid list node, skipping.\n");
> +		}
> +		i_loops++;
>  	}
> -	dev_cnt = 0;
> +
> +	printk(KERN_INFO "md: Passes %d : Scanned %d and added %d devices.\n",
> +						i_loops, i_scanned, i_passed);
>  
>  	autorun_devices(part);
>  }


I'm not dead-set against the change, and if you tidy it up I'll
probably accept it, but I really think you would be better off skipping
the in-kernel autodetect stuff altogether and use mdadm to assemble
your arrays.

NeilBrown

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch 1/1] md: Software Raid autodetect dev list not array
  2007-08-24  3:37 ` [patch 1/1] md: Software Raid autodetect dev list not array Neil Brown
@ 2007-08-24  5:50   ` Michael Evans
  2007-08-26 11:51   ` [patch v2 " Michael J. Evans
  1 sibling, 0 replies; 28+ messages in thread
From: Michael Evans @ 2007-08-24  5:50 UTC (permalink / raw)
  To: Neil Brown; +Cc: Michael J. Evans, Ingo Molnar, linux-raid, linux-kernel

I'll look at this again on my next weekend and make the changes.

If it exists I'd rather it functioned without issues.  My initrds are
created by gentoo's genkernel script, which places dmraid on them.
I'm not sure if it supports autodetect or not.

On 8/24/07, Neil Brown <neilb@suse.de> wrote:
> On Wednesday August 22, mjevans1983@comcast.net wrote:
> > From: Michael J. Evans <mjevans1983@gmail.com>
> >
> > 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.
>
> I must admit that I'm not very keen on this.
> I would much rather that in-kernel autodetect were deprecated rather
> than enhanced.
>
> Just use 'mdadm' in an initrd, or during normal boot, to assemble all
> your arrays.
>
> However.....
>
> > =============================================================
> > --- 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 <mjevans1983@gmail.com>
> > +
> >     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,25 @@ 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);
> > +/* FIXME : Should these 4 lines instead go in to include/linux/raid/md_k.h ?
>
> No.  No-one outside this file uses them, so they are fine where they
> are.
>
> > */
> > +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(&node_detected_dev->list, &all_detected_devices);
>
> Probably list_add_tail would be better so the ordering is the same as
> it was before?
>
>
> > +     } else {
> > +             printk(KERN_CRIT "md: kzAlloc node failed, skipping device."
> > +                              " : 0x%p.\n", node_detected_dev);
> > +     }
>
>
>
> >  }
> >
> >
> > @@ -5765,7 +5779,13 @@ 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, i_loops;
> > +     signed int i_found;
> > +     i_scanned = 0;
> > +     i_passed = 0;
> > +     i_loops = 0;
>
> i_passed is never used.
>
> And what is the point of i_loops (and i_scanned)?  The comments
> at the top of the patch should explain this if there is a good reason.
>
> >
> >       printk(KERN_INFO "md: Autodetecting RAID arrays.\n");
> >
> > @@ -5772,3 +5792,11 @@ 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   0x7FFFFFFF ? */
> > +     while (!list_empty(&all_detected_devices) && i_loops < 0x7FFFFFFF) {
> > +             i_scanned++;
> > +             node_detected_dev = NULL;
> > +             node_detected_dev = list_entry(all_detected_devices.next,
> > +                                     struct detected_devices_node, list);
> > +             if (node_detected_dev) {
>
> list_entry will *never* return NULL.   It simply doesn't pointer
> arithmetic.  (Well, I guess it could return NULL if it was passed
> NULL, and the struct-offset were 0, but that isn't the case here).
>
> So you don't need this 'if' at all.
>
> > +                     list_del(&node_detected_dev->list);
> > +                     dev = node_detected_dev->dev;
> > +                     kfree(node_detected_dev);
> > +             /* Indent is now off by one, but the old code is after this. */
>
> so you don't need to worrying about indents.
>
>
> > @@ -5781,8 +5809,16 @@ static void autostart_arrays(int part)
> >                       continue;
> >               }
> >               list_add(&rdev->same_set, &pending_raid_disks);
> > +             /* Indent is now off by one, but the old code is above this. */
> > +
> > +             } else {
> > +                     printk(KERN_CRIT "md: Invalid list node, skipping.\n");
> > +             }
> > +             i_loops++;
> >       }
> > -     dev_cnt = 0;
> > +
> > +     printk(KERN_INFO "md: Passes %d : Scanned %d and added %d devices.\n",
> > +                                             i_loops, i_scanned, i_passed);
> >
> >       autorun_devices(part);
> >  }
>
>
> I'm not dead-set against the change, and if you tidy it up I'll
> probably accept it, but I really think you would be better off skipping
> the in-kernel autodetect stuff altogether and use mdadm to assemble
> your arrays.
>
> NeilBrown
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [patch v2 1/1] md: Software Raid autodetect dev list not array
  2007-08-24  3:37 ` [patch 1/1] md: Software Raid autodetect dev list not array Neil Brown
  2007-08-24  5:50   ` Michael Evans
@ 2007-08-26 11:51   ` Michael J. Evans
  2007-08-26 12:20     ` Michael Evans
                       ` (2 more replies)
  1 sibling, 3 replies; 28+ messages in thread
From: Michael J. Evans @ 2007-08-26 11:51 UTC (permalink / raw)
  To: Neil Brown; +Cc: Ingo Molnar, linux-raid, linux-kernel, Michael J . Evans

From: Michael J. Evans <mjevans1983@gmail.com>

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 <mjevans1983@gmail.com>
--- 
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:	mingo@redhat.com
P:	Neil Brown
M:	neilb@suse.de
L:	linux-raid@vger.kernel.org
S:	Supported
Unless you have a reason NOT to do so, CC linux-kernel@vger.kernel.org.

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 <mjevans1983@gmail.com>
=============================================================
--- 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 <mjevans1983@gmail.com>
+
    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   0x7FFFFFFF ? */
+	while (!list_empty(&all_detected_devices) && i_scanned < 0x7FFFFFFF) {
+		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",
+						i_scanned, i_passed);
 
 	autorun_devices(part);
 }

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch v2 1/1] md: Software Raid autodetect dev list not array
  2007-08-26 11:51   ` [patch v2 " Michael J. Evans
@ 2007-08-26 12:20     ` Michael Evans
  2007-08-27  3:41       ` Kyle Moffett
  2007-08-26 12:56     ` Jan Engelhardt
  2007-08-26 16:56     ` Randy Dunlap
  2 siblings, 1 reply; 28+ messages in thread
From: Michael Evans @ 2007-08-26 12:20 UTC (permalink / raw)
  To: Michael J. Evans; +Cc: Neil Brown, Ingo Molnar, linux-raid, linux-kernel

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?

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch v2 1/1] md: Software Raid autodetect dev list not array
  2007-08-26 11:51   ` [patch v2 " Michael J. Evans
  2007-08-26 12:20     ` Michael Evans
@ 2007-08-26 12:56     ` Jan Engelhardt
  2007-08-26 15:58       ` Michael Evans
  2007-08-26 16:56     ` Randy Dunlap
  2 siblings, 1 reply; 28+ messages in thread
From: Jan Engelhardt @ 2007-08-26 12:56 UTC (permalink / raw)
  To: Michael J. Evans
  Cc: Neil Brown, Ingo Molnar, linux-raid, linux-kernel, Michael J . Evans


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   0x7FFFFFFF ? */
>+	while (!list_empty(&all_detected_devices) && i_scanned < 0x7FFFFFFF) {

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
0xFFFFFFFE devs (which is even less likely than 0x7FFFFFFF)

>+		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
-- 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch v2 1/1] md: Software Raid autodetect dev list not array
  2007-08-26 12:56     ` Jan Engelhardt
@ 2007-08-26 15:58       ` Michael Evans
  0 siblings, 0 replies; 28+ messages in thread
From: Michael Evans @ 2007-08-26 15:58 UTC (permalink / raw)
  To: Jan Engelhardt
  Cc: Michael J. Evans, Neil Brown, Ingo Molnar, linux-raid, linux-kernel

On 8/26/07, Jan Engelhardt <jengelh@computergmbh.de> 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   0x7FFFFFFF ? */
> >+      while (!list_empty(&all_detected_devices) && i_scanned < 0x7FFFFFFF) {
>
> 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
> 0xFFFFFFFE devs (which is even less likely than 0x7FFFFFFF)
>

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
> --
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch v2 1/1] md: Software Raid autodetect dev list not array
  2007-08-26 11:51   ` [patch v2 " Michael J. Evans
  2007-08-26 12:20     ` Michael Evans
  2007-08-26 12:56     ` Jan Engelhardt
@ 2007-08-26 16:56     ` Randy Dunlap
  2007-08-26 19:18       ` Michael Evans
  2 siblings, 1 reply; 28+ messages in thread
From: Randy Dunlap @ 2007-08-26 16:56 UTC (permalink / raw)
  To: Michael J. Evans
  Cc: Neil Brown, Ingo Molnar, linux-raid, linux-kernel, Michael J . Evans

On Sun, 26 Aug 2007 04:51:24 -0700 Michael J. Evans wrote:

> From: Michael J. Evans <mjevans1983@gmail.com>
> 
> 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 <mjevans1983@gmail.com>
> --- 
> 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:	mingo@redhat.com
> P:	Neil Brown
> M:	neilb@suse.de
> L:	linux-raid@vger.kernel.org
> S:	Supported
> Unless you have a reason NOT to do so, CC linux-kernel@vger.kernel.org.
> 
> 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 <mjevans1983@gmail.com>
> =============================================================
> --- 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
> @@ -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   0x7FFFFFFF ? */

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 < 0x7FFFFFFF) {
> +		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 ***

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch v2 1/1] md: Software Raid autodetect dev list not array
  2007-08-26 16:56     ` Randy Dunlap
@ 2007-08-26 19:18       ` Michael Evans
  2007-08-27 22:16         ` [patch v3 " Michael J. Evans
  0 siblings, 1 reply; 28+ messages in thread
From: Michael Evans @ 2007-08-26 19:18 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Michael J. Evans, Neil Brown, Ingo Molnar, linux-raid, linux-kernel

On 8/26/07, Randy Dunlap <randy.dunlap@oracle.com> wrote:
> On Sun, 26 Aug 2007 04:51:24 -0700 Michael J. Evans wrote:
>
> > From: Michael J. Evans <mjevans1983@gmail.com>
> >

>
> 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 ***
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch v2 1/1] md: Software Raid autodetect dev list not array
  2007-08-26 12:20     ` Michael Evans
@ 2007-08-27  3:41       ` Kyle Moffett
  2007-08-27  7:56         ` Michael Evans
  0 siblings, 1 reply; 28+ messages in thread
From: Kyle Moffett @ 2007-08-27  3:41 UTC (permalink / raw)
  To: Michael Evans
  Cc: Michael J. Evans, Neil Brown, Ingo Molnar, linux-raid, linux-kernel

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


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch v2 1/1] md: Software Raid autodetect dev list not array
  2007-08-27  3:41       ` Kyle Moffett
@ 2007-08-27  7:56         ` Michael Evans
  0 siblings, 0 replies; 28+ messages in thread
From: Michael Evans @ 2007-08-27  7:56 UTC (permalink / raw)
  To: Kyle Moffett
  Cc: Michael J. Evans, Neil Brown, Ingo Molnar, linux-raid, linux-kernel

On 8/26/07, Kyle Moffett <mrmacman_g4@mac.com> 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.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [patch v3 1/1] md: Software Raid autodetect dev list not array
  2007-08-26 19:18       ` Michael Evans
@ 2007-08-27 22:16         ` Michael J. Evans
  2007-08-27 22:30           ` Randy Dunlap
  0 siblings, 1 reply; 28+ messages in thread
From: Michael J. Evans @ 2007-08-27 22:16 UTC (permalink / raw)
  To: Michael Evans
  Cc: Randy Dunlap, Neil Brown, Ingo Molnar, linux-raid, linux-kernel

From: Michael J. Evans <mjevans1983@gmail.com>

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 <mjevans1983@gmail.com>
--- 
Sorry, it looks like I hit reply instead of reply to all yesterday.

Version 3:
md_autodetect_dev failure message is now more usefully verbose.
removed unused i_found that was leftover from initial verification.
Thank you Randy Dunlap for pointing where INT_MAX was, fixme fixed.

Version 2:
using list_add_tail, and corrected missing i_passed++;.
removed sections of code that would never be reached.

Version 1:
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:	mingo@redhat.com
P:	Neil Brown
M:	neilb@suse.de
L:	linux-raid@vger.kernel.org
S:	Supported
Unless you have a reason NOT to do so, CC linux-kernel@vger.kernel.org.

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 <mjevans1983@gmail.com>
=============================================================
--- 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 <mjevans1983@gmail.com>
+
    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,26 @@ 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;
+	char strbuf[BDEVNAME_SIZE];
+
+	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: md_autodetect_dev: kzAlloc node failed"
+		" (null return), skipping dev(%d,%d)\n", MAJOR(dev), MINOR(dev));
+	}
 }
 
 
@@ -5765,7 +5760,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;
+
+	i_scanned = 0;
+	i_passed = 0;
 
 	printk(KERN_INFO "md: Autodetecting RAID arrays.\n");
 
@@ -5772,3 +5792,7 @@ static void autostart_arrays(int part)
-	for (i = 0; i < dev_cnt; i++) {
-		dev_t dev = detected_devices[i];
-
+	while (!list_empty(&all_detected_devices) && i_scanned < INT_MAX) {
+		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 +5807,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",
+						i_scanned, i_passed);
 
 	autorun_devices(part);
 }

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch v3 1/1] md: Software Raid autodetect dev list not array
  2007-08-27 22:16         ` [patch v3 " Michael J. Evans
@ 2007-08-27 22:30           ` Randy Dunlap
  2007-08-28  4:38             ` Michael J. Evans
  2007-08-28  4:39             ` [patch v4 " Michael J. Evans
  0 siblings, 2 replies; 28+ messages in thread
From: Randy Dunlap @ 2007-08-27 22:30 UTC (permalink / raw)
  To: Michael J. Evans
  Cc: Michael Evans, Neil Brown, Ingo Molnar, linux-raid, linux-kernel

On Mon, 27 Aug 2007 15:16:21 -0700 Michael J. Evans wrote:

> 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)

so you should update the patch to the latest mainline.

It's up to the (RAID) maintainer(s) if they want to merge a patch with fuzz.
Andrew may fix it up.  Linus wouldn't accept it with fuzz.

> Signed-off-by: Michael J. Evans <mjevans1983@gmail.com>
> =============================================================
> --- 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 <mjevans1983@gmail.com>
> +

Nowadays we use an SCM for such comments, not the source file(s).

>     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,26 @@ 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;
> +	char strbuf[BDEVNAME_SIZE];
> +
> +	node_detected_dev = kzalloc(sizeof(*node_detected_dev), GFP_KERNEL);\

Drop the trailing '\', as someone has already commented on.

> +	if (node_detected_dev) {
> +		node_detected_dev->dev = dev;
> +		list_add_tail(&node_detected_dev->list, &all_detected_devices);
> +	} else {
> +		printk(KERN_CRIT "md: md_autodetect_dev: kzAlloc node failed"
> +		" (null return), skipping dev(%d,%d)\n", MAJOR(dev), MINOR(dev));

printk() formatting is bad.
Drop the " (null return)" and indent that line more than the
printk line is indented.

> +	}
>  }
>  
>  

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch v3 1/1] md: Software Raid autodetect dev list not array
  2007-08-27 22:30           ` Randy Dunlap
@ 2007-08-28  4:38             ` Michael J. Evans
  2007-08-28  4:46               ` Randy Dunlap
  2007-08-28  4:39             ` [patch v4 " Michael J. Evans
  1 sibling, 1 reply; 28+ messages in thread
From: Michael J. Evans @ 2007-08-28  4:38 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Michael Evans, Neil Brown, Ingo Molnar, linux-raid, linux-kernel

On Monday 27 August 2007, Randy Dunlap wrote:
> On Mon, 27 Aug 2007 15:16:21 -0700 Michael J. Evans wrote:
> 
> > =============================================================
> > --- 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 
<mjevans1983@gmail.com>
> > +
> 
> Nowadays we use an SCM for such comments, not the source file(s).

SCM?  Is this automatic, where do I go to see it?

> 
> >     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,26 @@ 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;
> > +	char strbuf[BDEVNAME_SIZE];
> > +
> > +	node_detected_dev = kzalloc(sizeof(*node_detected_dev), GFP_KERNEL);\
> 
> Drop the trailing '\', as someone has already commented on.
> 

I thought I had fixed that, I must have copied the unfixed version out of the 
way when I made the other changes to it.

> > +	if (node_detected_dev) {
> > +		node_detected_dev->dev = dev;
> > +		list_add_tail(&node_detected_dev->list, &all_detected_devices);
> > +	} else {
> > +		printk(KERN_CRIT "md: md_autodetect_dev: kzAlloc node failed"
> > +		" (null return), skipping dev(%d,%d)\n", MAJOR(dev), MINOR(dev));
> 
> printk() formatting is bad.
> Drop the " (null return)" and indent that line more than the
> printk line is indented.
> 
> > +	}
> >  }
> >  
> >  
> 
> ---
> ~Randy
> *** Remember to use Documentation/SubmitChecklist when testing your code ***
> 



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch v4 1/1] md: Software Raid autodetect dev list not array
  2007-08-27 22:30           ` Randy Dunlap
  2007-08-28  4:38             ` Michael J. Evans
@ 2007-08-28  4:39             ` Michael J. Evans
  1 sibling, 0 replies; 28+ messages in thread
From: Michael J. Evans @ 2007-08-28  4:39 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Michael Evans, Neil Brown, Ingo Molnar, linux-raid, linux-kernel

From: Michael J. Evans <mjevans1983@gmail.com>

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 <mjevans1983@gmail.com>
--- 
Version 4:
Minor formatting cleanup for kzAlloc error message.
Should I remove the first patch section in version 5?

Version 3:
md_autodetect_dev failure message is now more usefully verbose.
removed unused i_found that was leftover from initial verification.
Thank you Randy Dunlap for pointing where INT_MAX was, fixme fixed.

Version 2:
using list_add_tail, and corrected missing i_passed++;.
removed sections of code that would never be reached.

Version 1:
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:	mingo@redhat.com
P:	Neil Brown
M:	neilb@suse.de
L:	linux-raid@vger.kernel.org
S:	Supported
Unless you have a reason NOT to do so, CC linux-kernel@vger.kernel.org.

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.

Signed-off-by: Michael J. Evans <mjevans1983@gmail.com>
=============================================================
--- 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 <mjevans1983@gmail.com>
+
    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,26 @@ 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;
+	char strbuf[BDEVNAME_SIZE];
+
+	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: md_autodetect_dev: kzAlloc node failed"
+			", skipping dev(%d,%d)\n", MAJOR(dev), MINOR(dev));
+	}
 }
 
 
@@ -5765,7 +5760,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;
+
+	i_scanned = 0;
+	i_passed = 0;
 
 	printk(KERN_INFO "md: Autodetecting RAID arrays.\n");
 
@@ -5772,3 +5792,7 @@ static void autostart_arrays(int part)
-	for (i = 0; i < dev_cnt; i++) {
-		dev_t dev = detected_devices[i];
-
+	while (!list_empty(&all_detected_devices) && i_scanned < INT_MAX) {
+		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 +5807,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",
+						i_scanned, i_passed);
 
 	autorun_devices(part);
 }

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch v3 1/1] md: Software Raid autodetect dev list not array
  2007-08-28  4:38             ` Michael J. Evans
@ 2007-08-28  4:46               ` Randy Dunlap
  2007-08-28 13:08                 ` Michael Evans
  2007-08-28 13:27                 ` Michael J. Evans
  0 siblings, 2 replies; 28+ messages in thread
From: Randy Dunlap @ 2007-08-28  4:46 UTC (permalink / raw)
  To: Michael J. Evans
  Cc: Michael Evans, Neil Brown, Ingo Molnar, linux-raid, linux-kernel

Michael J. Evans wrote:
> On Monday 27 August 2007, Randy Dunlap wrote:
>> On Mon, 27 Aug 2007 15:16:21 -0700 Michael J. Evans wrote:
>>
>>> =============================================================
>>> --- 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 
> <mjevans1983@gmail.com>
>>> +
>> Nowadays we use an SCM for such comments, not the source file(s).
> 
> SCM?  Is this automatic, where do I go to see it?

See Documentation/SubmittingPatches:
14) The canonical patch format:

The canonical patch message body contains the following:

  - A "from" line specifying the patch author.

  - An empty line.

  - The body of the explanation, which will be copied to the
    permanent changelog to describe this patch.

  - The "Signed-off-by:" lines, described above, which will
    also go in the changelog.

  - A marker line containing simply "---".

  - Any additional comments not suitable for the changelog.

  - The actual patch (diff output).


so just put whatever you want to be in the permanent SCM logs
into the "body of the explanation" part of the email.


-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch v3 1/1] md: Software Raid autodetect dev list not array
  2007-08-28  4:46               ` Randy Dunlap
@ 2007-08-28 13:08                 ` Michael Evans
  2007-08-28 13:14                   ` Jan Engelhardt
  2007-08-28 13:24                   ` Bill Davidsen
  2007-08-28 13:27                 ` Michael J. Evans
  1 sibling, 2 replies; 28+ messages in thread
From: Michael Evans @ 2007-08-28 13:08 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Michael J. Evans, Neil Brown, Ingo Molnar, linux-raid, linux-kernel

On 8/27/07, Randy Dunlap <randy.dunlap@oracle.com> wrote:
> Michael J. Evans wrote:
> > On Monday 27 August 2007, Randy Dunlap wrote:
> >> On Mon, 27 Aug 2007 15:16:21 -0700 Michael J. Evans wrote:
> >>
> >>> =============================================================
> >>> --- 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
> > <mjevans1983@gmail.com>
> >>> +
> >> Nowadays we use an SCM for such comments, not the source file(s).
> >
> > SCM?  Is this automatic, where do I go to see it?
>
> See Documentation/SubmittingPatches:
> 14) The canonical patch format:
>
> The canonical patch message body contains the following:
>
>   - A "from" line specifying the patch author.
>
>   - An empty line.
>
>   - The body of the explanation, which will be copied to the
>     permanent changelog to describe this patch.
>
>   - The "Signed-off-by:" lines, described above, which will
>     also go in the changelog.
>
>   - A marker line containing simply "---".
>
>   - Any additional comments not suitable for the changelog.
>
>   - The actual patch (diff output).
>
>
> so just put whatever you want to be in the permanent SCM logs
> into the "body of the explanation" part of the email.
>
>
> --
> ~Randy
> *** Remember to use Documentation/SubmitChecklist when testing your code ***
>

Oh, I see.  I forgot about the changelogs.  I'd send out version 5
now, but I'm not sure what kernel version to make the patch against.
2.6.23-rc4 is on kernel.org and I don't see any git snapshots.
Additionally I never could tell what git tree was the 'mainline' as it
isn't labeled with such a keyword (at least in the list of git trees I
saw).

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch v3 1/1] md: Software Raid autodetect dev list not array
  2007-08-28 13:08                 ` Michael Evans
@ 2007-08-28 13:14                   ` Jan Engelhardt
  2007-08-28 13:26                     ` Michael J. Evans
  2007-08-28 13:24                   ` Bill Davidsen
  1 sibling, 1 reply; 28+ messages in thread
From: Jan Engelhardt @ 2007-08-28 13:14 UTC (permalink / raw)
  To: Michael Evans
  Cc: Randy Dunlap, Michael J. Evans, Neil Brown, Ingo Molnar,
	linux-raid, linux-kernel


On Aug 28 2007 06:08, Michael Evans wrote:
>
>Oh, I see.  I forgot about the changelogs.  I'd send out version 5
>now, but I'm not sure what kernel version to make the patch against.
>2.6.23-rc4 is on kernel.org and I don't see any git snapshots.

2.6.23-rc4 is a snapshot in itself, a tagged one at that.
Just use git pull to get the latest, which is always good.
Or git fetch; git checkout 2.6.23-rc4; if you need that particular one.

>Additionally I never could tell what git tree was the 'mainline' as it
>isn't labeled with such a keyword (at least in the list of git trees I
>saw).

/torvalds/linux-2.6.git or so; yes, it's not clearly marked. Then again, why?
/Mainline is tarballs for most people, and they don't want to go deeper ;-)


	Jan
-- 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch v3 1/1] md: Software Raid autodetect dev list not array
  2007-08-28 13:08                 ` Michael Evans
  2007-08-28 13:14                   ` Jan Engelhardt
@ 2007-08-28 13:24                   ` Bill Davidsen
  2007-08-28 13:32                     ` Michael Evans
  1 sibling, 1 reply; 28+ messages in thread
From: Bill Davidsen @ 2007-08-28 13:24 UTC (permalink / raw)
  To: Michael Evans
  Cc: Randy Dunlap, Michael J. Evans, Neil Brown, Ingo Molnar,
	linux-raid, linux-kernel

Michael Evans wrote:
> Oh, I see.  I forgot about the changelogs.  I'd send out version 5
> now, but I'm not sure what kernel version to make the patch against.
> 2.6.23-rc4 is on kernel.org and I don't see any git snapshots.
> Additionally I never could tell what git tree was the 'mainline' as it
> isn't labeled with such a keyword (at least in the list of git trees I
> saw).

I suspect you wait for 2.5.23 release, or send it to AKPM for inclusion 
in an "-mm" kernel. That's probably desirable, anyway.

-- 
bill davidsen <davidsen@tmr.com>
  CTO TMR Associates, Inc
  Doing interesting things with small computers since 1979


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch v3 1/1] md: Software Raid autodetect dev list not array
  2007-08-28 13:14                   ` Jan Engelhardt
@ 2007-08-28 13:26                     ` Michael J. Evans
  0 siblings, 0 replies; 28+ messages in thread
From: Michael J. Evans @ 2007-08-28 13:26 UTC (permalink / raw)
  To: Jan Engelhardt
  Cc: Michael Evans, Randy Dunlap, Neil Brown, Ingo Molnar, linux-raid,
	linux-kernel

On Tuesday 28 August 2007, Jan Engelhardt wrote:
> 
> On Aug 28 2007 06:08, Michael Evans wrote:
> >
> >Oh, I see.  I forgot about the changelogs.  I'd send out version 5
> >now, but I'm not sure what kernel version to make the patch against.
> >2.6.23-rc4 is on kernel.org and I don't see any git snapshots.
> 
> 2.6.23-rc4 is a snapshot in itself, a tagged one at that.
> Just use git pull to get the latest, which is always good.
> Or git fetch; git checkout 2.6.23-rc4; if you need that particular one.
> 
> >Additionally I never could tell what git tree was the 'mainline' as it
> >isn't labeled with such a keyword (at least in the list of git trees I
> >saw).
> 
> /torvalds/linux-2.6.git or so; yes, it's not clearly marked. Then again, 
why?
> /Mainline is tarballs for most people, and they don't want to go deeper ;-)
> 
> 
> 	Jan
> -- 
> 

Thank you, I'll try to keep that in mind the next time I try to use git.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch v5 1/1] md: Software Raid autodetect dev list not array
  2007-08-28  4:46               ` Randy Dunlap
  2007-08-28 13:08                 ` Michael Evans
@ 2007-08-28 13:27                 ` Michael J. Evans
  1 sibling, 0 replies; 28+ messages in thread
From: Michael J. Evans @ 2007-08-28 13:27 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Michael Evans, Neil Brown, Ingo Molnar, linux-raid, linux-kernel

From: Michael J. Evans <mjevans1983@gmail.com>

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 <mjevans1983@gmail.com>
--- 
Version 5:
patched modified to cleanly patch against 2.6.23-rc4 (Offsets only)
Removed credit from head of file since the changelogs are archived.

Version 4:
Minor formatting cleanup for kzAlloc error message.
Should I remove the first patch section in version 5?

Version 3:
md_autodetect_dev failure message is now more usefully verbose.
removed unused i_found that was leftover from initial verification.
Thank you Randy Dunlap for pointing where INT_MAX was, fixme fixed.

Version 2:
using list_add_tail, and corrected missing i_passed++;.
removed sections of code that would never be reached.

Version 1:
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:	mingo@redhat.com
P:	Neil Brown
M:	neilb@suse.de
L:	linux-raid@vger.kernel.org
S:	Supported
Unless you have a reason NOT to do so, CC linux-kernel@vger.kernel.org.

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.

Signed-off-by: Michael J. Evans <mjevans1983@gmail.com>
=============================================================
--- 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
@@ -5781,13 +5781,26 @@ 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;
+	char strbuf[BDEVNAME_SIZE];
+
+	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: md_autodetect_dev: kzAlloc node failed"
+			", skipping dev(%d,%d)\n", MAJOR(dev), MINOR(dev));
+	}
 }
 
 
@@ -5794,7 +5787,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;
+
+	i_scanned = 0;
+	i_passed = 0;
 
 	printk(KERN_INFO "md: Autodetecting RAID arrays.\n");
 
@@ -5801,3 +5821,7 @@ static void autostart_arrays(int part)
-	for (i = 0; i < dev_cnt; i++) {
-		dev_t dev = detected_devices[i];
-
+	while (!list_empty(&all_detected_devices) && i_scanned < INT_MAX) {
+		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);
@@ -5810,8 +5834,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",
+						i_scanned, i_passed);
 
 	autorun_devices(part);
 }

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch v3 1/1] md: Software Raid autodetect dev list not array
  2007-08-28 13:24                   ` Bill Davidsen
@ 2007-08-28 13:32                     ` Michael Evans
  2007-08-28 16:15                       ` Randy Dunlap
  0 siblings, 1 reply; 28+ messages in thread
From: Michael Evans @ 2007-08-28 13:32 UTC (permalink / raw)
  To: Bill Davidsen
  Cc: Randy Dunlap, Michael J. Evans, Neil Brown, Ingo Molnar,
	linux-raid, linux-kernel

On 8/28/07, Bill Davidsen <davidsen@tmr.com> wrote:
> Michael Evans wrote:
> > Oh, I see.  I forgot about the changelogs.  I'd send out version 5
> > now, but I'm not sure what kernel version to make the patch against.
> > 2.6.23-rc4 is on kernel.org and I don't see any git snapshots.
> > Additionally I never could tell what git tree was the 'mainline' as it
> > isn't labeled with such a keyword (at least in the list of git trees I
> > saw).
>
> I suspect you wait for 2.5.23 release, or send it to AKPM for inclusion
> in an "-mm" kernel. That's probably desirable, anyway.
>
> --
> bill davidsen <davidsen@tmr.com>
>   CTO TMR Associates, Inc
>   Doing interesting things with small computers since 1979
>
>

There's another list I should CC to?  Or does the section maintainer
do that when they're happy with the patch?

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch v3 1/1] md: Software Raid autodetect dev list not array
  2007-08-28 13:32                     ` Michael Evans
@ 2007-08-28 16:15                       ` Randy Dunlap
  2007-08-28 17:10                         ` Michael Evans
  0 siblings, 1 reply; 28+ messages in thread
From: Randy Dunlap @ 2007-08-28 16:15 UTC (permalink / raw)
  To: Michael Evans
  Cc: Bill Davidsen, Michael J. Evans, Neil Brown, Ingo Molnar,
	linux-raid, linux-kernel

Michael Evans wrote:
> On 8/28/07, Bill Davidsen <davidsen@tmr.com> wrote:
>> Michael Evans wrote:
>>> Oh, I see.  I forgot about the changelogs.  I'd send out version 5
>>> now, but I'm not sure what kernel version to make the patch against.
>>> 2.6.23-rc4 is on kernel.org and I don't see any git snapshots.
>>> Additionally I never could tell what git tree was the 'mainline' as it
>>> isn't labeled with such a keyword (at least in the list of git trees I
>>> saw).
>> 
>> I suspect you wait for 2.5.23 release, or send it to AKPM for inclusion
>> in an "-mm" kernel. That's probably desirable, anyway.
>>
>> --
> 
> There's another list I should CC to?  Or does the section maintainer
> do that when they're happy with the patch?

Not another list; just cc: akpm@linux-foundation.org so that he can merge
it into the -mm kernel patches for testing.  Most patches cook in the -mm
kernel(s) before they are merged into mainline.

Then generally the subsystem maintainer(s) are responsible for sending
patches on to Linus for merging into mainline, if/when they are happy
with the patch and they think that it has been tested enough.

Mainline progresses (roughly, e.g.) like so:

2.6.22
then patches are applied and we get (possibly) daily snapshots from git, like
2.6.22-git1, 2.6.22-git2, ...

After the primary 2-week merge window for new code (i.e., not just bug fixes),
we get 2.6.23-rc1.
Then as more patches are merged, we get 2.6.23-rc1-gitN...

Then 2.6.23-rc2 ... 2.6.23-rc2-gitN...

2.6.23-rc3 ... 2.6.23-rc3-gitN...

blah blah blah

You can use git to download and track the mainline kernel changes,
or you can download tarballs + patches (patches for each -rc as well as
for each -gitN snapshot) and apply the patches yourself or use a
script to do it. (like 'ketchup': http://www.selenic.com/ketchup/wiki/;
or like http://www.xenotime.net/linux/scripts/grab_kernel)

In general, patches should be made against latest/recent kernels, either
current git (cloned or updated via pull), or against recent patches
+ git snapshot if there is a git snapshot that is applicable.
Immediately after 2.6.23-rcN, there is no applicable git snapshot to
be applied (well, a few hours later there may be).

Sometimes it is appropriate to make patches against the -mm kernel
patchset, but only if the patch is for code that is only in that
kernel patchset (or if Andrew asks for a patch to be updated against
-mm).  [This is a mostly rough generalization.]  Andrew normally
takes patches against mainline and coerces them into -mm if coercion
is needed.

-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch v3 1/1] md: Software Raid autodetect dev list not array
  2007-08-28 16:15                       ` Randy Dunlap
@ 2007-08-28 17:10                         ` Michael Evans
  2007-08-28 17:22                           ` Randy Dunlap
  0 siblings, 1 reply; 28+ messages in thread
From: Michael Evans @ 2007-08-28 17:10 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Bill Davidsen, Michael J. Evans, Neil Brown, Ingo Molnar,
	linux-raid, linux-kernel

On 8/28/07, Randy Dunlap <randy.dunlap@oracle.com> wrote:
> Michael Evans wrote:
> > On 8/28/07, Bill Davidsen <davidsen@tmr.com> wrote:
> >> Michael Evans wrote:
> >>> Oh, I see.  I forgot about the changelogs.  I'd send out version 5
> >>> now, but I'm not sure what kernel version to make the patch against.
> >>> 2.6.23-rc4 is on kernel.org and I don't see any git snapshots.
> >>> Additionally I never could tell what git tree was the 'mainline' as it
> >>> isn't labeled with such a keyword (at least in the list of git trees I
> >>> saw).
> >>
> >> I suspect you wait for 2.5.23 release, or send it to AKPM for inclusion
> >> in an "-mm" kernel. That's probably desirable, anyway.
> >>
> >> --
> >
> > There's another list I should CC to?  Or does the section maintainer
> > do that when they're happy with the patch?
>
> Not another list; just cc: akpm@linux-foundation.org so that he can merge
> it into the -mm kernel patches for testing.  Most patches cook in the -mm
> kernel(s) before they are merged into mainline.
>
> Then generally the subsystem maintainer(s) are responsible for sending
> patches on to Linus for merging into mainline, if/when they are happy
> with the patch and they think that it has been tested enough.
>

So I should look at the git documentation again, try to pull down
Andrew's latest -mm, and see what I need to change (if anything) to
patch against it?  (I would probably only verify that boots once my
self given the other testing this has already seen and how often mm
breaks things my mythtv box likes, such as nvidia-drivers, etc,)

> Mainline progresses (roughly, e.g.) like so:
>

I knew about the progression order (but not timing) of the non-git
side of it, but it was helpful to have the whole thing spelled out,
thank you.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch v3 1/1] md: Software Raid autodetect dev list not array
  2007-08-28 17:10                         ` Michael Evans
@ 2007-08-28 17:22                           ` Randy Dunlap
  2007-08-28 19:11                             ` Michael Evans
  0 siblings, 1 reply; 28+ messages in thread
From: Randy Dunlap @ 2007-08-28 17:22 UTC (permalink / raw)
  To: Michael Evans
  Cc: Bill Davidsen, Michael J. Evans, Neil Brown, Ingo Molnar,
	linux-raid, linux-kernel

Michael Evans wrote:
> On 8/28/07, Randy Dunlap <randy.dunlap@oracle.com> wrote:
>> Michael Evans wrote:
>>> On 8/28/07, Bill Davidsen <davidsen@tmr.com> wrote:
>>>> Michael Evans wrote:
>>>>> Oh, I see.  I forgot about the changelogs.  I'd send out version 5
>>>>> now, but I'm not sure what kernel version to make the patch against.
>>>>> 2.6.23-rc4 is on kernel.org and I don't see any git snapshots.
>>>>> Additionally I never could tell what git tree was the 'mainline' as it
>>>>> isn't labeled with such a keyword (at least in the list of git trees I
>>>>> saw).
>>>> I suspect you wait for 2.5.23 release, or send it to AKPM for inclusion
>>>> in an "-mm" kernel. That's probably desirable, anyway.
>>>>
>>>> --
>>> There's another list I should CC to?  Or does the section maintainer
>>> do that when they're happy with the patch?
>> Not another list; just cc: akpm@linux-foundation.org so that he can merge
>> it into the -mm kernel patches for testing.  Most patches cook in the -mm
>> kernel(s) before they are merged into mainline.
>>
>> Then generally the subsystem maintainer(s) are responsible for sending
>> patches on to Linus for merging into mainline, if/when they are happy
>> with the patch and they think that it has been tested enough.
>>
> 
> So I should look at the git documentation again, try to pull down
> Andrew's latest -mm, and see what I need to change (if anything) to
> patch against it?  (I would probably only verify that boots once my
> self given the other testing this has already seen and how often mm
> breaks things my mythtv box likes, such as nvidia-drivers, etc,)

-mm is available only as a patchset, not via git.
There has been a git version of it, but it's not currently working and
hasn't been for 2 months or so.

I mostly use the grab_kernel script that I mentioned.  It knows how
to download linux-2.6.M and how to apply -rcN, -gitN, and/or -mmN
patches to the base (linux-2.6.M).

E.g.:
grab_kernel 2.6.23-rc3-git6 $PWD
or
grab_kernel 2.6.23-rc3-mm1 $PWD


-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch v3 1/1] md: Software Raid autodetect dev list not array
  2007-08-28 17:22                           ` Randy Dunlap
@ 2007-08-28 19:11                             ` Michael Evans
  2007-08-28 19:16                               ` Randy Dunlap
  0 siblings, 1 reply; 28+ messages in thread
From: Michael Evans @ 2007-08-28 19:11 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Bill Davidsen, Michael J. Evans, Neil Brown, Ingo Molnar,
	linux-raid, linux-kernel

On 8/28/07, Randy Dunlap <randy.dunlap@oracle.com> wrote:
> Michael Evans wrote:
> > On 8/28/07, Randy Dunlap <randy.dunlap@oracle.com> wrote:
> >> Michael Evans wrote:
> >>> On 8/28/07, Bill Davidsen <davidsen@tmr.com> wrote:
> >>>> Michael Evans wrote:
> >>>>> Oh, I see.  I forgot about the changelogs.  I'd send out version 5
> >>>>> now, but I'm not sure what kernel version to make the patch against.
> >>>>> 2.6.23-rc4 is on kernel.org and I don't see any git snapshots.
> >>>>> Additionally I never could tell what git tree was the 'mainline' as it
> >>>>> isn't labeled with such a keyword (at least in the list of git trees I
> >>>>> saw).
> >>>> I suspect you wait for 2.5.23 release, or send it to AKPM for inclusion
> >>>> in an "-mm" kernel. That's probably desirable, anyway.
> >>>>
> >>>> --
> >>> There's another list I should CC to?  Or does the section maintainer
> >>> do that when they're happy with the patch?
> >> Not another list; just cc: akpm@linux-foundation.org so that he can merge
> >> it into the -mm kernel patches for testing.  Most patches cook in the -mm
> >> kernel(s) before they are merged into mainline.
> >>
> >> Then generally the subsystem maintainer(s) are responsible for sending
> >> patches on to Linus for merging into mainline, if/when they are happy
> >> with the patch and they think that it has been tested enough.
> >>
> >
> > So I should look at the git documentation again, try to pull down
> > Andrew's latest -mm, and see what I need to change (if anything) to
> > patch against it?  (I would probably only verify that boots once my
> > self given the other testing this has already seen and how often mm
> > breaks things my mythtv box likes, such as nvidia-drivers, etc,)
>
> -mm is available only as a patchset, not via git.
> There has been a git version of it, but it's not currently working and
> hasn't been for 2 months or so.
>
> I mostly use the grab_kernel script that I mentioned.  It knows how
> to download linux-2.6.M and how to apply -rcN, -gitN, and/or -mmN
> patches to the base (linux-2.6.M).
>
> E.g.:
> grab_kernel 2.6.23-rc3-git6 $PWD
> or
> grab_kernel 2.6.23-rc3-mm1 $PWD
>
>
> --
> ~Randy
> *** Remember to use Documentation/SubmitChecklist when testing your code ***
>

Gentoo's mm-sources is version 2.6.23-rc3-mm1, which I'd guess is
close enough for an area like this.  Given that I didn't need to make
any changes to the patch should I just submit it to the mm for testing
without rebooting in to an mm-sources kernel?  I need to schedule when
I do that so it would take me overnight to do so.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch v3 1/1] md: Software Raid autodetect dev list not array
  2007-08-28 19:11                             ` Michael Evans
@ 2007-08-28 19:16                               ` Randy Dunlap
  2007-08-29  8:06                                 ` [patch v5 " Michael J. Evans
  0 siblings, 1 reply; 28+ messages in thread
From: Randy Dunlap @ 2007-08-28 19:16 UTC (permalink / raw)
  To: Michael Evans
  Cc: Bill Davidsen, Michael J. Evans, Neil Brown, Ingo Molnar,
	linux-raid, linux-kernel

Michael Evans wrote:
> On 8/28/07, Randy Dunlap <randy.dunlap@oracle.com> wrote:
>> Michael Evans wrote:
>>> On 8/28/07, Randy Dunlap <randy.dunlap@oracle.com> wrote:
>>>> Michael Evans wrote:
>>>>> On 8/28/07, Bill Davidsen <davidsen@tmr.com> wrote:
>>>>>> Michael Evans wrote:
>>>>>>> Oh, I see.  I forgot about the changelogs.  I'd send out version 5
>>>>>>> now, but I'm not sure what kernel version to make the patch against.
>>>>>>> 2.6.23-rc4 is on kernel.org and I don't see any git snapshots.
>>>>>>> Additionally I never could tell what git tree was the 'mainline' as it
>>>>>>> isn't labeled with such a keyword (at least in the list of git trees I
>>>>>>> saw).
>>>>>> I suspect you wait for 2.5.23 release, or send it to AKPM for inclusion
>>>>>> in an "-mm" kernel. That's probably desirable, anyway.
> 
> Gentoo's mm-sources is version 2.6.23-rc3-mm1, which I'd guess is
> close enough for an area like this.  Given that I didn't need to make
> any changes to the patch should I just submit it to the mm for testing
> without rebooting in to an mm-sources kernel?  I need to schedule when
> I do that so it would take me overnight to do so.

Yes, I think that you can go ahead with that, but it's up to the
subsystem maintainers where/when it goes from there.

-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [patch v5 1/1] md: Software Raid autodetect dev list not array
  2007-08-28 19:16                               ` Randy Dunlap
@ 2007-08-29  8:06                                 ` Michael J. Evans
  2007-08-29 15:18                                   ` Randy Dunlap
  0 siblings, 1 reply; 28+ messages in thread
From: Michael J. Evans @ 2007-08-29  8:06 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Michael Evans, Bill Davidsen, Neil Brown, Ingo Molnar,
	linux-raid, linux-kernel, akpm

From: Michael J. Evans <mjevans1983@gmail.com>

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 <mjevans1983@gmail.com>
--- 
CCing akpm@linux-foundation.org

Version 5:
patched modified to cleanly patch against 2.6.23-rc4 (Offsets only)
Removed credit from head of file since the changelogs are archived.

Version 4:
Minor formatting cleanup for kzAlloc error message.
Should I remove the first patch section in version 5?

Version 3:
md_autodetect_dev failure message is now more usefully verbose.
removed unused i_found that was leftover from initial verification.
Thank you Randy Dunlap for pointing where INT_MAX was, fixme fixed.

Version 2:
using list_add_tail, and corrected missing i_passed++;.
removed sections of code that would never be reached.

Version 1:
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:	mingo@redhat.com
P:	Neil Brown
M:	neilb@suse.de
L:	linux-raid@vger.kernel.org
S:	Supported
Unless you have a reason NOT to do so, CC linux-kernel@vger.kernel.org.

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.

Signed-off-by: Michael J. Evans <mjevans1983@gmail.com>
=============================================================
--- 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
@@ -5781,13 +5781,26 @@ 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;
+	char strbuf[BDEVNAME_SIZE];
+
+	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: md_autodetect_dev: kzAlloc node failed"
+			", skipping dev(%d,%d)\n", MAJOR(dev), MINOR(dev));
+	}
 }
 
 
@@ -5794,7 +5787,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;
+
+	i_scanned = 0;
+	i_passed = 0;
 
 	printk(KERN_INFO "md: Autodetecting RAID arrays.\n");
 
@@ -5801,3 +5821,7 @@ static void autostart_arrays(int part)
-	for (i = 0; i < dev_cnt; i++) {
-		dev_t dev = detected_devices[i];
-
+	while (!list_empty(&all_detected_devices) && i_scanned < INT_MAX) {
+		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);
@@ -5810,8 +5834,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",
+						i_scanned, i_passed);
 
 	autorun_devices(part);
 }

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [patch v5 1/1] md: Software Raid autodetect dev list not array
  2007-08-29  8:06                                 ` [patch v5 " Michael J. Evans
@ 2007-08-29 15:18                                   ` Randy Dunlap
  0 siblings, 0 replies; 28+ messages in thread
From: Randy Dunlap @ 2007-08-29 15:18 UTC (permalink / raw)
  To: Michael J. Evans
  Cc: Michael Evans, Bill Davidsen, Neil Brown, Ingo Molnar,
	linux-raid, linux-kernel, akpm

Michael J. Evans wrote:
> From: Michael J. Evans <mjevans1983@gmail.com>
> 
> 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 <mjevans1983@gmail.com>
> --- 

So only the text above goes into the permanent changelog (if it's
used as is).  That text tells what the patch does but not why it
does it.  Telling why is more important.  Some of the text below
should be moved up to the changelog area, possibly with some other
editing for readability (see BEGIN/END below).

> CCing akpm@linux-foundation.org
> 
> Version 5:
> patched modified to cleanly patch against 2.6.23-rc4 (Offsets only)
> Removed credit from head of file since the changelogs are archived.
> 
> Version 4:
> Minor formatting cleanup for kzAlloc error message.
> Should I remove the first patch section in version 5?
> 
> Version 3:
> md_autodetect_dev failure message is now more usefully verbose.
> removed unused i_found that was leftover from initial verification.
> Thank you Randy Dunlap for pointing where INT_MAX was, fixme fixed.
> 
> Version 2:
> using list_add_tail, and corrected missing i_passed++;.
> removed sections of code that would never be reached.
> 
> Version 1:
> 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.
> 

BEGIN

> 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.

END

> Subject: [patch 1/1] md: Software Raid autodetect dev list not array
> 
> SOFTWARE RAID (Multiple Disks) SUPPORT
> P:	Ingo Molnar
> M:	mingo@redhat.com
> P:	Neil Brown
> M:	neilb@suse.de
> L:	linux-raid@vger.kernel.org
> S:	Supported
> Unless you have a reason NOT to do so, CC linux-kernel@vger.kernel.org.
> 
> 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.
> 
> Signed-off-by: Michael J. Evans <mjevans1983@gmail.com>


-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2007-08-29 15:21 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <200708222058.45480.mjevans1983@comcast.net>
2007-08-24  3:37 ` [patch 1/1] md: Software Raid autodetect dev list not array Neil Brown
2007-08-24  5:50   ` Michael Evans
2007-08-26 11:51   ` [patch v2 " Michael J. Evans
2007-08-26 12:20     ` Michael Evans
2007-08-27  3:41       ` Kyle Moffett
2007-08-27  7:56         ` Michael Evans
2007-08-26 12:56     ` Jan Engelhardt
2007-08-26 15:58       ` Michael Evans
2007-08-26 16:56     ` Randy Dunlap
2007-08-26 19:18       ` Michael Evans
2007-08-27 22:16         ` [patch v3 " Michael J. Evans
2007-08-27 22:30           ` Randy Dunlap
2007-08-28  4:38             ` Michael J. Evans
2007-08-28  4:46               ` Randy Dunlap
2007-08-28 13:08                 ` Michael Evans
2007-08-28 13:14                   ` Jan Engelhardt
2007-08-28 13:26                     ` Michael J. Evans
2007-08-28 13:24                   ` Bill Davidsen
2007-08-28 13:32                     ` Michael Evans
2007-08-28 16:15                       ` Randy Dunlap
2007-08-28 17:10                         ` Michael Evans
2007-08-28 17:22                           ` Randy Dunlap
2007-08-28 19:11                             ` Michael Evans
2007-08-28 19:16                               ` Randy Dunlap
2007-08-29  8:06                                 ` [patch v5 " Michael J. Evans
2007-08-29 15:18                                   ` Randy Dunlap
2007-08-28 13:27                 ` Michael J. Evans
2007-08-28  4:39             ` [patch v4 " Michael J. Evans

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).