[PATCH 000 of 6] md: Introduction - patching those patches.

[PATCH 000 of 6] md: Introduction - patching those patches.

[NeilBrown]

This is the "Andrew Morton: Awesome code reviewer" patch series, that fixes
up issues identified in my recent series of md patches.

NeilBrown


 [PATCH 001 of 6] md: INIT_LIST_HEAD to LIST_HEAD conversions.
 [PATCH 002 of 6] md: Documentation and tidy up for resize_stripes
 [PATCH 003 of 6] md: Remove an unused variable.
 [PATCH 004 of 6] md: Improve comments about locking situation in raid5 make_request
 [PATCH 005 of 6] md: Remove some stray semi-colons after functions called in macro....
 [PATCH 006 of 6] md: Make new function stripe_to_pdidx static.

[PATCH 001 of 6] md: INIT_LIST_HEAD to LIST_HEAD conversions.

[NeilBrown]

A couple of places we all INIT_LIST_HEAD on a locally declared
variable.  This can be changed to a LIST_HEAD declaration.

Signed-off-by: Neil Brown <neilb@suse.de>

### Diffstat output
 ./drivers/md/md.c    |    2 +-
 ./drivers/md/raid5.c |    6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff ./drivers/md/md.c~current~ ./drivers/md/md.c
