[PATCH 0/4] more sector_t conversions

[PATCH 0/4] more sector_t conversions

[Andre Noll]

Hi

here's the next batch of patches against Neil's for-next branch that
change some of the remaining 1k-based representations to sector-based
representations.

Patch #1 fixes what I believe is a bug in the check for overlapping
partitions. This patch might also be suitable for -stable while #2-#4
are definitely material for -next.

Patch #2 changes the rdev_size_change function pointer of struct
super_type. The patched version takes the number of sectors instead
of the number of blocks.

Patches #3 and #4 convert the array_size fields of struct mddev_s
and of struct linear_private_data to sector-based representations
respectively.

Please review.
Andre
---

 drivers/md/faulty.c         |    2 +-
 drivers/md/linear.c         |   18 +++++++-------
 drivers/md/md.c             |   52 ++++++++++++++++++++----------------------
 drivers/md/multipath.c      |    2 +-
 drivers/md/raid0.c          |    8 +++---
 drivers/md/raid1.c          |   10 ++++----
 drivers/md/raid10.c         |    2 +-
 drivers/md/raid5.c          |   14 +++++-----
 include/linux/raid/linear.h |    2 +-
 include/linux/raid/md_k.h   |    2 +-
 10 files changed, 55 insertions(+), 57 deletions(-)

[PATCH 1/4] md: Fix check for overlapping devices.

[Andre Noll]

The checks in overlaps() expect all parameters either in block-based
or sector-based quantities. However, its single caller passes two
rdev->data_offset arguments as well as two rdev->size arguments, the
former being sector counts while the latter are measured in 1K blocks.

This could cause rdev_size_store() to accept an invalid size from user
space. Fix it by passing only sector-based quantities to overlaps().

Signed-off-by: Andre Noll <maan@systemlinux.org>
---
 drivers/md/md.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 95466bb..1bfa1f2 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -2143,8 +2143,8 @@ rdev_size_store(mdk_rdev_t *rdev, const char *buf, size_t len)
 				if (test_bit(AllReserved, &rdev2->flags) ||
 				    (rdev->bdev == rdev2->bdev &&
 				     rdev != rdev2 &&
-				     overlaps(rdev->data_offset, rdev->size,
-					    rdev2->data_offset, rdev2->size))) {
+				     overlaps(rdev->data_offset, rdev->size * 2,
+					    rdev2->data_offset, rdev2->size * 2))) {
 					overlap = 1;
 					break;
 				}
-- 
1.5.3.8

[PATCH 2/4] md: Make super_type->rdev_size_change() take sector-based sizes.

[Andre Noll]

Also, change the type of the size parameter from unsigned long long to
sector_t and rename it to num_sectors.

Signed-off-by: Andre Noll <maan@systemlinux.org>
---
 drivers/md/md.c |   40 +++++++++++++++++++---------------------
 1 files changed, 19 insertions(+), 21 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 1bfa1f2..c8673f4 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -655,7 +655,7 @@ struct super_type  {
 	int		    (*validate_super)(mddev_t *mddev, mdk_rdev_t *rdev);
 	void		    (*sync_super)(mddev_t *mddev, mdk_rdev_t *rdev);
 	unsigned long long  (*rdev_size_change)(mdk_rdev_t *rdev,
-						unsigned long long size);
+						sector_t num_sectors);
 };
 
 /*
@@ -998,20 +998,19 @@ static void super_90_sync(mddev_t *mddev, mdk_rdev_t *rdev)
  * rdev_size_change for 0.90.0
  */
 static unsigned long long
