[PATCH 000 of 3] md: Introduction - 3 bugfixs for -mm

[PATCH 000 of 3] md: Introduction - 3 bugfixs for -mm

[NeilBrown]

The first of these fixes issues with the new bmap based bitmap file
access code, and possibly should be an -mm hotfix, and without it,
'internal' bitmaps don't work any more :-(

Others are minor and unrelated.

Thanks,
NeilBrown


 [PATCH 001 of 3] md: Change md/bitmap file handling to use bmap to file blocks-fix
 [PATCH 002 of 3] md: Fix inverted test for 'repair' directive.
 [PATCH 003 of 3] md: Calculate correct array size for raid10 in new offset mode.

[PATCH 001 of 3] md: Change md/bitmap file handling to use bmap to file blocks-fix

[NeilBrown]

Fix problems with new bmap based access to bitmap files.

1/ When not using a file based bitmap, attach a NULL list of buffers
   to each page so the common free_buffer routine can cope.
2/ Use submit_bh to read as well as write, rather than vfs_read.  
   This makes read and write more symetric.
3/ sync the file before reading, to ensure that the page cache has no
   dirty pages that might get written out later.

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

### Diffstat output
 ./drivers/md/bitmap.c |   46 ++++++++++++++++++++++------------------------
 1 file changed, 22 insertions(+), 24 deletions(-)

diff ./drivers/md/bitmap.c~current~ ./drivers/md/bitmap.c
--- ./drivers/md/bitmap.c~current~	2006-05-16 11:09:26.000000000 +1000
+++ ./drivers/md/bitmap.c	2006-05-16 11:09:33.000000000 +1000
@@ -247,6 +247,8 @@ static struct page *read_sb_page(mddev_t
 
 		if (sync_page_io(rdev->bdev, target, PAGE_SIZE, page, READ)) {
 			page->index = index;
+			attach_page_buffers(page, NULL); /* so that free_buffer will
+							  * quietly no-op */
 			return page;
 		}
 	}