--- ./drivers/md/md.c~current~	2006-03-17 18:17:56.000000000 +1100
+++ ./drivers/md/md.c	2006-03-17 18:18:19.000000000 +1100
@@ -2895,7 +2895,6 @@ static void autorun_array(mddev_t *mddev
  */
 static void autorun_devices(int part)
 {
-	struct list_head candidates;
 	struct list_head *tmp;
 	mdk_rdev_t *rdev0, *rdev;
 	mddev_t *mddev;
@@ -2904,6 +2903,7 @@ static void autorun_devices(int part)
 	printk(KERN_INFO "md: autorun ...\n");
 	while (!list_empty(&pending_raid_disks)) {
 		dev_t dev;
+		LIST_HEAD(candidates);
 		rdev0 = list_entry(pending_raid_disks.next,
 					 mdk_rdev_t, same_set);
 

diff ./drivers/md/raid5.c~current~ ./drivers/md/raid5.c
--- ./drivers/md/raid5.c~current~	2006-03-17 18:17:57.000000000 +1100
+++ ./drivers/md/raid5.c	2006-03-17 18:18:19.000000000 +1100
@@ -345,7 +345,8 @@ static int resize_stripes(raid5_conf_t *
 	 * at some points in this operation.
 	 */
 	struct stripe_head *osh, *nsh;
-	struct list_head newstripes, oldstripes;
+	LIST_HEAD(newstripes);
+	LIST_HEAD(oldstripes);
 	struct disk_info *ndisks;
 	int err = 0;
 	kmem_cache_t *sc;
@@ -359,7 +360,7 @@ static int resize_stripes(raid5_conf_t *
 			       0, 0, NULL, NULL);
 	if (!sc)
 		return -ENOMEM;
-	INIT_LIST_HEAD(&newstripes);
+
 	for (i = conf->max_nr_stripes; i; i--) {
 		nsh = kmem_cache_alloc(sc, GFP_NOIO);
 		if (!nsh)
@@ -385,7 +386,6 @@ static int resize_stripes(raid5_conf_t *
 	/* OK, we have enough stripes, start collecting inactive
 	 * stripes and copying them over
 	 */
-	INIT_LIST_HEAD(&oldstripes);
 	list_for_each_entry(nsh, &newstripes, lru) {
 		spin_lock_irq(&conf->device_lock);
 		wait_event_lock_irq(conf->wait_for_stripe,

[PATCH 002 of 6] md: Documentation and tidy up for resize_stripes

[NeilBrown]

Explain the stages of resize_stripes so that it is clear what it
happening, why GFP_NOIO is needed, and how -ENOMEM is handled.

Also move the releasing of old stripes and the old kmem_cache
earlier and lose the need for 'oldstripes'.

Signed-off-by: Neil Brown <neilb@suse.de>

### Diffstat output
 ./drivers/md/raid5.c |   51 ++++++++++++++++++++++++++++++++-------------------
 1 file changed, 32 insertions(+), 19 deletions(-)

diff ./drivers/md/raid5.c~current~ ./drivers/md/raid5.c
--- ./drivers/md/raid5.c~current~	2006-03-17 18:18:19.000000000 +1100
+++ ./drivers/md/raid5.c	2006-03-17 18:18:32.000000000 +1100
@@ -334,19 +334,31 @@ static int grow_stripes(raid5_conf_t *co
 }
 static int resize_stripes(raid5_conf_t *conf, int newsize)
 {
-	/* make all the stripes able to hold 'newsize' devices.
+	/* Make all the stripes able to hold 'newsize' devices.
 	 * New slots in each stripe get 'page' set to a new page.
-	 * We allocate all the new stripes first, then if that succeeds,
-	 * copy everything across.
-	 * Finally we add new pages.  This could fail, but we leave
-	 * the stripe cache at it's new size, just with some pages empty.
 	 *
-	 * We use GFP_NOIO allocations as IO to the raid5 is blocked
-	 * at some points in this operation.
+	 * This happens in stages:
+	 * 1/ create a new kmem_cache and allocate the required number of
+	 *    stripe_heads.
+	 * 2/ gather all the old stripe_heads and tranfer the pages across
+	 *    to the new stripe_heads.  This will have the side effect of
+	 *    freezing the array as once all stripe_heads have been collected,
+	 *    no IO will be possible.  Old stripe heads are freed once their
+	 *    pages have been transferred over, and the old kmem_cache is
+	 *    freed when all stripes are done.
+	 * 3/ reallocate conf->disks to be suitable bigger.  If this fails,
+	 *    we simple return a failre status - no need to clean anything up.
+	 * 4/ allocate new pages for the new slots in the new stripe_heads.
+	 *    If this fails, we don't bother trying the shrink the
+	 *    stripe_heads down again, we just leave them as they are.
+	 *    As each stripe_head is processed the new one is released into
+	 *    active service.
+	 *
+	 * Once step2 is started, we cannot afford to wait for a write,
+	 * so we use GFP_NOIO allocations.
 	 */
 	struct stripe_head *osh, *nsh;
 	LIST_HEAD(newstripes);
-	LIST_HEAD(oldstripes);
 	struct disk_info *ndisks;
 	int err = 0;
 	kmem_cache_t *sc;
@@ -355,6 +367,7 @@ static int resize_stripes(raid5_conf_t *
 	if (newsize <= conf->pool_size)
 		return 0; /* never bother to shrink */
 
+	/* Step 1 */
 	sc = kmem_cache_create(conf->cache_name[1-conf->active_name],
 			       sizeof(struct stripe_head)+(newsize-1)*sizeof(struct r5dev),
 			       0, 0, NULL, NULL);
@@ -362,7 +375,7 @@ static int resize_stripes(raid5_conf_t *
 		return -ENOMEM;
 
 	for (i = conf->max_nr_stripes; i; i--) {
-		nsh = kmem_cache_alloc(sc, GFP_NOIO);
+		nsh = kmem_cache_alloc(sc, GFP_KERNEL);
 		if (!nsh)
 			break;
 
@@ -383,7 +396,8 @@ static int resize_stripes(raid5_conf_t *
 		kmem_cache_destroy(sc);
 		return -ENOMEM;
 	}
-	/* OK, we have enough stripes, start collecting inactive
+	/* Step 2 - Must use GFP_NOIO now.
+	 * OK, we have enough stripes, start collecting inactive
 	 * stripes and copying them over
 	 */
 	list_for_each_entry(nsh, &newstripes, lru) {
@@ -400,10 +414,11 @@ static int resize_stripes(raid5_conf_t *
 			nsh->dev[i].page = osh->dev[i].page;
 		for( ; i<newsize; i++)
 			nsh->dev[i].page = NULL;
-		list_add(&osh->lru, &oldstripes);
+		kmem_cache_free(conf->slab_cache, osh);
 	}
-	/* Got them all.
-	 * Return the new ones and free the old ones.
+	kmem_cache_destroy(conf->slab_cache);
+
+	/* Step 3.
 	 * At this point, we are holding all the stripes so the array
 	 * is completely stalled, so now is a good time to resize
 	 * conf->disks.
@@ -416,6 +431,8 @@ static int resize_stripes(raid5_conf_t *
 		conf->disks = ndisks;
 	} else
 		err = -ENOMEM;
+
+	/* Step 4, return new stripes to service */
 	while(!list_empty(&newstripes)) {
 		nsh = list_entry(newstripes.next, struct stripe_head, lru);
 		list_del_init(&nsh->lru);
@@ -428,12 +445,8 @@ static int resize_stripes(raid5_conf_t *
 			}
 		release_stripe(nsh);
 	}
-	while(!list_empty(&oldstripes)) {
-		osh = list_entry(oldstripes.next, struct stripe_head, lru);
-		list_del(&osh->lru);
-		kmem_cache_free(conf->slab_cache, osh);
-	}
-	kmem_cache_destroy(conf->slab_cache);
+	/* critical section pass, GFP_NOIO no longer needed */
+
 	conf->slab_cache = sc;
 	conf->active_name = 1-conf->active_name;
 	conf->pool_size = newsize;

[PATCH 003 of 6] md: Remove an unused variable.

[NeilBrown]

Signed-off-by: Neil Brown <neilb@suse.de>

### Diffstat output
 ./drivers/md/raid5.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff ./drivers/md/raid5.c~current~ ./drivers/md/raid5.c
--- ./drivers/md/raid5.c~current~	2006-03-17 18:18:32.000000000 +1100
+++ ./drivers/md/raid5.c	2006-03-17 18:18:35.000000000 +1100
@@ -2186,7 +2186,7 @@ static int run(mddev_t *mddev)
 		 * increase, and we must be past the point where
 		 * a stripe over-writes itself
 		 */
-		sector_t here_new, here_old, there_new;
+		sector_t here_new, here_old;
 		int old_disks;
 
 		if (mddev->new_level != mddev->level ||

[PATCH 004 of 6] md: Improve comments about locking situation in raid5 make_request

[NeilBrown]

Signed-off-by: Neil Brown <neilb@suse.de>

### Diffstat output
 ./drivers/md/raid5.c |   15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff ./drivers/md/raid5.c~current~ ./drivers/md/raid5.c
--- ./drivers/md/raid5.c~current~	2006-03-17 18:18:35.000000000 +1100
+++ ./drivers/md/raid5.c	2006-03-17 18:18:44.000000000 +1100
@@ -1766,6 +1766,14 @@ static int make_request(request_queue_t 
 		if (likely(conf->expand_progress == MaxSector))
 			disks = conf->raid_disks;
 		else {
+			/* spinlock is needed as expand_progress may be
+			 * 64bit on a 32bit platform, and so it might be
+			 * possible to see a half-updated value
+			 * Ofcourse expand_progress could change after
+			 * the lock is dropped, so once we get a reference
+			 * to the stripe that we think it is, we will have
+			 * to check again.
+			 */
 			spin_lock_irq(&conf->device_lock);
 			disks = conf->raid_disks;
 			if (logical_sector >= conf->expand_progress)
@@ -1789,7 +1797,12 @@ static int make_request(request_queue_t 
 		if (sh) {
 			if (unlikely(conf->expand_progress != MaxSector)) {
 				/* expansion might have moved on while waiting for a
-				 * stripe, so we much do the range check again.
+				 * stripe, so we must do the range check again.
+				 * Expansion could still move past after this
+				 * test, but as we are holding a reference to
+				 * 'sh', we know that if that happens,
+				 *  STRIPE_EXPANDING will get set and the expansion
+				 * won't proceed until we finish with the stripe.
 				 */
 				int must_retry = 0;
 				spin_lock_irq(&conf->device_lock);

[PATCH 005 of 6] md: Remove some stray semi-colons after functions called in macro....

[NeilBrown]

wait_event_lock_irq puts a ';' after its usage of the 4th arg, so we
don't need to.

Signed-off-by: Neil Brown <neilb@suse.de>

### Diffstat output
 ./drivers/md/raid5.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff ./drivers/md/raid5.c~current~ ./drivers/md/raid5.c
--- ./drivers/md/raid5.c~current~	2006-03-17 18:18:44.000000000 +1100
+++ ./drivers/md/raid5.c	2006-03-17 18:18:48.000000000 +1100
@@ -262,7 +262,7 @@ static struct stripe_head *get_active_st
 						     < (conf->max_nr_stripes *3/4)
 						     || !conf->inactive_blocked),
 						    conf->device_lock,
-						    unplug_slaves(conf->mddev);
+						    unplug_slaves(conf->mddev)
 					);
 				conf->inactive_blocked = 0;
 			} else
@@ -405,7 +405,7 @@ static int resize_stripes(raid5_conf_t *
 		wait_event_lock_irq(conf->wait_for_stripe,
 				    !list_empty(&conf->inactive_list),
 				    conf->device_lock,
-				    unplug_slaves(conf->mddev);
+				    unplug_slaves(conf->mddev)
 			);
 		osh = get_free_stripe(conf);
 		spin_unlock_irq(&conf->device_lock);

[PATCH 006 of 6] md: Make new function stripe_to_pdidx static.

[NeilBrown]

Signed-off-by: Neil Brown <neilb@suse.de>

### Diffstat output
 ./drivers/md/raid5.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff ./drivers/md/raid5.c~current~ ./drivers/md/raid5.c
--- ./drivers/md/raid5.c~current~	2006-03-17 18:18:48.000000000 +1100
+++ ./drivers/md/raid5.c	2006-03-17 18:18:50.000000000 +1100
@@ -1037,7 +1037,7 @@ static int add_stripe_bio(struct stripe_
 
 static void end_reshape(raid5_conf_t *conf);
 
-int stripe_to_pdidx(sector_t stripe, raid5_conf_t *conf, int disks)
+static int stripe_to_pdidx(sector_t stripe, raid5_conf_t *conf, int disks)
 {
 	int sectors_per_chunk = conf->chunk_size >> 9;
 	sector_t x = stripe;