[PATCH 000 of 3] md: a few little patches

[PATCH 000 of 3] md: a few little patches

[NeilBrown]

Following 3 patches for md provide some code tidyup and a small
functionality improvement.
They do not need to go into 2.6.24 but are definitely appropriate 25-rc1.

(Patches made against 2.6.24-rc3-mm2)

Thanks,
NeilBrown


 [PATCH 001 of 3] md: raid6: Fix mktable.c
 [PATCH 002 of 3] md: raid6: clean up the style of raid6test/test.c
 [PATCH 003 of 3] md: Update md bitmap during resync.

[PATCH 001 of 3] md: raid6: Fix mktable.c

[NeilBrown]

From: "H. Peter Anvin" <hpa@zytor.com>

Make both mktables.c and its output CodingStyle compliant.  Update the
copyright notice.

Signed-off-by: H. Peter Anvin <hpa@zytor.com>
Signed-off-by: Neil Brown <neilb@suse.de>

### Diffstat output
 ./drivers/md/mktables.c |   43 +++++++++++++++++--------------------------
 1 file changed, 17 insertions(+), 26 deletions(-)

diff .prev/drivers/md/mktables.c ./drivers/md/mktables.c
--- .prev/drivers/md/mktables.c	2007-12-03 14:47:09.000000000 +1100
+++ ./drivers/md/mktables.c	2007-12-03 14:56:06.000000000 +1100
@@ -1,13 +1,10 @@
-#ident "$Id: mktables.c,v 1.2 2002/12/12 22:41:27 hpa Exp $"
-/* ----------------------------------------------------------------------- *
+/* -*- linux-c -*- ------------------------------------------------------- *
  *
- *   Copyright 2002 H. Peter Anvin - All Rights Reserved
+ *   Copyright 2002-2007 H. Peter Anvin - All Rights Reserved
  *
- *   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, Inc., 53 Temple Place Ste 330,
- *   Bostom MA 02111-1307, USA; either version 2 of the License, or
- *   (at your option) any later version; incorporated herein by reference.
+ *   This file is part of the Linux kernel, and is made available under
+ *   the terms of the GNU General Public License version 2 or (at your
+ *   option) any later version; incorporated herein by reference.
  *
  * ----------------------------------------------------------------------- */
 
@@ -73,8 +70,8 @@ int main(int argc, char *argv[])
 		for (j = 0; j < 256; j += 8) {
 			printf("\t\t");
 			for (k = 0; k < 8; k++)
-				printf("0x%02x, ", gfmul(i, j+k));
-			printf("\n");
+				printf("0x%02x,%c", gfmul(i, j + k),
+				       (k == 7) ? '\n' : ' ');
 		}
 		printf("\t},\n");
 	}
@@ -83,47 +80,41 @@ int main(int argc, char *argv[])
 	/* Compute power-of-2 table (exponent) */
 	v = 1;
 	printf("\nconst u8 __attribute__((aligned(256)))\n"
-		"raid6_gfexp[256] =\n"
-		"{\n");
+	       "raid6_gfexp[256] =\n" "{\n");
 	for (i = 0; i < 256; i += 8) {
 		printf("\t");
 		for (j = 0; j < 8; j++) {
-			exptbl[i+j] = v;
-			printf("0x%02x, ", v);
+			exptbl[i + j] = v;
+			printf("0x%02x,%c", v, (j == 7) ? '\n' : ' ');
 			v = gfmul(v, 2);
 			if (v == 1)
 				v = 0;	/* For entry 255, not a real entry */
 		}
-		printf("\n");
 	}
 	printf("};\n");
 
 	/* Compute inverse table x^-1 == x^254 */
 	printf("\nconst u8 __attribute__((aligned(256)))\n"
-		"raid6_gfinv[256] =\n"
-		"{\n");
+	       "raid6_gfinv[256] =\n" "{\n");
 	for (i = 0; i < 256; i += 8) {
 		printf("\t");
 		for (j = 0; j < 8; j++) {
-			v = gfpow(i+j, 254);
-			invtbl[i+j] = v;
-			printf("0x%02x, ", v);
+			invtbl[i + j] = v = gfpow(i + j, 254);
+			printf("0x%02x,%c", v, (j == 7) ? '\n' : ' ');
 		}
-		printf("\n");
 	}
 	printf("};\n");
 
 	/* Compute inv(2^x + 1) (exponent-xor-inverse) table */
 	printf("\nconst u8 __attribute__((aligned(256)))\n"
-		"raid6_gfexi[256] =\n"
-		"{\n");
+	       "raid6_gfexi[256] =\n" "{\n");
 	for (i = 0; i < 256; i += 8) {
 		printf("\t");
 		for (j = 0; j < 8; j++)
-			printf("0x%02x, ", invtbl[exptbl[i+j]^1]);
-		printf("\n");
+			printf("0x%02x,%c", invtbl[exptbl[i + j] ^ 1],
+			       (j == 7) ? '\n' : ' ');
 	}