@@ -349,11 +351,8 @@ static struct page *read_page(struct fil
 {
 	struct page *page = NULL;
 	struct inode *inode = file->f_dentry->d_inode;
-	loff_t pos = index << PAGE_SHIFT;
-	int ret;
 	struct buffer_head *bh;
 	sector_t block;
-	mm_segment_t oldfs;
 
 	PRINTK("read bitmap file (%dB @ %Lu)\n", (int)PAGE_SIZE,
 			(unsigned long long)index << PAGE_SHIFT);
@@ -364,18 +363,6 @@ static struct page *read_page(struct fil
 	if (IS_ERR(page))
 		goto out;
 
-	oldfs = get_fs();
-	set_fs(KERNEL_DS);
-	ret = vfs_read(file, (char __user*) page_address(page), count, &pos);
-	set_fs(oldfs);
-
-	if (ret >= 0 && ret != count)
-		ret = -EIO;
-	if (ret < 0) {
-		put_page(page);
-		page = ERR_PTR(ret);
-		goto out;
-	}
 	bh = alloc_page_buffers(page, 1<<inode->i_blkbits, 0);
 	if (!bh) {
 		put_page(page);
@@ -403,12 +390,22 @@ static struct page *read_page(struct fil
 
 			bh->b_end_io = end_bitmap_write;
 			bh->b_private = bitmap;
+			atomic_inc(&bitmap->pending_writes);
+			set_buffer_locked(bh);
+			set_buffer_mapped(bh);
+			submit_bh(READ, bh);
 		}
 		block++;
 		bh = bh->b_this_page;
 	}
-
 	page->index = index;
+
+	wait_event(bitmap->write_wait,
+		   atomic_read(&bitmap->pending_writes)==0);
+	if (bitmap->flags & BITMAP_WRITE_ERROR) {
+		free_buffers(page);
+		page = ERR_PTR(-EIO);
+	}
 out:
 	if (IS_ERR(page))
 		printk(KERN_ALERT "md: bitmap read error: (%dB @ %Lu): %ld\n",
@@ -1415,15 +1412,19 @@ int bitmap_create(mddev_t *mddev)
 		return -ENOMEM;
 
 	spin_lock_init(&bitmap->lock);
+	atomic_set(&bitmap->pending_writes, 0);
+	init_waitqueue_head(&bitmap->write_wait);
+
 	bitmap->mddev = mddev;
 
 	bitmap->file = file;
 	bitmap->offset = mddev->bitmap_offset;
-	if (file) get_file(file);
-
-	/* Ensure we read fresh data */
-	invalidate_inode_pages(file->f_dentry->d_inode->i_mapping);
-
+	if (file) {
+		get_file(file);
+		do_sync_file_range(file, 0, LLONG_MAX,
+				   SYNC_FILE_RANGE_WRITE |
+				   SYNC_FILE_RANGE_WAIT_AFTER);
+	}
 	/* read superblock from bitmap file (this sets bitmap->chunksize) */
 	err = bitmap_read_sb(bitmap);
 	if (err)
@@ -1446,9 +1447,6 @@ int bitmap_create(mddev_t *mddev)
 
 	bitmap->syncchunk = ~0UL;
 
-	atomic_set(&bitmap->pending_writes, 0);
-	init_waitqueue_head(&bitmap->write_wait);
-
 #ifdef INJECT_FATAL_FAULT_1
 	bitmap->bp = NULL;
 #else

[PATCH 002 of 3] md: Fix inverted test for 'repair' directive.

[NeilBrown]

We should be able to write 'repair' to /sys/block/mdX/md/sync_action,
however due to and inverted test, that always given EINVAL.

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

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

diff ./drivers/md/md.c~current~ ./drivers/md/md.c
--- ./drivers/md/md.c~current~	2006-05-16 11:09:25.000000000 +1000
+++ ./drivers/md/md.c	2006-05-16 11:09:41.000000000 +1000
@@ -2318,7 +2318,7 @@ action_store(mddev_t *mddev, const char 
 	} else {
 		if (cmd_match(page, "check"))
 			set_bit(MD_RECOVERY_CHECK, &mddev->recovery);
-		else if (cmd_match(page, "repair"))
+		else if (!cmd_match(page, "repair"))
 			return -EINVAL;
 		set_bit(MD_RECOVERY_REQUESTED, &mddev->recovery);
 		set_bit(MD_RECOVERY_SYNC, &mddev->recovery);

[PATCH 003 of 3] md: Calculate correct array size for raid10 in new offset mode.

[NeilBrown]

The size calculation made assumtion which the new offset mode
didn't follow.  This gets the size right in all cases.

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

### Diffstat output
 ./drivers/md/raid10.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff ./drivers/md/raid10.c~current~ ./drivers/md/raid10.c
--- ./drivers/md/raid10.c~current~	2006-05-16 11:09:25.000000000 +1000
+++ ./drivers/md/raid10.c	2006-05-16 11:09:42.000000000 +1000
@@ -2060,7 +2060,13 @@ static int run(mddev_t *mddev)
 	/*
 	 * Ok, everything is just fine now
 	 */
-	size = conf->stride * conf->raid_disks;
+	if (conf->far_offset) {
+		size = mddev->size >> (conf->chunk_shift-1);
+		size *= conf->raid_disks;
+		size <<= conf->chunk_shift;
+		sector_div(size, conf->far_copies);
+	} else
+		size = conf->stride * conf->raid_disks;
 	sector_div(size, conf->near_copies);
 	mddev->array_size = size/2;
 	mddev->resync_max_sectors = size;

Re: [PATCH 001 of 3] md: Change md/bitmap file handling to use bmap to file blocks-fix

[Andrew Morton]

NeilBrown <neilb@suse.de> wrote:
>
>  +		do_sync_file_range(file, 0, LLONG_MAX,
>  +				   SYNC_FILE_RANGE_WRITE |
>  +				   SYNC_FILE_RANGE_WAIT_AFTER);

That needs a SYNC_FILE_RANGE_WAIT_BEFORE too.  Otherwise any dirty,
under-writeback pages will remain dirty.  I'll make that change.

Re: [PATCH 001 of 3] md: Change md/bitmap file handling to use bmap to file blocks-fix

[Neil Brown]

On Monday May 15, akpm@osdl.org wrote:
> NeilBrown <neilb@suse.de> wrote:
> >
> >  +		do_sync_file_range(file, 0, LLONG_MAX,
> >  +				   SYNC_FILE_RANGE_WRITE |
> >  +				   SYNC_FILE_RANGE_WAIT_AFTER);
> 
> That needs a SYNC_FILE_RANGE_WAIT_BEFORE too.  Otherwise any dirty,
> under-writeback pages will remain dirty.  I'll make that change.

Ahhh.. yes... that makes sense!  Thanks :-)

NeilBrown

Re: [PATCH 000 of 3] md: Introduction - 3 bugfixs for -mm

[Reuben Farrelly]

On 16/05/2006 1:12 p.m., NeilBrown wrote:
> The first of these fixes issues with the new bmap based bitmap file
> access code, and possibly should be an -mm hotfix, and without it,
> 'internal' bitmaps don't work any more :-(
> 
> Others are minor and unrelated.
> 
> Thanks,
> NeilBrown
> 
> 
>  [PATCH 001 of 3] md: Change md/bitmap file handling to use bmap to file blocks-fix
>  [PATCH 002 of 3] md: Fix inverted test for 'repair' directive.
>  [PATCH 003 of 3] md: Calculate correct array size for raid10 in new offset mode.

Patch 1 fixes the problems I was having with RAID-1 arrays not able to start up 
on 2.6.17-rc4-mm1.  Thanks for that.

However things appear still not quite right on boot, as each mount works but 
displays as though it didn't work, ie:

md: considering sdc2 ...
md:  adding sdc2 ...
md:  adding sda2 ...
md: created md0
md: bind<sda2>
md: bind<sdc2>
md: running: <sdc2><sda2>
raid1: raid set md0 active with 0 out of 2 mirrors

0 out of 2 ?

cat /proc/mdstats  shows that everything does in fact seem to be working:

md0 : active raid1 sdc2[1] sda2[0]
       24410688 blocks [2/2] [UU]
       bitmap: 21/187 pages [84KB], 64KB chunk

The array otherwise seems to be fine.  I guess it's just a visual glitch.

reuben

Re: [PATCH 000 of 3] md: Introduction - 3 bugfixs for -mm

[Neil Brown]

On Thursday May 18, reuben-lkml@reub.net wrote:
> 
> However things appear still not quite right on boot, as each mount works but 
> displays as though it didn't work, ie:
> 
> md: considering sdc2 ...
> md:  adding sdc2 ...
> md:  adding sda2 ...
> md: created md0
> md: bind<sda2>
> md: bind<sdc2>
> md: running: <sdc2><sda2>
> raid1: raid set md0 active with 0 out of 2 mirrors
> 
> 0 out of 2 ?

That is fixed by this patch, which I thought I had submitted...
Time get the latest -mm and see which of my patches are still pending
I guess.

Thanks,
NeilBrown

------------------------------
Fix recently broken calculation of degraded for raid1

A recent patch broke this code: rdev doesn't have meaningful
value at this point - disk->rdev is what should be used.

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

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

diff ./drivers/md/raid1.c~current~ ./drivers/md/raid1.c
--- ./drivers/md/raid1.c~current~	2006-05-02 14:15:28.000000000 +1000
+++ ./drivers/md/raid1.c	2006-05-02 14:15:44.000000000 +1000
@@ -1889,7 +1889,7 @@ static int run(mddev_t *mddev)
 		disk = conf->mirrors + i;
 
 		if (!disk->rdev ||
-		    !test_bit(In_sync, &rdev->flags)) {
+		    !test_bit(In_sync, &disk->rdev->flags)) {
 			disk->head_position = 0;
 			mddev->degraded++;
 		}