[PATCH 0/6] md: More sector_t conversions -- intro

[PATCH 0/6] md: More sector_t conversions -- intro

[Andre Noll]

Hi

this patch set against the for-next tree as of last Friday converts a
couple of remaining internal representation of md sizes to sector_t,
fixes a bug in the reshape code and adds a couple of trivial cleanups.

Patch #1 converts mddev->chunk_size which was previously stored
in bytes.  Patch #2 fixes what I believe is a bug in super_1_sync()
and might be a candiate for -stable. Patches #3 and #4 are further
straight-forward conversions that deal with mddev->new_chunk,
conf->chunk_size and conf->prev_chunk. Patches #5 and #6 are pure
cleanups.

All patches pass the tests of the mdadm test suite except
07reshape5intr which appears fail also for the vanilla for-next tree.

Please review and consider for inclusion into for-next.

Thanks
Andre

[PATCH 1/6] md: Make mddev->chunk_size sector-based.

[Andre Noll]

This patch changes the type of the chunk_size field from int to
sector_t and renames it to chunk_sectors with the implied change of
semantics. Since

	is_power_of_2(chunk_size) = is_power_of_2(chunk_sectors << 9)
				  = is_power_of_2(chunk_sectors)

these bits don't need an adjustment for the shift.

Signed-off-by: Andre Noll <maan@systemlinux.org>
---
 drivers/md/linear.c |    2 +-
 drivers/md/md.c     |   49 +++++++++++++++++++++++++------------------------
 drivers/md/md.h     |    2 +-
 drivers/md/raid0.c  |   29 +++++++++++++++--------------
 drivers/md/raid1.c  |    4 ++--
 drivers/md/raid10.c |   15 ++++++++-------
 drivers/md/raid5.c  |   41 ++++++++++++++++++++++-------------------
 7 files changed, 74 insertions(+), 68 deletions(-)

diff --git a/drivers/md/linear.c b/drivers/md/linear.c
index 189cc54..ead1051 100644
--- a/drivers/md/linear.c
+++ b/drivers/md/linear.c
@@ -305,7 +305,7 @@ static int linear_make_request (struct request_queue *q, struct bio *bio)
 static void linear_status (struct seq_file *seq, mddev_t *mddev)
 {
 
-	seq_printf(seq, " %dk rounding", mddev->chunk_size/1024);
+	seq_printf(seq, " %dk rounding", (int)mddev->chunk_sectors / 2);
 }
 
 
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 149eb0d..48bdbdf 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -869,7 +869,7 @@ static int super_90_validate(mddev_t *mddev, mdk_rdev_t *rdev)
 		mddev->minor_version = sb->minor_version;
 		mddev->patch_version = sb->patch_version;
 		mddev->external = 0;
-		mddev->chunk_size = sb->chunk_size;
+		mddev->chunk_sectors = sb->chunk_size >> 9;
 		mddev->ctime = sb->ctime;
 		mddev->utime = sb->utime;
 		mddev->level = sb->level;
@@ -892,7 +892,7 @@ static int super_90_validate(mddev_t *mddev, mdk_rdev_t *rdev)
 			mddev->delta_disks = 0;
 			mddev->new_level = mddev->level;
 			mddev->new_layout = mddev->layout;
-			mddev->new_chunk = mddev->chunk_size;
+			mddev->new_chunk = mddev->chunk_sectors << 9;
 		}
 
 		if (sb->state & (1<<MD_SB_CLEAN))
@@ -1021,7 +1021,7 @@ static void super_90_sync(mddev_t *mddev, mdk_rdev_t *rdev)
 		sb->recovery_cp = 0;
 
 	sb->layout = mddev->layout;
-	sb->chunk_size = mddev->chunk_size;
+	sb->chunk_size = mddev->chunk_sectors << 9;
 
 	if (mddev->bitmap && mddev->bitmap_file == NULL)
 		sb->state |= (1<<MD_SB_BITMAP_PRESENT);
@@ -1274,7 +1274,7 @@ static int super_1_validate(mddev_t *mddev, mdk_rdev_t *rdev)
 		mddev->major_version = 1;
 		mddev->patch_version = 0;
 		mddev->external = 0;
-		mddev->chunk_size = le32_to_cpu(sb->chunksize) << 9;
+		mddev->chunk_sectors = le32_to_cpu(sb->chunksize);
 		mddev->ctime = le64_to_cpu(sb->ctime) & ((1ULL << 32)-1);
 		mddev->utime = le64_to_cpu(sb->utime) & ((1ULL << 32)-1);
 		mddev->level = le32_to_cpu(sb->level);
@@ -1306,7 +1306,7 @@ static int super_1_validate(mddev_t *mddev, mdk_rdev_t *rdev)
 			mddev->delta_disks = 0;
 			mddev->new_level = mddev->level;
 			mddev->new_layout = mddev->layout;
-			mddev->new_chunk = mddev->chunk_size;
+			mddev->new_chunk = mddev->chunk_sectors << 9;
 		}
 
 	} else if (mddev->pers == NULL) {
@@ -2746,7 +2746,7 @@ level_store(mddev_t *mddev, const char *buf, size_t len)
 	if (IS_ERR(priv)) {
 		mddev->new_level = mddev->level;
 		mddev->new_layout = mddev->layout;
-		mddev->new_chunk = mddev->chunk_size;
+		mddev->new_chunk = mddev->chunk_sectors << 9;
 		mddev->raid_disks -= mddev->delta_disks;
 		mddev->delta_disks = 0;
 		module_put(pers->owner);
@@ -2764,7 +2764,7 @@ level_store(mddev_t *mddev, const char *buf, size_t len)
 	strlcpy(mddev->clevel, pers->name, sizeof(mddev->clevel));
 	mddev->level = mddev->new_level;
 	mddev->layout = mddev->new_layout;
-	mddev->chunk_size = mddev->new_chunk;
+	mddev->chunk_sectors = mddev->new_chunk >> 9;
 	mddev->delta_disks = 0;
 	pers->run(mddev);
 	mddev_resume(mddev);
@@ -2857,10 +2857,10 @@ static ssize_t
 chunk_size_show(mddev_t *mddev, char *page)
 {
 	if (mddev->reshape_position != MaxSector &&
-	    mddev->chunk_size != mddev->new_chunk)
+	    mddev->chunk_sectors << 9 != mddev->new_chunk)
 		return sprintf(page, "%d (%d)\n", mddev->new_chunk,
-			       mddev->chunk_size);
-	return sprintf(page, "%d\n", mddev->chunk_size);
+			       (int)mddev->chunk_sectors << 9);
+	return sprintf(page, "%d\n", (int)mddev->chunk_sectors << 9);
 }
 
 static ssize_t
@@ -2882,7 +2882,7 @@ chunk_size_store(mddev_t *mddev, const char *buf, size_t len)
 	} else {
 		mddev->new_chunk = n;
 		if (mddev->reshape_position == MaxSector)
-			mddev->chunk_size = n;
+			mddev->chunk_sectors = n >> 9;
 	}
 	return len;
 }
@@ -3520,9 +3520,9 @@ min_sync_store(mddev_t *mddev, const char *buf, size_t len)
 		return -EBUSY;
 
 	/* Must be a multiple of chunk_size */
-	if (mddev->chunk_size) {
+	if (mddev->chunk_sectors) {
 		sector_t temp = min;
-		if (sector_div(temp, (mddev->chunk_size>>9)))
+		if (sector_div(temp, mddev->chunk_sectors))
 			return -EINVAL;
 	}
 	mddev->resync_min = min;
@@ -3558,9 +3558,9 @@ max_sync_store(mddev_t *mddev, const char *buf, size_t len)
 			return -EBUSY;
 
 		/* Must be a multiple of chunk_size */
-		if (mddev->chunk_size) {
+		if (mddev->chunk_sectors) {
 			sector_t temp = max;
-			if (sector_div(temp, (mddev->chunk_size>>9)))
+			if (sector_div(temp, mddev->chunk_sectors))
 				return -EINVAL;
 		}
 		mddev->resync_max = max;
@@ -3651,7 +3651,7 @@ reshape_position_store(mddev_t *mddev, const char *buf, size_t len)
 	mddev->delta_disks = 0;
 	mddev->new_level = mddev->level;
 	mddev->new_layout = mddev->layout;
-	mddev->new_chunk = mddev->chunk_size;
+	mddev->new_chunk = mddev->chunk_sectors << 9;
 	return len;
 }
 
@@ -3993,7 +3993,7 @@ static int do_md_run(mddev_t * mddev)
 		analyze_sbs(mddev);
 	}
 