-	printf("};\n\n");
+	printf("};\n");
 
 	return 0;
 }

[PATCH 002 of 3] md: raid6: clean up the style of raid6test/test.c

[NeilBrown]

From: "H. Peter Anvin" <hpa@zytor.com>
Date: Fri, 26 Oct 2007 11:22:42 -0700

Clean up the coding style in raid6test/test.c.  Break it apart into
subfunctions to make the code more readable.

Signed-off-by: H. Peter Anvin <hpa@zytor.com>
Signed-off-by: Neil Brown <neilb@suse.de>

### Diffstat output
 ./drivers/md/raid6test/test.c |  115 ++++++++++++++++++++++++------------------
 1 file changed, 68 insertions(+), 47 deletions(-)

diff .prev/drivers/md/raid6test/test.c ./drivers/md/raid6test/test.c
--- .prev/drivers/md/raid6test/test.c	2007-12-03 14:57:55.000000000 +1100
+++ ./drivers/md/raid6test/test.c	2007-12-03 14:57:55.000000000 +1100
@@ -1,12 +1,10 @@
 /* -*- linux-c -*- ------------------------------------------------------- *
  *
- *   Copyright 2002 H. Peter Anvin - All Rights Reserved
+ *   Copyright 2002-2007 H. Peter Anvin - All Rights Reserved
  *
- *   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, Inc., 53 Temple Place Ste 330,
- *   Bostom MA 02111-1307, USA; either version 2 of the License, or
- *   (at your option) any later version; incorporated herein by reference.
+ *   This file is part of the Linux kernel, and is made available under
+ *   the terms of the GNU General Public License version 2 or (at your
+ *   option) any later version; incorporated herein by reference.
  *
  * ----------------------------------------------------------------------- */
 
@@ -30,67 +28,87 @@ char *dataptrs[NDISKS];
 char data[NDISKS][PAGE_SIZE];
 char recovi[PAGE_SIZE], recovj[PAGE_SIZE];
 