-super_90_rdev_size_change(mdk_rdev_t *rdev, unsigned long long size)
+super_90_rdev_size_change(mdk_rdev_t *rdev, sector_t num_sectors)
 {
-	if (size && size < rdev->mddev->size)
+	if (num_sectors && num_sectors < rdev->mddev->size * 2)
 		return 0; /* component must fit device */
-	size *= 2; /* convert to sectors */
 	if (rdev->mddev->bitmap_offset)
 		return 0; /* can't move bitmap */
 	rdev->sb_start = calc_dev_sboffset(rdev->bdev);
-	if (!size || size > rdev->sb_start)
-		size = rdev->sb_start;
+	if (!num_sectors || num_sectors > rdev->sb_start)
+		num_sectors = rdev->sb_start;
 	md_super_write(rdev->mddev, rdev, rdev->sb_start, rdev->sb_size,
 		       rdev->sb_page);
 	md_super_wait(rdev->mddev);
-	return size/2; /* kB for sysfs */
+	return num_sectors / 2; /* kB for sysfs */
 }
 
 
@@ -1339,19 +1338,18 @@ static void super_1_sync(mddev_t *mddev, mdk_rdev_t *rdev)
 }
 
 static unsigned long long
-super_1_rdev_size_change(mdk_rdev_t *rdev, unsigned long long size)
+super_1_rdev_size_change(mdk_rdev_t *rdev, sector_t num_sectors)
 {
 	struct mdp_superblock_1 *sb;
-	unsigned long long max_size;
-	if (size && size < rdev->mddev->size)
+	sector_t max_sectors;
+	if (num_sectors && num_sectors < rdev->mddev->size * 2)
 		return 0; /* component must fit device */
-	size *= 2; /* convert to sectors */
 	if (rdev->sb_start < rdev->data_offset) {
 		/* minor versions 1 and 2; superblock before data */
-		max_size = (rdev->bdev->bd_inode->i_size >> 9);
-		max_size -= rdev->data_offset;
-		if (!size || size > max_size)
-			size = max_size;
+		max_sectors = rdev->bdev->bd_inode->i_size >> 9;
+		max_sectors -= rdev->data_offset;
+		if (!num_sectors || num_sectors > max_sectors)
+			num_sectors = max_sectors;
 	} else if (rdev->mddev->bitmap_offset) {
 		/* minor version 0 with bitmap we can't move */
 		return 0;
@@ -1360,19 +1358,19 @@ super_1_rdev_size_change(mdk_rdev_t *rdev, unsigned long long size)
 		sector_t sb_start;
 		sb_start = (rdev->bdev->bd_inode->i_size >> 9) - 8*2;
 		sb_start &= ~(sector_t)(4*2 - 1);
-		max_size = rdev->size*2 + sb_start - rdev->sb_start;
-		if (!size || size > max_size)
-			size = max_size;
+		max_sectors = rdev->size * 2 + sb_start - rdev->sb_start;
+		if (!num_sectors || num_sectors > max_sectors)
+			num_sectors = max_sectors;
 		rdev->sb_start = sb_start;
 	}
 	sb = (struct mdp_superblock_1 *) page_address(rdev->sb_page);
-	sb->data_size = cpu_to_le64(size);
+	sb->data_size = cpu_to_le64(num_sectors);
 	sb->super_offset = rdev->sb_start;
 	sb->sb_csum = calc_sb_1_csum(sb);
 	md_super_write(rdev->mddev, rdev, rdev->sb_start, rdev->sb_size,
 		       rdev->sb_page);
 	md_super_wait(rdev->mddev);
-	return size/2; /* kB for sysfs */
+	return num_sectors / 2; /* kB for sysfs */
 }
 
 static struct super_type super_types[] = {
@@ -2112,7 +2110,7 @@ rdev_size_store(mdk_rdev_t *rdev, const char *buf, size_t len)
 	if (my_mddev->pers && rdev->raid_disk >= 0) {
 		if (my_mddev->persistent) {
 			size = super_types[my_mddev->major_version].
-				rdev_size_change(rdev, size);
+				rdev_size_change(rdev, size * 2);
 			if (!size)
 				return -EBUSY;
 		} else if (!size) {
-- 
1.5.3.8

[PATCH 3/4] md: Make mddev->array_size sector-based.

[Andre Noll]

This patch renames the array_size field of struct mddev_s to array_sectors
and converts all instances to use units of 512 byte sectors instead of 1k
blocks.

Signed-off-by: Andre Noll <maan@systemlinux.org>
---
 drivers/md/faulty.c       |    2 +-
 drivers/md/linear.c       |    6 +++---
 drivers/md/md.c           |    8 ++++----
 drivers/md/multipath.c    |    2 +-
 drivers/md/raid0.c        |    8 ++++----
 drivers/md/raid1.c        |   10 +++++-----
 drivers/md/raid10.c       |    2 +-
 drivers/md/raid5.c        |   14 +++++++-------
 include/linux/raid/md_k.h |    2 +-
 9 files changed, 27 insertions(+), 27 deletions(-)

diff --git a/drivers/md/faulty.c b/drivers/md/faulty.c
index d107ddc..268547d 100644
--- a/drivers/md/faulty.c
+++ b/drivers/md/faulty.c
@@ -297,7 +297,7 @@ static int run(mddev_t *mddev)
 	rdev_for_each(rdev, tmp, mddev)
 		conf->rdev = rdev;
 
-	mddev->array_size = mddev->size;
+	mddev->array_sectors = mddev->size * 2;
 	mddev->private = conf;
 
 	reconfig(mddev, mddev->layout, -1);
diff --git a/drivers/md/linear.c b/drivers/md/linear.c
index ec921f5..57644a7 100644
--- a/drivers/md/linear.c
+++ b/drivers/md/linear.c
@@ -256,7 +256,7 @@ static int linear_run (mddev_t *mddev)
 	if (!conf)
 		return 1;
 	mddev->private = conf;
-	mddev->array_size = conf->array_size;
+	mddev->array_sectors = conf->array_size * 2;
 
 	blk_queue_merge_bvec(mddev->queue, linear_mergeable_bvec);
 	mddev->queue->unplug_fn = linear_unplug;
@@ -290,8 +290,8 @@ static int linear_add(mddev_t *mddev, mdk_rdev_t *rdev)
 	newconf->prev = mddev_to_conf(mddev);
 	mddev->private = newconf;
 	mddev->raid_disks++;
-	mddev->array_size = newconf->array_size;
-	set_capacity(mddev->gendisk, mddev->array_size << 1);
+	mddev->array_sectors = newconf->array_size * 2;
+	set_capacity(mddev->gendisk, mddev->array_sectors);
 	return 0;
 }
 
diff --git a/drivers/md/md.c b/drivers/md/md.c
index c8673f4..ba99af4 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -3703,7 +3703,7 @@ static int do_md_run(mddev_t * mddev)
 	if (mddev->flags)
 		md_update_sb(mddev, 0);
 
-	set_capacity(disk, mddev->array_size<<1);
+	set_capacity(disk, mddev->array_sectors);
 
 	/* If we call blk_queue_make_request here, it will
 	 * re-initialise max_sectors etc which may have been
@@ -3904,7 +3904,7 @@ static int do_md_stop(mddev_t * mddev, int mode, int is_open)
 
 		export_array(mddev);
 
-		mddev->array_size = 0;
+		mddev->array_sectors = 0;
 		mddev->size = 0;
 		mddev->raid_disks = 0;
 		mddev->recovery_cp = 0;
@@ -4643,7 +4643,7 @@ static int update_size(mddev_t *mddev, sector_t num_sectors)
 		bdev = bdget_disk(mddev->gendisk, 0);
 		if (bdev) {
 			mutex_lock(&bdev->bd_inode->i_mutex);
-			i_size_write(bdev->bd_inode, (loff_t)mddev->array_size << 10);
+			i_size_write(bdev->bd_inode, (loff_t)mddev->array_sectors << 9);
 			mutex_unlock(&bdev->bd_inode->i_mutex);
 			bdput(bdev);
 		}
@@ -5390,7 +5390,7 @@ static int md_seq_show(struct seq_file *seq, void *v)
 		if (!list_empty(&mddev->disks)) {
 			if (mddev->pers)
 				seq_printf(seq, "\n      %llu blocks",
-					(unsigned long long)mddev->array_size);
+					(unsigned long long)mddev->array_sectors / 2);
 			else
 				seq_printf(seq, "\n      %llu blocks",
 					(unsigned long long)size);
diff --git a/drivers/md/multipath.c b/drivers/md/multipath.c
index 541cbe3..c4779cc 100644
--- a/drivers/md/multipath.c
+++ b/drivers/md/multipath.c
@@ -504,7 +504,7 @@ static int multipath_run (mddev_t *mddev)
 	/*
 	 * Ok, everything is just fine now
 	 */
-	mddev->array_size = mddev->size;
+	mddev->array_sectors = mddev->size * 2;
 
 	mddev->queue->unplug_fn = multipath_unplug;
 	mddev->queue->backing_dev_info.congested_fn = multipath_congested;
diff --git a/drivers/md/raid0.c b/drivers/md/raid0.c
index 914c04d..2f30ebd 100644
--- a/drivers/md/raid0.c
+++ b/drivers/md/raid0.c
@@ -293,16 +293,16 @@ static int raid0_run (mddev_t *mddev)
 		goto out_free_conf;
 
 	/* calculate array device size */
-	mddev->array_size = 0;
+	mddev->array_sectors = 0;
 	rdev_for_each(rdev, tmp, mddev)
-		mddev->array_size += rdev->size;
+		mddev->array_sectors += rdev->size * 2;
 
 	printk("raid0 : md_size is %llu blocks.\n", 
-		(unsigned long long)mddev->array_size);
+		(unsigned long long)mddev->array_sectors / 2);
 	printk("raid0 : conf->hash_spacing is %llu blocks.\n",
 		(unsigned long long)conf->hash_spacing);
 	{
-		sector_t s = mddev->array_size;
+		sector_t s = mddev->array_sectors / 2;
 		sector_t space = conf->hash_spacing;
 		int round;
 		conf->preshift = 0;
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 491dc2d..fc5b44b 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -2043,7 +2043,7 @@ static int run(mddev_t *mddev)
 	/*
 	 * Ok, everything is just fine now
 	 */
-	mddev->array_size = mddev->size;
+	mddev->array_sectors = mddev->size * 2;
 
 	mddev->queue->unplug_fn = raid1_unplug;
 	mddev->queue->backing_dev_info.congested_fn = raid1_congested;
@@ -2105,14 +2105,14 @@ static int raid1_resize(mddev_t *mddev, sector_t sectors)
 	 * any io in the removed space completes, but it hardly seems
 	 * worth it.
 	 */
-	mddev->array_size = sectors>>1;
-	set_capacity(mddev->gendisk, mddev->array_size << 1);
+	mddev->array_sectors = sectors;
+	set_capacity(mddev->gendisk, mddev->array_sectors);
 	mddev->changed = 1;
-	if (mddev->array_size > mddev->size && mddev->recovery_cp == MaxSector) {
+	if (mddev->array_sectors / 2 > mddev->size && mddev->recovery_cp == MaxSector) {
 		mddev->recovery_cp = mddev->size << 1;
 		set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
 	}
-	mddev->size = mddev->array_size;
+	mddev->size = mddev->array_sectors / 2;
 	mddev->resync_max_sectors = sectors;
 	return 0;
 }
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index df08a9f..2acea40 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -2164,7 +2164,7 @@ static int run(mddev_t *mddev)
 	/*
 	 * Ok, everything is just fine now
 	 */
-	mddev->array_size = size << (conf->chunk_shift-1);
+	mddev->array_sectors = size << conf->chunk_shift;
 	mddev->resync_max_sectors = size << conf->chunk_shift;
 
 	mddev->queue->unplug_fn = raid10_unplug;
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 8f4c70a..1e1a217 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -3540,7 +3540,7 @@ static sector_t reshape_request(mddev_t *mddev, sector_t sector_nr, int *skipped
 			    j == raid6_next_disk(sh->pd_idx, sh->disks))
 				continue;
 			s = compute_blocknr(sh, j);
-			if (s < (mddev->array_size<<1)) {
+			if (s < mddev->array_sectors) {
 				skipped = 1;
 				continue;
 			}
@@ -4189,7 +4189,7 @@ static int run(mddev_t *mddev)
 	mddev->queue->backing_dev_info.congested_data = mddev;
 	mddev->queue->backing_dev_info.congested_fn = raid5_congested;
 
-	mddev->array_size =  mddev->size * (conf->previous_raid_disks -
+	mddev->array_sectors = 2 * mddev->size * (conf->previous_raid_disks -
 					    conf->max_degraded);
 
 	blk_queue_merge_bvec(mddev->queue, raid5_mergeable_bvec);
@@ -4413,8 +4413,8 @@ static int raid5_resize(mddev_t *mddev, sector_t sectors)
 	raid5_conf_t *conf = mddev_to_conf(mddev);
 
 	sectors &= ~((sector_t)mddev->chunk_size/512 - 1);
-	mddev->array_size = (sectors * (mddev->raid_disks-conf->max_degraded))>>1;
-	set_capacity(mddev->gendisk, mddev->array_size << 1);
+	mddev->array_sectors = sectors * mddev->raid_disks-conf->max_degraded;
+	set_capacity(mddev->gendisk, mddev->array_sectors);
 	mddev->changed = 1;
 	if (sectors/2  > mddev->size && mddev->recovery_cp == MaxSector) {
 		mddev->recovery_cp = mddev->size << 1;
@@ -4547,15 +4547,15 @@ static void end_reshape(raid5_conf_t *conf)
 	struct block_device *bdev;
 
 	if (!test_bit(MD_RECOVERY_INTR, &conf->mddev->recovery)) {
-		conf->mddev->array_size = conf->mddev->size *
+		conf->mddev->array_sectors = 2 * conf->mddev->size *
 			(conf->raid_disks - conf->max_degraded);
-		set_capacity(conf->mddev->gendisk, conf->mddev->array_size << 1);
+		set_capacity(conf->mddev->gendisk, conf->mddev->array_sectors);
 		conf->mddev->changed = 1;
 
 		bdev = bdget_disk(conf->mddev->gendisk, 0);
 		if (bdev) {
 			mutex_lock(&bdev->bd_inode->i_mutex);
-			i_size_write(bdev->bd_inode, (loff_t)conf->mddev->array_size << 10);
+			i_size_write(bdev->bd_inode, (loff_t)conf->mddev->array_sectors << 9);
 			mutex_unlock(&bdev->bd_inode->i_mutex);
 			bdput(bdev);
 		}
diff --git a/include/linux/raid/md_k.h b/include/linux/raid/md_k.h
index e37aaa4..6f72b47 100644
--- a/include/linux/raid/md_k.h
+++ b/include/linux/raid/md_k.h
@@ -150,7 +150,7 @@ struct mddev_s
 	int				raid_disks;
 	int				max_disks;
 	sector_t			size; /* used size of component devices */
-	sector_t			array_size; /* exported array size */
+	sector_t			array_sectors; /* exported array size */
 	__u64				events;
 
 	char				uuid[16];
-- 
1.5.3.8

[PATCH 4/4] md: linear: Make array_size sector-based and rename it to array_sectors.

[Andre Noll]

Signed-off-by: Andre Noll <maan@systemlinux.org>
---
 drivers/md/linear.c         |   16 ++++++++--------
 include/linux/raid/linear.h |    2 +-
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/md/linear.c b/drivers/md/linear.c
index 57644a7..1cafaa9 100644
--- a/drivers/md/linear.c
+++ b/drivers/md/linear.c
@@ -120,7 +120,7 @@ static linear_conf_t *linear_conf(mddev_t *mddev, int raid_disks)
 		return NULL;
 
 	cnt = 0;
-	conf->array_size = 0;
+	conf->array_sectors = 0;
 
 	rdev_for_each(rdev, tmp, mddev) {
 		int j = rdev->raid_disk;
@@ -144,7 +144,7 @@ static linear_conf_t *linear_conf(mddev_t *mddev, int raid_disks)
 			blk_queue_max_sectors(mddev->queue, PAGE_SIZE>>9);
 
 		disk->size = rdev->size;
-		conf->array_size += rdev->size;
+		conf->array_sectors += rdev->size * 2;
 
 		cnt++;
 	}
@@ -153,7 +153,7 @@ static linear_conf_t *linear_conf(mddev_t *mddev, int raid_disks)
 		goto out;
 	}
 
-	min_spacing = conf->array_size;
+	min_spacing = conf->array_sectors / 2;
 	sector_div(min_spacing, PAGE_SIZE/sizeof(struct dev_info *));
 
 	/* min_spacing is the minimum spacing that will fit the hash
@@ -162,7 +162,7 @@ static linear_conf_t *linear_conf(mddev_t *mddev, int raid_disks)
 	 * that is larger than min_spacing as use the size of that as
 	 * the actual spacing
 	 */
-	conf->hash_spacing = conf->array_size;
+	conf->hash_spacing = conf->array_sectors / 2;
 	for (i=0; i < cnt-1 ; i++) {
 		sector_t sz = 0;
 		int j;
@@ -192,7 +192,7 @@ static linear_conf_t *linear_conf(mddev_t *mddev, int raid_disks)
 		unsigned round;
 		unsigned long base;
 
-		sz = conf->array_size >> conf->preshift;
+		sz = conf->array_sectors >> (conf->preshift + 1);
 		sz += 1; /* force round-up */
 		base = conf->hash_spacing >> conf->preshift;
 		round = sector_div(sz, base);
@@ -219,7 +219,7 @@ static linear_conf_t *linear_conf(mddev_t *mddev, int raid_disks)
 	curr_offset = 0;
 	i = 0;
 	for (curr_offset = 0;
-	     curr_offset < conf->array_size;
+	     curr_offset < conf->array_sectors / 2;
 	     curr_offset += conf->hash_spacing) {
 
 		while (i < raid_disks-1 &&
@@ -256,7 +256,7 @@ static int linear_run (mddev_t *mddev)
 	if (!conf)
 		return 1;
 	mddev->private = conf;
-	mddev->array_sectors = conf->array_size * 2;
+	mddev->array_sectors = conf->array_sectors;
 
 	blk_queue_merge_bvec(mddev->queue, linear_mergeable_bvec);
 	mddev->queue->unplug_fn = linear_unplug;
@@ -290,7 +290,7 @@ static int linear_add(mddev_t *mddev, mdk_rdev_t *rdev)
 	newconf->prev = mddev_to_conf(mddev);
 	mddev->private = newconf;
 	mddev->raid_disks++;
-	mddev->array_sectors = newconf->array_size * 2;
+	mddev->array_sectors = newconf->array_sectors;
 	set_capacity(mddev->gendisk, mddev->array_sectors);
 	return 0;
 }
diff --git a/include/linux/raid/linear.h b/include/linux/raid/linear.h
index ba15469..7e37511 100644
--- a/include/linux/raid/linear.h
+++ b/include/linux/raid/linear.h
@@ -16,7 +16,7 @@ struct linear_private_data
 	struct linear_private_data *prev;	/* earlier version */
 	dev_info_t		**hash_table;
 	sector_t		hash_spacing;
-	sector_t		array_size;
+	sector_t		array_sectors;
 	int			preshift; /* shift before dividing by hash_spacing */
 	dev_info_t		disks[0];
 };
-- 
1.5.3.8

Re: [PATCH 3/4] md: Make mddev->array_size sector-based.

[Neil Brown]

On Friday July 18, maan@systemlinux.org wrote:
> index 8f4c70a..1e1a217 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -4413,8 +4413,8 @@ static int raid5_resize(mddev_t *mddev, sector_t sectors)
>  	raid5_conf_t *conf = mddev_to_conf(mddev);
>  
>  	sectors &= ~((sector_t)mddev->chunk_size/512 - 1);
> -	mddev->array_size = (sectors * (mddev->raid_disks-conf->max_degraded))>>1;
> -	set_capacity(mddev->gendisk, mddev->array_size << 1);
> +	mddev->array_sectors = sectors * mddev->raid_disks-conf->max_degraded;
> +	set_capacity(mddev->gendisk, mddev->array_sectors);
>  	mddev->changed = 1;
>  	if (sectors/2  > mddev->size && mddev->recovery_cp == MaxSector) {
>  		mddev->recovery_cp = mddev->size << 1;

Can you spot the bug in this chunk?

There is a subtraction in there which is no longer inside parentheses,
so the multiplication takes precedence.
I've fixed it and put spaces around the '-' to make it more obvious.

The rest are OK.

For future patches, could you please:

1/ run ./scripts/checkpatch.pl on them and fix the warnings, mostly
   line length issues in this case.
2/ run the mdadm test suite on a kernel with the patches applied.
   In an mdadm source directory,
        make test
        sudo sh test

   You probably need
        echo 2000 > /proc/sys/dev/raid/speed_limit_max
   first.. I should make that part of the test code one day.

   Interpreting the errors can be a bit tricky, and while seeing
   complete success doesn't prove it is bug free, it is encouraging, and
   did catch the above bug.
   Note that you cannot use this test suite on a machine that has
   active md arrays.

Thanks,
NeilBrown

Re: [PATCH 3/4] md: Make mddev->array_size sector-based.

[Andre Noll]

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

On 20:29, Neil Brown wrote:
> On Friday July 18, maan@systemlinux.org wrote:
> > index 8f4c70a..1e1a217 100644
> > --- a/drivers/md/raid5.c
> > +++ b/drivers/md/raid5.c
> > @@ -4413,8 +4413,8 @@ static int raid5_resize(mddev_t *mddev, sector_t sectors)
> >  	raid5_conf_t *conf = mddev_to_conf(mddev);
> >  
> >  	sectors &= ~((sector_t)mddev->chunk_size/512 - 1);
> > -	mddev->array_size = (sectors * (mddev->raid_disks-conf->max_degraded))>>1;
> > -	set_capacity(mddev->gendisk, mddev->array_size << 1);
> > +	mddev->array_sectors = sectors * mddev->raid_disks-conf->max_degraded;
> > +	set_capacity(mddev->gendisk, mddev->array_sectors);
> >  	mddev->changed = 1;
> >  	if (sectors/2  > mddev->size && mddev->recovery_cp == MaxSector) {
> >  		mddev->recovery_cp = mddev->size << 1;
> 
> Can you spot the bug in this chunk?
> 
> There is a subtraction in there which is no longer inside parentheses,
> so the multiplication takes precedence.

Uhuhu.. Such bugs are hard to spot for the human eye.

> I've fixed it and put spaces around the '-' to make it more obvious.

Thanks.

> The rest are OK.

Glad to hear that.

> For future patches, could you please:
> 
> 1/ run ./scripts/checkpatch.pl on them and fix the warnings, mostly
>    line length issues in this case.
> 2/ run the mdadm test suite on a kernel with the patches applied.

Will do that before sending the next batch (probably tomorrow).

Thanks
Andre
-- 
The only person who always got his work done by Friday was Robinson Crusoe

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]