-	chunk_size = mddev->chunk_size;
+	chunk_size = mddev->chunk_sectors << 9;
 
 	if (chunk_size) {
 		if (chunk_size > MAX_CHUNK_SIZE) {
@@ -4392,7 +4392,7 @@ static int do_md_stop(mddev_t * mddev, int mode, int is_open)
 		mddev->flags = 0;
 		mddev->ro = 0;
 		mddev->metadata_type[0] = 0;
-		mddev->chunk_size = 0;
+		mddev->chunk_sectors = 0;
 		mddev->ctime = mddev->utime = 0;
 		mddev->layout = 0;
 		mddev->max_disks = 0;
@@ -4605,7 +4605,7 @@ static int get_array_info(mddev_t * mddev, void __user * arg)
 	info.spare_disks   = spare;
 
 	info.layout        = mddev->layout;
-	info.chunk_size    = mddev->chunk_size;
+	info.chunk_size    = mddev->chunk_sectors << 9;
 
 	if (copy_to_user(arg, &info, sizeof(info)))
 		return -EFAULT;
@@ -4830,7 +4830,8 @@ static int add_new_disk(mddev_t * mddev, mdu_disk_info_t *info)
 			rdev->sb_start = rdev->bdev->bd_inode->i_size / 512;
 		} else 
 			rdev->sb_start = calc_dev_sboffset(rdev->bdev);
-		rdev->sectors = calc_num_sectors(rdev, mddev->chunk_size);
+		rdev->sectors = calc_num_sectors(rdev,
+						 mddev->chunk_sectors << 9);
 
 		err = bind_rdev_to_array(rdev, mddev);
 		if (err) {
@@ -4900,7 +4901,7 @@ static int hot_add_disk(mddev_t * mddev, dev_t dev)
 	else
 		rdev->sb_start = rdev->bdev->bd_inode->i_size / 512;
 
-	rdev->sectors = calc_num_sectors(rdev, mddev->chunk_size);
+	rdev->sectors = calc_num_sectors(rdev, mddev->chunk_sectors << 9);
 
 	if (test_bit(Faulty, &rdev->flags)) {
 		printk(KERN_WARNING 
@@ -5049,7 +5050,7 @@ static int set_array_info(mddev_t * mddev, mdu_array_info_t *info)
 	mddev->external	     = 0;
 
 	mddev->layout        = info->layout;
-	mddev->chunk_size    = info->chunk_size;
+	mddev->chunk_sectors = info->chunk_size >> 9;
 
 	mddev->max_disks     = MD_SB_DISKS;
 
@@ -5068,7 +5069,7 @@ static int set_array_info(mddev_t * mddev, mdu_array_info_t *info)
 	get_random_bytes(mddev->uuid, 16);
 
 	mddev->new_level = mddev->level;
-	mddev->new_chunk = mddev->chunk_size;
+	mddev->new_chunk = mddev->chunk_sectors << 9;
 	mddev->new_layout = mddev->layout;
 	mddev->delta_disks = 0;
 
@@ -5178,7 +5179,7 @@ static int update_array_info(mddev_t *mddev, mdu_array_info_t *info)
 	    mddev->level         != info->level         ||
 /*	    mddev->layout        != info->layout        || */
 	    !mddev->persistent	 != info->not_persistent||
-	    mddev->chunk_size    != info->chunk_size    ||
+	    mddev->chunk_sectors != info->chunk_size >> 9 ||
 	    /* ignore bottom 8 bits of state, and allow SB_BITMAP_PRESENT to change */
 	    ((state^info->state) & 0xfffffe00)
 		)
diff --git a/drivers/md/md.h b/drivers/md/md.h
index 8227ab9..7cbb895 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -145,7 +145,7 @@ struct mddev_s
 	int 				external;	/* metadata is
 							 * managed externally */
 	char				metadata_type[17]; /* externally set*/
-	int				chunk_size;
+	sector_t			chunk_sectors;
 	time_t				ctime, utime;
 	int				level, layout;
 	char				clevel[16];
diff --git a/drivers/md/raid0.c b/drivers/md/raid0.c
index d57d8ab..7d680bc 100644
--- a/drivers/md/raid0.c
+++ b/drivers/md/raid0.c
@@ -240,10 +240,10 @@ static int create_strip_zones(mddev_t *mddev)
 	 * now since we have the hard sector sizes, we can make sure
 	 * chunk size is a multiple of that sector size
 	 */
-	if (mddev->chunk_size % mddev->queue->hardsect_size) {
+	if ((mddev->chunk_sectors << 9) % mddev->queue->hardsect_size) {
 		printk(KERN_ERR "%s chunk_size of %d not valid\n",
 		       mdname(mddev),
-		       mddev->chunk_size);
+		       (int)mddev->chunk_sectors << 9);
 		goto abort;
 	}
 	printk(KERN_INFO "raid0: done.\n");
@@ -272,10 +272,10 @@ static int raid0_mergeable_bvec(struct request_queue *q,
 	mddev_t *mddev = q->queuedata;
 	sector_t sector = bvm->bi_sector + get_start_sect(bvm->bi_bdev);
 	int max;
-	unsigned int chunk_sectors = mddev->chunk_size >> 9;
+	unsigned int chunk_sectors = mddev->chunk_sectors;
 	unsigned int bio_sectors = bvm->bi_size >> 9;
 
-	if (is_power_of_2(mddev->chunk_size))
+	if (is_power_of_2(mddev->chunk_sectors))
 		max =  (chunk_sectors - ((sector & (chunk_sectors-1))
 						+ bio_sectors)) << 9;
 	else
@@ -306,12 +306,12 @@ static int raid0_run(mddev_t *mddev)
 {
 	int ret;
 
-	if (mddev->chunk_size == 0 ||
-	    !is_power_of_2(mddev->chunk_size)) {
+	if (mddev->chunk_sectors == 0 ||
+	    !is_power_of_2(mddev->chunk_sectors)) {
 		printk(KERN_ERR "md/raid0: chunk size must be a power of 2.\n");
 		return -EINVAL;
 	}
-	blk_queue_max_sectors(mddev->queue, mddev->chunk_size >> 9);
+	blk_queue_max_sectors(mddev->queue, mddev->chunk_sectors);
 	mddev->queue->queue_lock = &mddev->queue->__queue_lock;
 
 	ret = create_strip_zones(mddev);
@@ -333,7 +333,8 @@ static int raid0_run(mddev_t *mddev)
 	 * chunksize should be used in that case.
 	 */
 	{
-		int stripe = mddev->raid_disks * mddev->chunk_size / PAGE_SIZE;
+		int stripe = mddev->raid_disks *
+			(mddev->chunk_sectors << 9) / PAGE_SIZE;
 		if (mddev->queue->backing_dev_info.ra_pages < 2* stripe)
 			mddev->queue->backing_dev_info.ra_pages = 2* stripe;
 	}
@@ -384,9 +385,9 @@ static mdk_rdev_t *map_sector(mddev_t *mddev, struct strip_zone *zone,
 	unsigned int sect_in_chunk;
 	sector_t chunk;
 	raid0_conf_t *conf = mddev->private;
-	unsigned int chunk_sects = mddev->chunk_size >> 9;
+	unsigned int chunk_sects = mddev->chunk_sectors;
 
-	if (is_power_of_2(mddev->chunk_size)) {
+	if (is_power_of_2(mddev->chunk_sectors)) {
 		int chunksect_bits = ffz(~chunk_sects);
 		/* find the sector offset inside the chunk */
 		sect_in_chunk  = sector & (chunk_sects - 1);
@@ -416,7 +417,7 @@ static mdk_rdev_t *map_sector(mddev_t *mddev, struct strip_zone *zone,
 static inline int is_io_in_chunk_boundary(mddev_t *mddev,
 			unsigned int chunk_sects, struct bio *bio)
 {
-	if (likely(is_power_of_2(mddev->chunk_size))) {
+	if (likely(is_power_of_2(mddev->chunk_sectors))) {
 		return chunk_sects >= ((bio->bi_sector & (chunk_sects-1))
 					+ (bio->bi_size >> 9));
 	} else{
@@ -447,7 +448,7 @@ static int raid0_make_request(struct request_queue *q, struct bio *bio)
 		      bio_sectors(bio));
 	part_stat_unlock();
 
-	chunk_sects = mddev->chunk_size >> 9;
+	chunk_sects = mddev->chunk_sectors;
 	if (unlikely(!is_io_in_chunk_boundary(mddev, chunk_sects, bio))) {
 		sector_t sector = bio->bi_sector;
 		struct bio_pair *bp;
@@ -458,7 +459,7 @@ static int raid0_make_request(struct request_queue *q, struct bio *bio)
 		/* This is a one page bio that upper layers
 		 * refuse to split for us, so we need to split it.
 		 */
-		if (likely(is_power_of_2(mddev->chunk_size)))
+		if (likely(is_power_of_2(mddev->chunk_sectors)))
 			bp = bio_split(bio, chunk_sects - (sector &
 							   (chunk_sects-1)));
 		else
@@ -522,7 +523,7 @@ static void raid0_status(struct seq_file *seq, mddev_t *mddev)
 		zone_start = conf->strip_zone[j].zone_end;
 	}
 #endif
-	seq_printf(seq, " %dk chunks", mddev->chunk_size/1024);
+	seq_printf(seq, " %dk chunks", (int)mddev->chunk_sectors / 2);
 	return;
 }
 
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 2c8d9e6..b56f075 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -2161,10 +2161,10 @@ static int raid1_reshape(mddev_t *mddev)
 	int d, d2, err;
 
 	/* Cannot change chunk_size, layout, or level */
-	if (mddev->chunk_size != mddev->new_chunk ||
+	if (mddev->chunk_sectors << 9 != mddev->new_chunk ||
 	    mddev->layout != mddev->new_layout ||
 	    mddev->level != mddev->new_level) {
-		mddev->new_chunk = mddev->chunk_size;
+		mddev->new_chunk = mddev->chunk_sectors << 9;
 		mddev->new_layout = mddev->layout;
 		mddev->new_level = mddev->level;
 		return -EINVAL;
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 33131fc..d75cb96 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -461,7 +461,7 @@ static int raid10_mergeable_bvec(struct request_queue *q,
 	mddev_t *mddev = q->queuedata;
 	sector_t sector = bvm->bi_sector + get_start_sect(bvm->bi_bdev);
 	int max;
-	unsigned int chunk_sectors = mddev->chunk_size >> 9;
+	unsigned int chunk_sectors = mddev->chunk_sectors;
 	unsigned int bio_sectors = bvm->bi_size >> 9;
 
 	max =  (chunk_sectors - ((sector & (chunk_sectors - 1)) + bio_sectors)) << 9;
@@ -985,7 +985,7 @@ static void status(struct seq_file *seq, mddev_t *mddev)
 	int i;
 
 	if (conf->near_copies < conf->raid_disks)
-		seq_printf(seq, " %dK chunks", mddev->chunk_size/1024);
+		seq_printf(seq, " %dK chunks", (int)mddev->chunk_sectors / 2);
 	if (conf->near_copies > 1)
 		seq_printf(seq, " %d near-copies", conf->near_copies);
 	if (conf->far_copies > 1) {
@@ -2050,8 +2050,8 @@ static int run(mddev_t *mddev)
 	int nc, fc, fo;
 	sector_t stride, size;
 
-	if (mddev->chunk_size < PAGE_SIZE ||
-	    !is_power_of_2(mddev->chunk_size)) {
+	if (mddev->chunk_sectors << 9 < PAGE_SIZE ||
+	    !is_power_of_2(mddev->chunk_sectors)) {
 		printk(KERN_ERR "md/raid10: chunk size must be "
 		       "at least PAGE_SIZE(%ld) and be a power of 2.\n", PAGE_SIZE);
 		return -EINVAL;
@@ -2096,8 +2096,8 @@ static int run(mddev_t *mddev)
 	conf->far_copies = fc;
 	conf->copies = nc*fc;
 	conf->far_offset = fo;
-	conf->chunk_mask = (sector_t)(mddev->chunk_size>>9)-1;
-	conf->chunk_shift = ffz(~mddev->chunk_size) - 9;
+	conf->chunk_mask = mddev->chunk_sectors - 1;
+	conf->chunk_shift = ffz(~mddev->chunk_sectors);
 	size = mddev->dev_sectors >> conf->chunk_shift;
 	sector_div(size, fc);
 	size = size * conf->raid_disks;
@@ -2205,7 +2205,8 @@ static int run(mddev_t *mddev)
 	 * maybe...
 	 */
 	{
-		int stripe = conf->raid_disks * (mddev->chunk_size / PAGE_SIZE);
+		int stripe = conf->raid_disks *
+			((mddev->chunk_sectors << 9) / PAGE_SIZE);
 		stripe /= conf->near_copies;
 		if (mddev->queue->backing_dev_info.ra_pages < 2* stripe)
 			mddev->queue->backing_dev_info.ra_pages = 2* stripe;
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 4c9e602..85653e5 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -3352,13 +3352,13 @@ static int raid5_mergeable_bvec(struct request_queue *q,
 	mddev_t *mddev = q->queuedata;
 	sector_t sector = bvm->bi_sector + get_start_sect(bvm->bi_bdev);
 	int max;
-	unsigned int chunk_sectors = mddev->chunk_size >> 9;
+	unsigned int chunk_sectors = mddev->chunk_sectors;
 	unsigned int bio_sectors = bvm->bi_size >> 9;
 
 	if ((bvm->bi_rw & 1) == WRITE)
 		return biovec->bv_len; /* always allow writes to be mergeable */
 
-	if (mddev->new_chunk < mddev->chunk_size)
+	if (mddev->new_chunk < mddev->chunk_sectors << 9)
 		chunk_sectors = mddev->new_chunk >> 9;
 	max =  (chunk_sectors - ((sector & (chunk_sectors - 1)) + bio_sectors)) << 9;
 	if (max < 0) max = 0;
@@ -3372,10 +3372,10 @@ static int raid5_mergeable_bvec(struct request_queue *q,
 static int in_chunk_boundary(mddev_t *mddev, struct bio *bio)
 {
 	sector_t sector = bio->bi_sector + get_start_sect(bio->bi_bdev);
-	unsigned int chunk_sectors = mddev->chunk_size >> 9;
+	unsigned int chunk_sectors = mddev->chunk_sectors;
 	unsigned int bio_sectors = bio->bi_size >> 9;
 
-	if (mddev->new_chunk < mddev->chunk_size)
+	if (mddev->new_chunk < mddev->chunk_sectors << 9)
 		chunk_sectors = mddev->new_chunk >> 9;
 	return  chunk_sectors >=
 		((sector & (chunk_sectors - 1)) + bio_sectors);
@@ -3791,10 +3791,10 @@ static sector_t reshape_request(mddev_t *mddev, sector_t sector_nr, int *skipped
 	 * If old and new chunk sizes differ, we need to process the
 	 * largest of these
 	 */
-	if (mddev->new_chunk > mddev->chunk_size)
+	if (mddev->new_chunk > mddev->chunk_sectors << 9)
 		reshape_sectors = mddev->new_chunk / 512;
 	else
-		reshape_sectors = mddev->chunk_size / 512;
+		reshape_sectors = mddev->chunk_sectors;
 
 	/* we update the metadata when there is more than 3Meg
 	 * in the block range (that is rather arbitrary, should
@@ -4303,7 +4303,7 @@ raid5_size(mddev_t *mddev, sector_t sectors, int raid_disks)
 			raid_disks = conf->previous_raid_disks;
 	}
 
-	sectors &= ~((sector_t)mddev->chunk_size/512 - 1);
+	sectors &= ~(mddev->chunk_sectors - 1);
 	sectors &= ~((sector_t)mddev->new_chunk/512 - 1);
 	return sectors * (raid_disks - conf->max_degraded);
 }
@@ -4412,7 +4412,7 @@ static raid5_conf_t *setup_conf(mddev_t *mddev)
 	conf->max_nr_stripes = NR_STRIPES;
 	conf->reshape_progress = mddev->reshape_position;
 	if (conf->reshape_progress != MaxSector) {
-		conf->prev_chunk = mddev->chunk_size;
+		conf->prev_chunk = mddev->chunk_sectors << 9;
 		conf->prev_algo = mddev->layout;
 	}
 
@@ -4484,7 +4484,7 @@ static int run(mddev_t *mddev)
 		}
 		/* here_new is the stripe we will write to */
 		here_old = mddev->reshape_position;
-		sector_div(here_old, (mddev->chunk_size>>9)*
+		sector_div(here_old, mddev->chunk_sectors *
 			   (old_disks-max_degraded));
 		/* here_old is the first stripe that we might need to read
 		 * from */
@@ -4499,7 +4499,7 @@ static int run(mddev_t *mddev)
 	} else {
 		BUG_ON(mddev->level != mddev->new_level);
 		BUG_ON(mddev->layout != mddev->new_layout);
-		BUG_ON(mddev->chunk_size != mddev->new_chunk);
+		BUG_ON(mddev->chunk_sectors << 9 != mddev->new_chunk);
 		BUG_ON(mddev->delta_disks != 0);
 	}
 
@@ -4533,7 +4533,7 @@ static int run(mddev_t *mddev)
 	}
 
 	/* device size must be a multiple of chunk size */
-	mddev->dev_sectors &= ~(mddev->chunk_size / 512 - 1);
+	mddev->dev_sectors &= ~(mddev->chunk_sectors - 1);
 	mddev->resync_max_sectors = mddev->dev_sectors;
 
 	if (mddev->degraded > 0 &&
@@ -4582,7 +4582,7 @@ static int run(mddev_t *mddev)
 	{
 		int data_disks = conf->previous_raid_disks - conf->max_degraded;
 		int stripe = data_disks *
-			(mddev->chunk_size / PAGE_SIZE);
+			((mddev->chunk_sectors << 9) / PAGE_SIZE);
 		if (mddev->queue->backing_dev_info.ra_pages < 2 * stripe)
 			mddev->queue->backing_dev_info.ra_pages = 2 * stripe;
 	}
@@ -4679,7 +4679,8 @@ static void status(struct seq_file *seq, mddev_t *mddev)
 	raid5_conf_t *conf = (raid5_conf_t *) mddev->private;
 	int i;
 
-	seq_printf (seq, " level %d, %dk chunk, algorithm %d", mddev->level, mddev->chunk_size >> 10, mddev->layout);
+	seq_printf(seq, " level %d, %dk chunk, algorithm %d", mddev->level,
+		(int)mddev->chunk_sectors / 2, mddev->layout);
 	seq_printf (seq, " [%d/%d] [", conf->raid_disks, conf->raid_disks - mddev->degraded);
 	for (i = 0; i < conf->raid_disks; i++)
 		seq_printf (seq, "%s",
@@ -4827,7 +4828,7 @@ static int raid5_resize(mddev_t *mddev, sector_t sectors)
 	 * any io in the removed space completes, but it hardly seems
 	 * worth it.
 	 */
-	sectors &= ~((sector_t)mddev->chunk_size/512 - 1);
+	sectors &= ~(mddev->chunk_sectors - 1);
 	md_set_array_sectors(mddev, raid5_size(mddev, sectors,
 					       mddev->raid_disks));
 	if (mddev->array_sectors >
@@ -4850,7 +4851,7 @@ static int raid5_check_reshape(mddev_t *mddev)
 
 	if (mddev->delta_disks == 0 &&
 	    mddev->new_layout == mddev->layout &&
-	    mddev->new_chunk == mddev->chunk_size)
+	    mddev->new_chunk == mddev->chunk_sectors << 9)
 		return -EINVAL; /* nothing to do */
 	if (mddev->bitmap)
 		/* Cannot grow a bitmap yet */
@@ -4878,10 +4879,11 @@ static int raid5_check_reshape(mddev_t *mddev)
 	 * If the chunk size is greater, user-space should request more
 	 * stripe_heads first.
 	 */
-	if ((mddev->chunk_size / STRIPE_SIZE) * 4 > conf->max_nr_stripes ||
+	if (((mddev->chunk_sectors << 9) / STRIPE_SIZE) * 4
+		> conf->max_nr_stripes ||
 	    (mddev->new_chunk / STRIPE_SIZE) * 4 > conf->max_nr_stripes) {
 		printk(KERN_WARNING "raid5: reshape: not enough stripes.  Needed %lu\n",
-		       (max(mddev->chunk_size, mddev->new_chunk)
+		       (max((int)mddev->chunk_sectors << 9, mddev->new_chunk)
 			/ STRIPE_SIZE)*4);
 		return -ENOSPC;
 	}
@@ -5054,7 +5056,7 @@ static void raid5_finish_reshape(mddev_t *mddev)
 				raid5_remove_disk(mddev, d);
 		}
 		mddev->layout = conf->algorithm;
-		mddev->chunk_size = conf->chunk_size;
+		mddev->chunk_sectors = conf->chunk_size >> 9;
 		mddev->reshape_position = MaxSector;
 		mddev->delta_disks = 0;
 	}
@@ -5183,7 +5185,8 @@ static int raid5_reconfig(mddev_t *mddev, int new_layout, int new_chunk)
 		}
 		if (new_chunk > 0) {
 			conf->chunk_size = new_chunk;
-			mddev->chunk_size = mddev->new_chunk = new_chunk;
+			mddev->new_chunk = new_chunk;
+			mddev->chunk_sectors = new_chunk >> 9;
 		}
 		set_bit(MD_CHANGE_DEVS, &mddev->flags);
 		md_wakeup_thread(mddev->thread);
-- 
1.5.4.3

Re: [PATCH 1/6] md: Make mddev->chunk_size sector-based.

[Neil Brown]

On Monday May 25, maan@systemlinux.org wrote:
> This patch changes the type of the chunk_size field from int to
> sector_t and renames it to chunk_sectors with the implied change of
> semantics. Since
> 
> 	is_power_of_2(chunk_size) = is_power_of_2(chunk_sectors << 9)
> 				  = is_power_of_2(chunk_sectors)
> 
> these bits don't need an adjustment for the shift.

Thanks.

I don't really want chunk_sectors to be sector_t - it is a waste on
32bit, and would cause
> +		if (sector_div(temp, mddev->chunk_sectors))

to fail.
I've left it as 'int' and removed all the (int) casts and (sector_t)
casts.   I adjusted the comment too.

I also changed:
	if (mddev->chunk_sectors << 9 < PAGE_SIZE ||
to
	if (mddev->chunk_sectors < (PAGE_SIZE >> 9) ||
primarily because I need the brackets around the shift, even if the
C compiler doesn't.


Thanks,
NeilBrown

[PATCH 2/6] md: Fix a bug in super_1_sync().

[Andre Noll]

The new_chunk fields of both struct mddev_s and struct mdp_superblock_1
are measured in bytes. However, in super_1_sync(), mddev->new_chunk
is converted to sectors before it assigned to sb->new_chunk.

This results in a too small number being stored in the in-memory
image of the superblock which could confuse the reshape process.

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

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 48bdbdf..66e32e4 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -1402,7 +1402,7 @@ static void super_1_sync(mddev_t *mddev, mdk_rdev_t *rdev)
 		sb->new_layout = cpu_to_le32(mddev->new_layout);
 		sb->delta_disks = cpu_to_le32(mddev->delta_disks);
 		sb->new_level = cpu_to_le32(mddev->new_level);
-		sb->new_chunk = cpu_to_le32(mddev->new_chunk>>9);
+		sb->new_chunk = cpu_to_le32(mddev->new_chunk);
 	}
 
 	max_dev = 0;
-- 
1.5.4.3

Re: [PATCH 2/6] md: Fix a bug in super_1_sync().

[NeilBrown]

On Mon, May 25, 2009 7:27 pm, Andre Noll wrote:
> The new_chunk fields of both struct mddev_s and struct mdp_superblock_1
> are measured in bytes. However, in super_1_sync(), mddev->new_chunk
> is converted to sectors before it assigned to sb->new_chunk.
>
> This results in a too small number being stored in the in-memory
> image of the superblock which could confuse the reshape process.
>
> Signed-off-by: Andre Noll <maan@systemlinux.org>
> ---
>  drivers/md/md.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 48bdbdf..66e32e4 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -1402,7 +1402,7 @@ static void super_1_sync(mddev_t *mddev, mdk_rdev_t
> *rdev)
>  		sb->new_layout = cpu_to_le32(mddev->new_layout);
>  		sb->delta_disks = cpu_to_le32(mddev->delta_disks);
>  		sb->new_level = cpu_to_le32(mddev->new_level);
> -		sb->new_chunk = cpu_to_le32(mddev->new_chunk>>9);
> +		sb->new_chunk = cpu_to_le32(mddev->new_chunk);

I think the current code is correct.
For v1.x metadata, the chunk size (both chunksize and new_chunk) is
stored in sectors.  in 0.90 it is stored in bytes. (in DDF, it is stored
as the base-2 log of sectors, but that is not relevant for the kernel).

So as mddev->new_chunk is (still) in bytes, we need to shift by 9.
??

I'll sort through the rest of the patches tomorrow, but that look fine.

Thanks,
NeilBrown

Re: [PATCH 2/6] md: Fix a bug in super_1_sync().

[Andre Noll]

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

On 19:49, NeilBrown wrote:
> > diff --git a/drivers/md/md.c b/drivers/md/md.c
> > index 48bdbdf..66e32e4 100644
> > --- a/drivers/md/md.c
> > +++ b/drivers/md/md.c
> > @@ -1402,7 +1402,7 @@ static void super_1_sync(mddev_t *mddev, mdk_rdev_t
> > *rdev)
> >  		sb->new_layout = cpu_to_le32(mddev->new_layout);
> >  		sb->delta_disks = cpu_to_le32(mddev->delta_disks);
> >  		sb->new_level = cpu_to_le32(mddev->new_level);
> > -		sb->new_chunk = cpu_to_le32(mddev->new_chunk>>9);
> > +		sb->new_chunk = cpu_to_le32(mddev->new_chunk);
> 
> I think the current code is correct.
> For v1.x metadata, the chunk size (both chunksize and new_chunk) is
> stored in sectors.  in 0.90 it is stored in bytes.

Oops you're right, I got confused by the comment in md_p.h that says
new_chunk is in bytes, even for v1.x metadata. So please drop this
second patch.

Note that the corresponding hunk in the third patch needs to be
changed as well:

-           sb->new_chunk = cpu_to_le32(mddev->new_chunk);
+           sb->new_chunk = cpu_to_le32(mddev->new_chunk_sectors << 9);

must become

-           sb->new_chunk = cpu_to_le32(mddev->new_chunk>>9);
+           sb->new_chunk = cpu_to_le32(mddev->new_chunk_sectors);

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

[PATCH 3/6] md: Convert mddev->new_chunk to sectors.

[Andre Noll]

A straight-forward conversion which gets rid of some
multiplications/divisions/shifts. The patch also introduces a couple
of new ones, most of which are due to conf->chunk_size still being
represented in bytes. This will be cleaned up in subsequent patches.

Signed-off-by: Andre Noll <maan@systemlinux.org>
---
 drivers/md/md.c    |   29 +++++++++++++++--------------
 drivers/md/md.h    |    3 ++-
 drivers/md/raid1.c |    4 ++--
 drivers/md/raid5.c |   45 ++++++++++++++++++++++++---------------------
 4 files changed, 43 insertions(+), 38 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 66e32e4..a54ec91 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -886,13 +886,13 @@ static int super_90_validate(mddev_t *mddev, mdk_rdev_t *rdev)
 			mddev->delta_disks = sb->delta_disks;
 			mddev->new_level = sb->new_level;
 			mddev->new_layout = sb->new_layout;
-			mddev->new_chunk = sb->new_chunk;
+			mddev->new_chunk_sectors = sb->new_chunk >> 9;
 		} else {
 			mddev->reshape_position = MaxSector;
 			mddev->delta_disks = 0;
 			mddev->new_level = mddev->level;
 			mddev->new_layout = mddev->layout;
-			mddev->new_chunk = mddev->chunk_sectors << 9;
+			mddev->new_chunk_sectors = mddev->chunk_sectors;
 		}
 
 		if (sb->state & (1<<MD_SB_CLEAN))
@@ -1007,7 +1007,7 @@ static void super_90_sync(mddev_t *mddev, mdk_rdev_t *rdev)
 		sb->new_level = mddev->new_level;
 		sb->delta_disks = mddev->delta_disks;
 		sb->new_layout = mddev->new_layout;
-		sb->new_chunk = mddev->new_chunk;
+		sb->new_chunk = mddev->new_chunk_sectors << 9;
 	}
 	mddev->minor_version = sb->minor_version;
 	if (mddev->in_sync)
@@ -1300,13 +1300,13 @@ static int super_1_validate(mddev_t *mddev, mdk_rdev_t *rdev)
 			mddev->delta_disks = le32_to_cpu(sb->delta_disks);
 			mddev->new_level = le32_to_cpu(sb->new_level);
 			mddev->new_layout = le32_to_cpu(sb->new_layout);
-			mddev->new_chunk = le32_to_cpu(sb->new_chunk)<<9;
+			mddev->new_chunk_sectors = le32_to_cpu(sb->new_chunk);
 		} else {
 			mddev->reshape_position = MaxSector;
 			mddev->delta_disks = 0;
 			mddev->new_level = mddev->level;
 			mddev->new_layout = mddev->layout;
-			mddev->new_chunk = mddev->chunk_sectors << 9;
+			mddev->new_chunk_sectors = mddev->chunk_sectors;
 		}
 
 	} else if (mddev->pers == NULL) {
@@ -1402,7 +1402,7 @@ static void super_1_sync(mddev_t *mddev, mdk_rdev_t *rdev)
 		sb->new_layout = cpu_to_le32(mddev->new_layout);
 		sb->delta_disks = cpu_to_le32(mddev->delta_disks);
 		sb->new_level = cpu_to_le32(mddev->new_level);
-		sb->new_chunk = cpu_to_le32(mddev->new_chunk);
+		sb->new_chunk = cpu_to_le32(mddev->new_chunk_sectors << 9);
 	}
 
 	max_dev = 0;
@@ -2746,7 +2746,7 @@ level_store(mddev_t *mddev, const char *buf, size_t len)
 	if (IS_ERR(priv)) {
 		mddev->new_level = mddev->level;
 		mddev->new_layout = mddev->layout;
-		mddev->new_chunk = mddev->chunk_sectors << 9;
+		mddev->new_chunk_sectors = mddev->chunk_sectors;
 		mddev->raid_disks -= mddev->delta_disks;
 		mddev->delta_disks = 0;
 		module_put(pers->owner);
@@ -2764,7 +2764,7 @@ level_store(mddev_t *mddev, const char *buf, size_t len)
 	strlcpy(mddev->clevel, pers->name, sizeof(mddev->clevel));
 	mddev->level = mddev->new_level;
 	mddev->layout = mddev->new_layout;
-	mddev->chunk_sectors = mddev->new_chunk >> 9;
+	mddev->chunk_sectors = mddev->new_chunk_sectors;
 	mddev->delta_disks = 0;
 	pers->run(mddev);
 	mddev_resume(mddev);
@@ -2857,8 +2857,9 @@ static ssize_t
 chunk_size_show(mddev_t *mddev, char *page)
 {
 	if (mddev->reshape_position != MaxSector &&
-	    mddev->chunk_sectors << 9 != mddev->new_chunk)
-		return sprintf(page, "%d (%d)\n", mddev->new_chunk,
+	    mddev->chunk_sectors != mddev->new_chunk_sectors)
+		return sprintf(page, "%d (%d)\n",
+			       (int)mddev->new_chunk_sectors << 9,
 			       (int)mddev->chunk_sectors << 9);
 	return sprintf(page, "%d\n", (int)mddev->chunk_sectors << 9);
 }
@@ -2880,7 +2881,7 @@ chunk_size_store(mddev_t *mddev, const char *buf, size_t len)
 		if (err)
 			return err;
 	} else {
-		mddev->new_chunk = n;
+		mddev->new_chunk_sectors = n >> 9;
 		if (mddev->reshape_position == MaxSector)
 			mddev->chunk_sectors = n >> 9;
 	}
@@ -3651,7 +3652,7 @@ reshape_position_store(mddev_t *mddev, const char *buf, size_t len)
 	mddev->delta_disks = 0;
 	mddev->new_level = mddev->level;
 	mddev->new_layout = mddev->layout;
-	mddev->new_chunk = mddev->chunk_sectors << 9;
+	mddev->new_chunk_sectors = mddev->chunk_sectors;
 	return len;
 }
 
@@ -4400,7 +4401,7 @@ static int do_md_stop(mddev_t * mddev, int mode, int is_open)
 		mddev->delta_disks = 0;
 		mddev->new_level = LEVEL_NONE;
 		mddev->new_layout = 0;
-		mddev->new_chunk = 0;
+		mddev->new_chunk_sectors = 0;
 		mddev->curr_resync = 0;
 		mddev->resync_mismatches = 0;
 		mddev->suspend_lo = mddev->suspend_hi = 0;
@@ -5069,7 +5070,7 @@ static int set_array_info(mddev_t * mddev, mdu_array_info_t *info)
 	get_random_bytes(mddev->uuid, 16);
 
 	mddev->new_level = mddev->level;
-	mddev->new_chunk = mddev->chunk_sectors << 9;
+	mddev->new_chunk_sectors = mddev->chunk_sectors;
 	mddev->new_layout = mddev->layout;
 	mddev->delta_disks = 0;
 
diff --git a/drivers/md/md.h b/drivers/md/md.h
index 7cbb895..4274cdc 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -166,7 +166,8 @@ struct mddev_s
 	 * If reshape_position is MaxSector, then no reshape is happening (yet).
 	 */
 	sector_t			reshape_position;
-	int				delta_disks, new_level, new_layout, new_chunk;
+	int				delta_disks, new_level, new_layout;
+	sector_t			new_chunk_sectors;
 
 	struct mdk_thread_s		*thread;	/* management thread */
 	struct mdk_thread_s		*sync_thread;	/* doing resync or reconstruct */
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index b56f075..4692a8a 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -2161,10 +2161,10 @@ static int raid1_reshape(mddev_t *mddev)
 	int d, d2, err;
 
 	/* Cannot change chunk_size, layout, or level */
-	if (mddev->chunk_sectors << 9 != mddev->new_chunk ||
+	if (mddev->chunk_sectors != mddev->new_chunk_sectors ||
 	    mddev->layout != mddev->new_layout ||
 	    mddev->level != mddev->new_level) {
-		mddev->new_chunk = mddev->chunk_sectors << 9;
+		mddev->new_chunk_sectors = mddev->chunk_sectors;
 		mddev->new_layout = mddev->layout;
 		mddev->new_level = mddev->level;
 		return -EINVAL;
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 85653e5..ab89dd0 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -3358,8 +3358,8 @@ static int raid5_mergeable_bvec(struct request_queue *q,
 	if ((bvm->bi_rw & 1) == WRITE)
 		return biovec->bv_len; /* always allow writes to be mergeable */
 
-	if (mddev->new_chunk < mddev->chunk_sectors << 9)
-		chunk_sectors = mddev->new_chunk >> 9;
+	if (mddev->new_chunk_sectors < mddev->chunk_sectors)
+		chunk_sectors = mddev->new_chunk_sectors;
 	max =  (chunk_sectors - ((sector & (chunk_sectors - 1)) + bio_sectors)) << 9;
 	if (max < 0) max = 0;
 	if (max <= biovec->bv_len && bio_sectors == 0)
@@ -3375,8 +3375,8 @@ static int in_chunk_boundary(mddev_t *mddev, struct bio *bio)
 	unsigned int chunk_sectors = mddev->chunk_sectors;
 	unsigned int bio_sectors = bio->bi_size >> 9;
 
-	if (mddev->new_chunk < mddev->chunk_sectors << 9)
-		chunk_sectors = mddev->new_chunk >> 9;
+	if (mddev->new_chunk_sectors < mddev->chunk_sectors)
+		chunk_sectors = mddev->new_chunk_sectors;
 	return  chunk_sectors >=
 		((sector & (chunk_sectors - 1)) + bio_sectors);
 }
@@ -3791,8 +3791,8 @@ static sector_t reshape_request(mddev_t *mddev, sector_t sector_nr, int *skipped
 	 * If old and new chunk sizes differ, we need to process the
 	 * largest of these
 	 */
-	if (mddev->new_chunk > mddev->chunk_sectors << 9)
-		reshape_sectors = mddev->new_chunk / 512;
+	if (mddev->new_chunk_sectors > mddev->chunk_sectors)
+		reshape_sectors = mddev->new_chunk_sectors;
 	else
 		reshape_sectors = mddev->chunk_sectors;
 
@@ -4304,7 +4304,7 @@ raid5_size(mddev_t *mddev, sector_t sectors, int raid_disks)
 	}
 
 	sectors &= ~(mddev->chunk_sectors - 1);
-	sectors &= ~((sector_t)mddev->new_chunk/512 - 1);
+	sectors &= ~(mddev->new_chunk_sectors - 1);
 	return sectors * (raid_disks - conf->max_degraded);
 }
 
@@ -4336,10 +4336,11 @@ static raid5_conf_t *setup_conf(mddev_t *mddev)
 		return ERR_PTR(-EINVAL);
 	}
 
-	if (!mddev->new_chunk || mddev->new_chunk % PAGE_SIZE ||
-	    !is_power_of_2(mddev->new_chunk)) {
+	if (!mddev->new_chunk_sectors ||
+	    (mddev->new_chunk_sectors << 9) % PAGE_SIZE ||
+	    !is_power_of_2(mddev->new_chunk_sectors)) {
 		printk(KERN_ERR "raid5: invalid chunk size %d for %s\n",
-			mddev->new_chunk, mdname(mddev));
+			(int)mddev->new_chunk_sectors << 9, mdname(mddev));
 		return ERR_PTR(-EINVAL);
 	}
 
@@ -4402,7 +4403,7 @@ static raid5_conf_t *setup_conf(mddev_t *mddev)
 			conf->fullsync = 1;
 	}
 
-	conf->chunk_size = mddev->new_chunk;
+	conf->chunk_size = mddev->new_chunk_sectors << 9;
 	conf->level = mddev->new_level;
 	if (conf->level == 6)
 		conf->max_degraded = 2;
@@ -4476,7 +4477,7 @@ static int run(mddev_t *mddev)
 		 * geometry.
 		 */
 		here_new = mddev->reshape_position;
-		if (sector_div(here_new, (mddev->new_chunk>>9)*
+		if (sector_div(here_new, mddev->new_chunk_sectors *
 			       (mddev->raid_disks - max_degraded))) {
 			printk(KERN_ERR "raid5: reshape_position not "
 			       "on a stripe boundary\n");
@@ -4499,7 +4500,7 @@ static int run(mddev_t *mddev)
 	} else {
 		BUG_ON(mddev->level != mddev->new_level);
 		BUG_ON(mddev->layout != mddev->new_layout);
-		BUG_ON(mddev->chunk_sectors << 9 != mddev->new_chunk);
+		BUG_ON(mddev->chunk_sectors != mddev->new_chunk_sectors);
 		BUG_ON(mddev->delta_disks != 0);
 	}
 
@@ -4851,7 +4852,7 @@ static int raid5_check_reshape(mddev_t *mddev)
 
 	if (mddev->delta_disks == 0 &&
 	    mddev->new_layout == mddev->layout &&
-	    mddev->new_chunk == mddev->chunk_sectors << 9)
+	    mddev->new_chunk_sectors == mddev->chunk_sectors)
 		return -EINVAL; /* nothing to do */
 	if (mddev->bitmap)
 		/* Cannot grow a bitmap yet */
@@ -4881,9 +4882,11 @@ static int raid5_check_reshape(mddev_t *mddev)
 	 */
 	if (((mddev->chunk_sectors << 9) / STRIPE_SIZE) * 4
 		> conf->max_nr_stripes ||
-	    (mddev->new_chunk / STRIPE_SIZE) * 4 > conf->max_nr_stripes) {
+	    ((mddev->new_chunk_sectors << 9) / STRIPE_SIZE) * 4
+		> conf->max_nr_stripes) {
 		printk(KERN_WARNING "raid5: reshape: not enough stripes.  Needed %lu\n",
-		       (max((int)mddev->chunk_sectors << 9, mddev->new_chunk)
+		       (max(mddev->chunk_sectors << 9,
+			mddev->new_chunk_sectors << 9)
 			/ STRIPE_SIZE)*4);
 		return -ENOSPC;
 	}
@@ -4929,7 +4932,7 @@ static int raid5_start_reshape(mddev_t *mddev)
 	conf->previous_raid_disks = conf->raid_disks;
 	conf->raid_disks += mddev->delta_disks;
 	conf->prev_chunk = conf->chunk_size;
-	conf->chunk_size = mddev->new_chunk;
+	conf->chunk_size = mddev->new_chunk_sectors << 9;
 	conf->prev_algo = conf->algorithm;
 	conf->algorithm = mddev->new_layout;
 	if (mddev->delta_disks < 0)
@@ -5114,7 +5117,7 @@ static void *raid5_takeover_raid1(mddev_t *mddev)
 
 	mddev->new_level = 5;
 	mddev->new_layout = ALGORITHM_LEFT_SYMMETRIC;
-	mddev->new_chunk = chunksect << 9;
+	mddev->new_chunk_sectors = chunksect;
 
 	return setup_conf(mddev);
 }
@@ -5185,7 +5188,7 @@ static int raid5_reconfig(mddev_t *mddev, int new_layout, int new_chunk)
 		}
 		if (new_chunk > 0) {
 			conf->chunk_size = new_chunk;
-			mddev->new_chunk = new_chunk;
+			mddev->new_chunk_sectors = new_chunk >> 9;
 			mddev->chunk_sectors = new_chunk >> 9;
 		}
 		set_bit(MD_CHANGE_DEVS, &mddev->flags);
@@ -5194,7 +5197,7 @@ static int raid5_reconfig(mddev_t *mddev, int new_layout, int new_chunk)
 		if (new_layout >= 0)
 			mddev->new_layout = new_layout;
 		if (new_chunk > 0)
-			mddev->new_chunk = new_chunk;
+			mddev->new_chunk_sectors = new_chunk >> 9;
 	}
 	return 0;
 }
@@ -5219,7 +5222,7 @@ static int raid6_reconfig(mddev_t *mddev, int new_layout, int new_chunk)
 	if (new_layout >= 0)
 		mddev->new_layout = new_layout;
 	if (new_chunk > 0)
-		mddev->new_chunk = new_chunk;
+		mddev->new_chunk_sectors = new_chunk >> 9;
 
 	return 0;
 }
-- 
1.5.4.3

[PATCH 4/6] md: convert conf->chunk_size and conf->prev_chunk to sectors.

[Andre Noll]

This kills some more shifts.

Signed-off-by: Andre Noll <maan@systemlinux.org>
---
 drivers/md/raid5.c |   31 +++++++++++++++----------------
 drivers/md/raid5.h |    6 ++++--
 2 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index ab89dd0..e7952f0 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -1274,8 +1274,8 @@ static sector_t raid5_compute_sector(raid5_conf_t *conf, sector_t r_sector,
 	sector_t new_sector;
 	int algorithm = previous ? conf->prev_algo
 				 : conf->algorithm;
-	int sectors_per_chunk = previous ? (conf->prev_chunk >> 9)
-					 : (conf->chunk_size >> 9);
+	int sectors_per_chunk = previous ? conf->prev_chunk_sectors
+					 : conf->chunk_sectors;
 	int raid_disks = previous ? conf->previous_raid_disks
 				  : conf->raid_disks;
 	int data_disks = raid_disks - conf->max_degraded;
@@ -1480,8 +1480,8 @@ static sector_t compute_blocknr(struct stripe_head *sh, int i, int previous)
 	int raid_disks = sh->disks;
 	int data_disks = raid_disks - conf->max_degraded;
 	sector_t new_sector = sh->sector, check;
-	int sectors_per_chunk = previous ? (conf->prev_chunk >> 9)
-					 : (conf->chunk_size >> 9);
+	int sectors_per_chunk = previous ? conf->prev_chunk_sectors
+					 : conf->chunk_sectors;
 	int algorithm = previous ? conf->prev_algo
 				 : conf->algorithm;
 	sector_t stripe;
@@ -1997,8 +1997,7 @@ static void stripe_set_idx(sector_t stripe, raid5_conf_t *conf, int previous,
 			    struct stripe_head *sh)
 {
 	int sectors_per_chunk =
-		previous ? (conf->prev_chunk >> 9)
-			 : (conf->chunk_size >> 9);
+		previous ? conf->prev_chunk_sectors : conf->chunk_sectors;
 	int dd_idx;
 	int chunk_offset = sector_div(stripe, sectors_per_chunk);
 	int disks = previous ? conf->previous_raid_disks : conf->raid_disks;
@@ -3916,9 +3915,9 @@ static sector_t reshape_request(mddev_t *mddev, sector_t sector_nr, int *skipped
 		raid5_compute_sector(conf, stripe_addr*(new_data_disks),
 				     1, &dd_idx, NULL);
 	last_sector =
-		raid5_compute_sector(conf, ((stripe_addr+conf->chunk_size/512)
-					    *(new_data_disks) - 1),
-				     1, &dd_idx, NULL);
+		raid5_compute_sector(conf, (stripe_addr + conf->chunk_sectors)
+					   * new_data_disks - 1,
+					   1, &dd_idx, NULL);
 	if (last_sector >= mddev->dev_sectors)
 		last_sector = mddev->dev_sectors - 1;
 	while (first_sector <= last_sector) {
@@ -4403,7 +4402,7 @@ static raid5_conf_t *setup_conf(mddev_t *mddev)
 			conf->fullsync = 1;
 	}
 
-	conf->chunk_size = mddev->new_chunk_sectors << 9;
+	conf->chunk_sectors = mddev->new_chunk_sectors;
 	conf->level = mddev->new_level;
 	if (conf->level == 6)
 		conf->max_degraded = 2;
@@ -4413,7 +4412,7 @@ static raid5_conf_t *setup_conf(mddev_t *mddev)
 	conf->max_nr_stripes = NR_STRIPES;
 	conf->reshape_progress = mddev->reshape_position;
 	if (conf->reshape_progress != MaxSector) {
-		conf->prev_chunk = mddev->chunk_sectors << 9;
+		conf->prev_chunk_sectors = mddev->chunk_sectors;
 		conf->prev_algo = mddev->layout;
 	}
 
@@ -4931,8 +4930,8 @@ static int raid5_start_reshape(mddev_t *mddev)
 	spin_lock_irq(&conf->device_lock);
 	conf->previous_raid_disks = conf->raid_disks;
 	conf->raid_disks += mddev->delta_disks;
-	conf->prev_chunk = conf->chunk_size;
-	conf->chunk_size = mddev->new_chunk_sectors << 9;
+	conf->prev_chunk_sectors = conf->chunk_sectors;
+	conf->chunk_sectors = mddev->new_chunk_sectors;
 	conf->prev_algo = conf->algorithm;
 	conf->algorithm = mddev->new_layout;
 	if (mddev->delta_disks < 0)
@@ -5014,7 +5013,7 @@ static void end_reshape(raid5_conf_t *conf)
 		 */
 		{
 			int data_disks = conf->raid_disks - conf->max_degraded;
-			int stripe = data_disks * (conf->chunk_size
+			int stripe = data_disks * ((conf->chunk_sectors << 9)
 						   / PAGE_SIZE);
 			if (conf->mddev->queue->backing_dev_info.ra_pages < 2 * stripe)
 				conf->mddev->queue->backing_dev_info.ra_pages = 2 * stripe;
@@ -5059,7 +5058,7 @@ static void raid5_finish_reshape(mddev_t *mddev)
 				raid5_remove_disk(mddev, d);
 		}
 		mddev->layout = conf->algorithm;
-		mddev->chunk_sectors = conf->chunk_size >> 9;
+		mddev->chunk_sectors = conf->chunk_sectors;
 		mddev->reshape_position = MaxSector;
 		mddev->delta_disks = 0;
 	}
@@ -5187,7 +5186,7 @@ static int raid5_reconfig(mddev_t *mddev, int new_layout, int new_chunk)
 			mddev->layout = mddev->new_layout = new_layout;
 		}
 		if (new_chunk > 0) {
-			conf->chunk_size = new_chunk;
+			conf->chunk_sectors = new_chunk >> 9;
 			mddev->new_chunk_sectors = new_chunk >> 9;
 			mddev->chunk_sectors = new_chunk >> 9;
 		}
diff --git a/drivers/md/raid5.h b/drivers/md/raid5.h
index 1a25c9e..f2e1259 100644
--- a/drivers/md/raid5.h
+++ b/drivers/md/raid5.h
@@ -334,7 +334,8 @@ struct raid5_private_data {
 	struct hlist_head	*stripe_hashtbl;
 	mddev_t			*mddev;
 	struct disk_info	*spare;
-	int			chunk_size, level, algorithm;
+	sector_t		chunk_sectors;
+	int			level, algorithm;
 	int			max_degraded;
 	int			raid_disks;
 	int			max_nr_stripes;
@@ -350,7 +351,8 @@ struct raid5_private_data {
 	 */
 	sector_t		reshape_safe;
 	int			previous_raid_disks;
-	int			prev_chunk, prev_algo;
+	sector_t		prev_chunk_sectors;
+	int			prev_algo;
 	short			generation; /* increments with every reshape */
 	unsigned long		reshape_checkpoint; /* Time we last updated
 						     * metadata */
-- 
1.5.4.3

Re: [PATCH 4/6] md: convert conf->chunk_size and conf->prev_chunk to sectors.

[Neil Brown]

On Monday May 25, maan@systemlinux.org wrote:
> This kills some more shifts.
> 

> @@ -3916,9 +3915,9 @@ static sector_t reshape_request(mddev_t *mddev, sector_t sector_nr, int *skipped
>  		raid5_compute_sector(conf, stripe_addr*(new_data_disks),
>  				     1, &dd_idx, NULL);
>  	last_sector =
> -		raid5_compute_sector(conf, ((stripe_addr+conf->chunk_size/512)
> -					    *(new_data_disks) - 1),
> -				     1, &dd_idx, NULL);
> +		raid5_compute_sector(conf, (stripe_addr + conf->chunk_sectors)
> +					   * new_data_disks - 1,
> +					   1, &dd_idx, NULL);

FYI, the () that you removed there were present for a reason.  They
help emacs C-mode to get the indenting right.
Without them, the indent function make it look like
> +		raid5_compute_sector(conf, (stripe_addr + conf->chunk_sectors)
> +				     * new_data_disks - 1,
> +				     1, &dd_idx, NULL);

With them I get
		raid5_compute_sector(conf, ((stripe_addr + conf->chunk_sectors)
					    * new_data_disks - 1),
				     1, &dd_idx, NULL);

which is "correct".

> diff --git a/drivers/md/raid5.h b/drivers/md/raid5.h
> index 1a25c9e..f2e1259 100644
> --- a/drivers/md/raid5.h
> +++ b/drivers/md/raid5.h
> @@ -334,7 +334,8 @@ struct raid5_private_data {
>  	struct hlist_head	*stripe_hashtbl;
>  	mddev_t			*mddev;
>  	struct disk_info	*spare;
> -	int			chunk_size, level, algorithm;
> +	sector_t		chunk_sectors;
> +	int			level, algorithm;
>  	int			max_degraded;
>  	int			raid_disks;
>  	int			max_nr_stripes;
> @@ -350,7 +351,8 @@ struct raid5_private_data {
>  	 */
>  	sector_t		reshape_safe;
>  	int			previous_raid_disks;
> -	int			prev_chunk, prev_algo;
> +	sector_t		prev_chunk_sectors;
> +	int			prev_algo;
>  	short			generation; /* increments with every reshape */
>  	unsigned long		reshape_checkpoint; /* Time we last updated
>  						     * metadata */

I restored these to 'int' too.

Thanks!

NeilBrown

[PATCH 5/6] md/raid5: Use is_power_of_2() in raid5_reconfig()/raid6_reconfig().

[Andre Noll]

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

diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index e7952f0..772593d 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -5167,8 +5167,7 @@ static int raid5_reconfig(mddev_t *mddev, int new_layout, int new_chunk)
 	if (new_layout >= 0 && !algorithm_valid_raid5(new_layout))
 		return -EINVAL;
 	if (new_chunk > 0) {
-		if (new_chunk & (new_chunk-1))
-			/* not a power of 2 */
+		if (!is_power_of_2(new_chunk))
 			return -EINVAL;
 		if (new_chunk < PAGE_SIZE)
 			return -EINVAL;
@@ -5206,8 +5205,7 @@ static int raid6_reconfig(mddev_t *mddev, int new_layout, int new_chunk)
 	if (new_layout >= 0 && !algorithm_valid_raid6(new_layout))
 		return -EINVAL;
 	if (new_chunk > 0) {
-		if (new_chunk & (new_chunk-1))
-			/* not a power of 2 */
+		if (!is_power_of_2(new_chunk))
 			return -EINVAL;
 		if (new_chunk < PAGE_SIZE)
 			return -EINVAL;
-- 
1.5.4.3

[PATCH 6/6] md/raid5: Kill outdated comment.

[Andre Noll]

Raid5 has learned to take over also raid4 and raid6 arrays.
---
 drivers/md/raid5.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 772593d..9ec505b 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -5231,8 +5231,6 @@ static void *raid5_takeover(mddev_t *mddev)
 	 *  raid1 - if there are two drives.  We need to know the chunk size
 	 *  raid4 - trivial - just use a raid4 layout.
 	 *  raid6 - Providing it is a *_6 layout
-	 *
-	 * For now, just do raid1
 	 */
 
 	if (mddev->level == 1)
-- 
1.5.4.3

Re: [PATCH 0/6] md: More sector_t conversions -- intro

[Neil Brown]

On Monday May 25, maan@systemlinux.org wrote:
> Hi
> 
> this patch set against the for-next tree as of last Friday converts a
> couple of remaining internal representation of md sizes to sector_t,
> fixes a bug in the reshape code and adds a couple of trivial cleanups.
> 
> Patch #1 converts mddev->chunk_size which was previously stored
> in bytes.  Patch #2 fixes what I believe is a bug in super_1_sync()
> and might be a candiate for -stable. Patches #3 and #4 are further
> straight-forward conversions that deal with mddev->new_chunk,
> conf->chunk_size and conf->prev_chunk. Patches #5 and #6 are pure
> cleanups.

Thanks.
With the exception of #2, and with the other changes I mentioned,
there are all i my for-next now.

Even #2 was useful.  It helped me to realise that super_1_sync wasn't
writing out any changed to level/layout/chunk_size.  Until just
recently these couldn't change so there was no point.  But now
they can.  So I added another patch (below) to my for-linus set and
modified your patches to match.

> 
> All patches pass the tests of the mdadm test suite except
> 07reshape5intr which appears fail also for the vanilla for-next tree.

Hmm... I guess I should look into that!

Thanks,
NeilBrown


commit 62e1e389f87a8839ad83b08c44691d1df8320846
Author: NeilBrown <neilb@suse.de>
Date:   Tue May 26 09:40:59 2009 +1000

    md: always update level / chunk_size / layout when writing v1.x metadata.
    
    We previously didn't update these fields when writing the metadata
    because they could never change.  They can now, so we better write
    them.
    v0.90 metadata always updated these fields.
    
    Signed-off-by: NeilBrown <neilb@suse.de>

diff --git a/drivers/md/md.c b/drivers/md/md.c
index fccc834..aa79d55 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -1375,6 +1375,9 @@ static void super_1_sync(mddev_t *mddev, mdk_rdev_t *rdev)
 
 	sb->raid_disks = cpu_to_le32(mddev->raid_disks);
 	sb->size = cpu_to_le64(mddev->dev_sectors);
+	sb->chunksize = cpu_to_le32(mddev->chunk_size >> 9);
+	sb->level = cpu_to_le32(mddev->level);
+	sb->layout = cpu_to_le32(mddev->layout);
 
 	if (mddev->bitmap && mddev->bitmap_file == NULL) {
 		sb->bitmap_offset = cpu_to_le32((__u32)mddev->bitmap_offset);

Re: [PATCH 0/6] md: More sector_t conversions -- intro

[Neil Brown]

On Tuesday May 26, neilb@suse.de wrote:
> > 
> > All patches pass the tests of the mdadm test suite except
> > 07reshape5intr which appears fail also for the vanilla for-next tree.
> 
> Hmm... I guess I should look into that!

Yep. There is a big there.  Thanks for helping find it.

Here is the patch.

Thanks,
NeilBrown


Author: NeilBrown <neilb@suse.de>
Date:   Tue May 26 12:39:27 2009 +1000

    md: raid5: avoid sector values going negative when testing reshape progress.
    
    As sector_t in unsigned, we cannot afford to let 'safepos' etc go
    negative.
    So replace
       a -= b;
    by
       a -= min(b,a);
    
    Signed-off-by: NeilBrown <neilb@suse.de>

diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 6b89a7e..dce7741 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -3810,13 +3810,13 @@ static sector_t reshape_request(mddev_t *mddev, sector_t sector_nr, int *skipped
 	safepos = conf->reshape_safe;
 	sector_div(safepos, data_disks);
 	if (mddev->delta_disks < 0) {
-		writepos -= reshape_sectors;
+		writepos -= min(reshape_sectors, writepos);
 		readpos += reshape_sectors;
 		safepos += reshape_sectors;
 	} else {
 		writepos += reshape_sectors;
-		readpos -= reshape_sectors;
-		safepos -= reshape_sectors;
+		readpos -= min(reshape_sectors, readpos);
+		safepos -= min(reshape_sectors, safepos);
 	}
 
 	/* 'writepos' is the most advanced device address we might write.

Re: [PATCH 0/6] md: More sector_t conversions -- intro

[Andre Noll]

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

On 09:49, Neil Brown wrote:

> > Patch #1 converts mddev->chunk_size which was previously stored
> > in bytes.  Patch #2 fixes what I believe is a bug in super_1_sync()
> > and might be a candiate for -stable. Patches #3 and #4 are further
> > straight-forward conversions that deal with mddev->new_chunk,
> > conf->chunk_size and conf->prev_chunk. Patches #5 and #6 are pure
> > cleanups.
> 
> Thanks.
> With the exception of #2, and with the other changes I mentioned,
> there are all i my for-next now.

Thanks. BTW: I have a couple of other patches ready that push down
code from md.c to the personalities as you suggested. These need to
be rebased to current for-next though. I'll send them later this week
or at the beginning of next week.

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