-void makedata(void)
+static void makedata(void)
 {
 	int i, j;
 
-	for (  i = 0 ; i < NDISKS ; i++ ) {
-		for ( j = 0 ; j < PAGE_SIZE ; j++ ) {
+	for (i = 0; i < NDISKS; i++) {
+		for (j = 0; j < PAGE_SIZE; j++)
 			data[i][j] = rand();
-		}
+
 		dataptrs[i] = data[i];
 	}
 }
 
+static char disk_type(int d)
+{
+	switch (d) {
+	case NDISKS-2:
+		return 'P';
+	case NDISKS-1:
+		return 'Q';
+	default:
+		return 'D';
+	}
+}
+
+static int test_disks(int i, int j)
+{
+	int erra, errb;
+
+	memset(recovi, 0xf0, PAGE_SIZE);
+	memset(recovj, 0xba, PAGE_SIZE);
+
+	dataptrs[i] = recovi;
+	dataptrs[j] = recovj;
+
+	raid6_dual_recov(NDISKS, PAGE_SIZE, i, j, (void **)&dataptrs);
+
+	erra = memcmp(data[i], recovi, PAGE_SIZE);
+	errb = memcmp(data[j], recovj, PAGE_SIZE);
+
+	if (i < NDISKS-2 && j == NDISKS-1) {
+		/* We don't implement the DQ failure scenario, since it's
+		   equivalent to a RAID-5 failure (XOR, then recompute Q) */
+		erra = errb = 0;
+	} else {
+		printf("algo=%-8s  faila=%3d(%c)  failb=%3d(%c)  %s\n",
+		       raid6_call.name,
+		       i, disk_type(i),
+		       j, disk_type(j),
+		       (!erra && !errb) ? "OK" :
+		       !erra ? "ERRB" :
+		       !errb ? "ERRA" : "ERRAB");
+	}
+
+	dataptrs[i] = data[i];
+	dataptrs[j] = data[j];
+
+	return erra || errb;
+}
+
 int main(int argc, char *argv[])
 {
-	const struct raid6_calls * const * algo;
+	const struct raid6_calls *const *algo;
 	int i, j;
-	int erra, errb;
+	int err = 0;
 
 	makedata();
 
-	for ( algo = raid6_algos ; *algo ; algo++ ) {
-		if ( !(*algo)->valid || (*algo)->valid() ) {
+	for (algo = raid6_algos; *algo; algo++) {
+		if (!(*algo)->valid || (*algo)->valid()) {
 			raid6_call = **algo;
 
 			/* Nuke syndromes */
 			memset(data[NDISKS-2], 0xee, 2*PAGE_SIZE);
 
 			/* Generate assumed good syndrome */
-			raid6_call.gen_syndrome(NDISKS, PAGE_SIZE, (void **)&dataptrs);
+			raid6_call.gen_syndrome(NDISKS, PAGE_SIZE,
+						(void **)&dataptrs);
 
-			for ( i = 0 ; i < NDISKS-1 ; i++ ) {
-				for ( j = i+1 ; j < NDISKS ; j++ ) {
-					memset(recovi, 0xf0, PAGE_SIZE);
-					memset(recovj, 0xba, PAGE_SIZE);
-
-					dataptrs[i] = recovi;
-					dataptrs[j] = recovj;
-
-					raid6_dual_recov(NDISKS, PAGE_SIZE, i, j, (void **)&dataptrs);
-
-					erra = memcmp(data[i], recovi, PAGE_SIZE);
-					errb = memcmp(data[j], recovj, PAGE_SIZE);
-
-					if ( i < NDISKS-2 && j == NDISKS-1 ) {
-						/* We don't implement the DQ failure scenario, since it's
-						   equivalent to a RAID-5 failure (XOR, then recompute Q) */
-					} else {
-						printf("algo=%-8s  faila=%3d(%c)  failb=%3d(%c)  %s\n",
-						       raid6_call.name,
-						       i, (i==NDISKS-2)?'P':'D',
-						       j, (j==NDISKS-1)?'Q':(j==NDISKS-2)?'P':'D',
-						       (!erra && !errb) ? "OK" :
-						       !erra ? "ERRB" :
-						       !errb ? "ERRA" :
-						       "ERRAB");
-					}
-
-					dataptrs[i] = data[i];
-					dataptrs[j] = data[j];
-				}
-			}
+			for (i = 0; i < NDISKS-1; i++)
+				for (j = i+1; j < NDISKS; j++)
+					err += test_disks(i, j);
 		}
 		printf("\n");
 	}
@@ -99,5 +117,8 @@ int main(int argc, char *argv[])
 	/* Pick the best algorithm test */
 	raid6_select_algo();
 
-	return 0;
+	if (err)
+		printf("\n*** ERRORS FOUND ***\n");
+
+	return err;
 }

[PATCH 003 of 3] md: Update md bitmap during resync.

[NeilBrown]

Currently and md array with a write-intent bitmap does not updated
that bitmap to reflect successful partial resync.  Rather the entire
bitmap is updated when the resync completes.

This is because there is no guarentee that resync requests will
complete in order, and tracking each request individually is
unnecessarily burdensome.

However there is value in regularly updating the bitmap, so add code
to periodically pause while all pending sync requests complete, then
update the bitmap.  Doing this only every few seconds (the same as the
bitmap update time) does not notciably affect resync performance.

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

### Diffstat output
 ./drivers/md/bitmap.c         |   34 +++++++++++++++++++++++++++++-----
 ./drivers/md/raid1.c          |    1 +
 ./drivers/md/raid10.c         |    2 ++
 ./drivers/md/raid5.c          |    3 +++
 ./include/linux/raid/bitmap.h |    3 +++
 5 files changed, 38 insertions(+), 5 deletions(-)

diff .prev/drivers/md/bitmap.c ./drivers/md/bitmap.c
--- .prev/drivers/md/bitmap.c	2007-12-03 14:58:48.000000000 +1100
+++ ./drivers/md/bitmap.c	2007-12-03 14:59:00.000000000 +1100
@@ -1342,14 +1342,38 @@ void bitmap_close_sync(struct bitmap *bi
 	 */
 	sector_t sector = 0;
 	int blocks;
-	if (!bitmap) return;
+	if (!bitmap)
+		return;
 	while (sector < bitmap->mddev->resync_max_sectors) {
 		bitmap_end_sync(bitmap, sector, &blocks, 0);
-/*
-		if (sector < 500) printk("bitmap_close_sync: sec %llu blks %d\n",
-					 (unsigned long long)sector, blocks);
-*/		sector += blocks;
+		sector += blocks;
+	}
+}
+
+void bitmap_cond_end_sync(struct bitmap *bitmap, sector_t sector)
+{
+	sector_t s = 0;
+	int blocks;
+
+	if (!bitmap)
+		return;
+	if (sector == 0) {
+		bitmap->last_end_sync = jiffies;
+		return;
+	}
+	if (time_before(jiffies, (bitmap->last_end_sync
+				  + bitmap->daemon_sleep * HZ)))
+		return;
+	wait_event(bitmap->mddev->recovery_wait,
+		   atomic_read(&bitmap->mddev->recovery_active) == 0);
+
+	sector &= ~((1ULL << CHUNK_BLOCK_SHIFT(bitmap)) - 1);
+	s = 0;
+	while (s < sector && s < bitmap->mddev->resync_max_sectors) {
+		bitmap_end_sync(bitmap, s, &blocks, 0);
+		s += blocks;
 	}
+	bitmap->last_end_sync = jiffies;
 }
 
 static void bitmap_set_memory_bits(struct bitmap *bitmap, sector_t offset, int needed)

diff .prev/drivers/md/raid10.c ./drivers/md/raid10.c
--- .prev/drivers/md/raid10.c	2007-12-03 14:58:48.000000000 +1100
+++ ./drivers/md/raid10.c	2007-12-03 14:58:10.000000000 +1100
@@ -1670,6 +1670,8 @@ static sector_t sync_request(mddev_t *md
 	if (!go_faster && conf->nr_waiting)
 		msleep_interruptible(1000);
 
+	bitmap_cond_end_sync(mddev->bitmap, sector_nr);
+
 	/* Again, very different code for resync and recovery.
 	 * Both must result in an r10bio with a list of bios that
 	 * have bi_end_io, bi_sector, bi_bdev set,

diff .prev/drivers/md/raid1.c ./drivers/md/raid1.c
--- .prev/drivers/md/raid1.c	2007-12-03 14:58:48.000000000 +1100
+++ ./drivers/md/raid1.c	2007-12-03 14:58:10.000000000 +1100
@@ -1684,6 +1684,7 @@ static sector_t sync_request(mddev_t *md
 	if (!go_faster && conf->nr_waiting)
 		msleep_interruptible(1000);
 
+	bitmap_cond_end_sync(mddev->bitmap, sector_nr);
 	raise_barrier(conf);
 
 	conf->next_resync = sector_nr;

diff .prev/drivers/md/raid5.c ./drivers/md/raid5.c
--- .prev/drivers/md/raid5.c	2007-12-03 14:58:48.000000000 +1100
+++ ./drivers/md/raid5.c	2007-12-03 14:58:10.000000000 +1100
@@ -4333,6 +4333,9 @@ static inline sector_t sync_request(mdde
 		return sync_blocks * STRIPE_SECTORS; /* keep things rounded to whole stripes */
 	}
 
+
+	bitmap_cond_end_sync(mddev->bitmap, sector_nr);
+
 	pd_idx = stripe_to_pdidx(sector_nr, conf, raid_disks);
 
 	sh = wait_for_inactive_cache(conf, sector_nr, raid_disks, pd_idx);

diff .prev/include/linux/raid/bitmap.h ./include/linux/raid/bitmap.h
--- .prev/include/linux/raid/bitmap.h	2007-12-03 14:58:48.000000000 +1100
+++ ./include/linux/raid/bitmap.h	2007-12-03 14:58:10.000000000 +1100
@@ -244,6 +244,8 @@ struct bitmap {
 	 */
 	unsigned long daemon_lastrun; /* jiffies of last run */
 	unsigned long daemon_sleep; /* how many seconds between updates? */
+	unsigned long last_end_sync; /* when we lasted called end_sync to
+				      * update bitmap with resync progress */
 
 	atomic_t pending_writes; /* pending writes to the bitmap file */
 	wait_queue_head_t write_wait;
@@ -275,6 +277,7 @@ void bitmap_endwrite(struct bitmap *bitm
 int bitmap_start_sync(struct bitmap *bitmap, sector_t offset, int *blocks, int degraded);
 void bitmap_end_sync(struct bitmap *bitmap, sector_t offset, int *blocks, int aborted);
 void bitmap_close_sync(struct bitmap *bitmap);
+void bitmap_cond_end_sync(struct bitmap *bitmap, sector_t sector);
 
 void bitmap_unplug(struct bitmap *bitmap);
 void bitmap_daemon_work(struct bitmap *bitmap);

Re: [PATCH 003 of 3] md: Update md bitmap during resync.

[Mike Snitzer]

[-- Attachment #1: Type: text/plain, Size: 838 bytes --]

On Dec 7, 2007 12:42 AM, NeilBrown <neilb@suse.de> wrote:
>
> Currently and md array with a write-intent bitmap does not updated
> that bitmap to reflect successful partial resync.  Rather the entire
> bitmap is updated when the resync completes.
>
> This is because there is no guarentee that resync requests will
> complete in order, and tracking each request individually is
> unnecessarily burdensome.
>
> However there is value in regularly updating the bitmap, so add code
> to periodically pause while all pending sync requests complete, then
> update the bitmap.  Doing this only every few seconds (the same as the
> bitmap update time) does not notciably affect resync performance.
>
> Signed-off-by: Neil Brown <neilb@suse.de>

Hi Neil,

You forgot to export bitmap_cond_end_sync.  Please see the attached patch.

regards,
Mike

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: export_bitmap_cond_end_sync.patch --]
[-- Type: text/x-patch; name=export_bitmap_cond_end_sync.patch, Size: 330 bytes --]

diff --git a/drivers/md/bitmap.c b/drivers/md/bitmap.c
index f31ea4f..b596538 100644
--- a/drivers/md/bitmap.c
+++ b/drivers/md/bitmap.c
@@ -1566,3 +1566,4 @@ EXPORT_SYMBOL(bitmap_start_sync);
 EXPORT_SYMBOL(bitmap_end_sync);
 EXPORT_SYMBOL(bitmap_unplug);
 EXPORT_SYMBOL(bitmap_close_sync);
+EXPORT_SYMBOL(bitmap_cond_end_sync);