[PATCH 0/6] md: Make linear raid use sector_t for internal representations

[PATCH 0/6] md: Make linear raid use sector_t for internal representations

[Andre Noll]

Hi

here's a bunch of patches that convert all representations of
internal variables in linear.c to sectors. Patches 1-3 are only
trivial cleanups.

Patches #4 and #5 are straight-forward replacements of the size and
offset fields of struct dev_info and some local variables respectively.

The only patch in this series that is not self-explanatory is #6 that
converts the hash_spacing and the preshift fields of struct dev_info
to sectors. I've added a detailed changelog message for this patch
that explains the details of the changes introduced by this patch.

checkpatch.pl is happy with all six patches, and the patched kernel
was tested by the test suite of mdadm. While the 00linear test was
successful, a couple of other tests failed. However, this seems to
be unrelated to the changes of this series because the same tests
also failed with an unpached ubuntu-hardy kernel.


 drivers/md/linear.c         |  122 +++++++++++++++++-------------------------
 include/linux/raid/linear.h |   10 ++--
 2 files changed, 55 insertions(+), 77 deletions(-)

[PATCH 0/6] md: Make linear raid use sector_t for internal representations

[Andre Noll]

[try #2, rebased to Neil's for-next branch as of today]

here's a bunch of patches that convert all representations of
internal variables in linear.c to sectors. Patches 1-3 are only
trivial cleanups.

Patches #4 and #5 are straight-forward replacements of the size and
offset fields of struct dev_info and some local variables respectively.

The only patch in this series that is not self-explanatory is #6 that
converts the hash_spacing and the preshift fields of struct dev_info
to sectors. I've added a detailed changelog message for this patch
that explains the details of the changes introduced by this patch.

checkpatch.pl is happy with all six patches, and the patched kernel
was tested by the test suite of mdadm. While the 00linear test was
successful, a couple of other tests failed. However, this seems to
be unrelated to the changes of this series because the same tests
also failed with an unpached ubuntu-hardy kernel.

[PATCH 1/6] md: linear.c: Fix typo in comment.

[Andre Noll]

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

diff --git a/drivers/md/linear.c b/drivers/md/linear.c
index b1eebf8..5f64435 100644
--- a/drivers/md/linear.c
+++ b/drivers/md/linear.c
@@ -161,7 +161,7 @@ static linear_conf_t *linear_conf(mddev_t *mddev, int raid_disks)
 	/* min_spacing is the minimum spacing that will fit the hash
 	 * table in one PAGE.  This may be much smaller than needed.
 	 * We find the smallest non-terminal set of consecutive devices
-	 * that is larger than min_spacing as use the size of that as
+	 * that is larger than min_spacing and use the size of that as
 	 * the actual spacing
 	 */
 	conf->hash_spacing = conf->array_sectors / 2;
-- 
1.5.3.8

[PATCH 2/6] md: linear.c: Remove pointless initialization of curr_offset.

[Andre Noll]

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

diff --git a/drivers/md/linear.c b/drivers/md/linear.c
index 5f64435..b5e5e11 100644
--- a/drivers/md/linear.c
+++ b/drivers/md/linear.c
@@ -218,7 +218,6 @@ static linear_conf_t *linear_conf(mddev_t *mddev, int raid_disks)
 			conf->disks[i-1].size;
 
 	table = conf->hash_table;
-	curr_offset = 0;
 	i = 0;
 	for (curr_offset = 0;
 	     curr_offset < conf->array_sectors / 2;
-- 
1.5.3.8

[PATCH 3/6] md: linear.c: Remove broken debug code.

[Andre Noll]

conf->smallest_size is undefined since day one of the git repo..

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

diff --git a/drivers/md/linear.c b/drivers/md/linear.c
index b5e5e11..2aa0e41 100644
--- a/drivers/md/linear.c
+++ b/drivers/md/linear.c
@@ -367,29 +367,6 @@ static int linear_make_request (struct request_queue *q, struct bio *bio)
 static void linear_status (struct seq_file *seq, mddev_t *mddev)
 {
 
-#undef MD_DEBUG
-#ifdef MD_DEBUG
-	int j;
-	linear_conf_t *conf = mddev_to_conf(mddev);
-	sector_t s = 0;
-  
-	seq_printf(seq, "      ");
-	for (j = 0; j < mddev->raid_disks; j++)
-	{
-		char b[BDEVNAME_SIZE];
-		s += conf->smallest_size;
-		seq_printf(seq, "[%s",
-			   bdevname(conf->hash_table[j][0].rdev->bdev,b));
-
-		while (s > conf->hash_table[j][0].offset +
-		           conf->hash_table[j][0].size)
-			seq_printf(seq, "/%s] ",
-				   bdevname(conf->hash_table[j][1].rdev->bdev,b));
-		else
-			seq_printf(seq, "] ");
-	}
-	seq_printf(seq, "\n");
-#endif
 	seq_printf(seq, " %dk rounding", mddev->chunk_size/1024);
 }
 
-- 
1.5.3.8

[PATCH 4/6] md: linear: Represent dev_info->size and dev_info->offset in sectors.

[Andre Noll]

Rename them to num_sectors and start_sector which is more descriptive.

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

diff --git a/drivers/md/linear.c b/drivers/md/linear.c
index 2aa0e41..f2d38e1 100644
--- a/drivers/md/linear.c
+++ b/drivers/md/linear.c
@@ -42,7 +42,7 @@ static inline dev_info_t *which_dev(mddev_t *mddev, sector_t sector)
 	(void)sector_div(block, conf->hash_spacing);
 	hash = conf->hash_table[block];
 
-	while ((sector>>1) >= (hash->size + hash->offset))
+	while (sector >= hash->num_sectors + hash->start_sector)
 		hash++;
 	return hash;
 }
@@ -65,7 +65,7 @@ static int linear_mergeable_bvec(struct request_queue *q,
 	sector_t sector = bvm->bi_sector + get_start_sect(bvm->bi_bdev);
 
 	dev0 = which_dev(mddev, sector);
-	maxsectors = (dev0->size << 1) - (sector - (dev0->offset<<1));
+	maxsectors = dev0->num_sectors - sector + dev0->start_sector;
 
 	if (maxsectors < bio_sectors)
 		maxsectors = 0;
@@ -113,7 +113,7 @@ static linear_conf_t *linear_conf(mddev_t *mddev, int raid_disks)
 	mdk_rdev_t *rdev;
 	int i, nb_zone, cnt;
 	sector_t min_spacing;
-	sector_t curr_offset;
+	sector_t curr_sector;
 	struct list_head *tmp;
 
 	conf = kzalloc (sizeof (*conf) + raid_disks*sizeof(dev_info_t),
@@ -145,7 +145,7 @@ static linear_conf_t *linear_conf(mddev_t *mddev, int raid_disks)
 		    mddev->queue->max_sectors > (PAGE_SIZE>>9))
 			blk_queue_max_sectors(mddev->queue, PAGE_SIZE>>9);
 
-		disk->size = rdev->size;
+		disk->num_sectors = rdev->size * 2;
 		conf->array_sectors += rdev->size * 2;
 
 		cnt++;
@@ -169,7 +169,7 @@ static linear_conf_t *linear_conf(mddev_t *mddev, int raid_disks)
 		sector_t sz = 0;
 		int j;
 		for (j = i; j < cnt - 1 && sz < min_spacing; j++)
-			sz += conf->disks[j].size;
+			sz += conf->disks[j].num_sectors / 2;
 		if (sz >= min_spacing && sz < conf->hash_spacing)
 			conf->hash_spacing = sz;
 	}
@@ -211,20 +211,20 @@ static linear_conf_t *linear_conf(mddev_t *mddev, int raid_disks)
 	 * Here we generate the linear hash table
 	 * First calculate the device offsets.
 	 */
-	conf->disks[0].offset = 0;
+	conf->disks[0].start_sector = 0;
 	for (i = 1; i < raid_disks; i++)
-		conf->disks[i].offset =
-			conf->disks[i-1].offset +
-			conf->disks[i-1].size;
+		conf->disks[i].start_sector =
+			conf->disks[i-1].start_sector +
+			conf->disks[i-1].num_sectors;
 
 	table = conf->hash_table;
 	i = 0;
-	for (curr_offset = 0;
-	     curr_offset < conf->array_sectors / 2;
-	     curr_offset += conf->hash_spacing) {
+	for (curr_sector = 0;
+	     curr_sector < conf->array_sectors;
+	     curr_sector += conf->hash_spacing * 2) {
 
 		while (i < raid_disks-1 &&
-		       curr_offset >= conf->disks[i+1].offset)
+		       curr_sector >= conf->disks[i+1].start_sector)
 			i++;
 
 		*table ++ = conf->disks + i;
@@ -316,7 +316,6 @@ static int linear_make_request (struct request_queue *q, struct bio *bio)
 	const int rw = bio_data_dir(bio);
 	mddev_t *mddev = q->queuedata;
 	dev_info_t *tmp_dev;
-	sector_t block;
 
 	if (unlikely(bio_barrier(bio))) {
 		bio_endio(bio, -EOPNOTSUPP);
@@ -327,29 +326,30 @@ static int linear_make_request (struct request_queue *q, struct bio *bio)
 	disk_stat_add(mddev->gendisk, sectors[rw], bio_sectors(bio));
 
 	tmp_dev = which_dev(mddev, bio->bi_sector);
-	block = bio->bi_sector >> 1;
     
-	if (unlikely(block >= (tmp_dev->size + tmp_dev->offset)
-		     || block < tmp_dev->offset)) {
+	if (unlikely(bio->bi_sector >= tmp_dev->num_sectors
+		+ tmp_dev->start_sector || bio->bi_sector <
+		tmp_dev->start_sector)) {
 		char b[BDEVNAME_SIZE];
 
-		printk("linear_make_request: Block %llu out of bounds on "
-			"dev %s size %llu offset %llu\n",
-			(unsigned long long)block,
+		printk("linear_make_request: Sector %llu out of bounds on "
+			"dev %s: %llu sectors, offset %llu\n",
+			(unsigned long long)bio->bi_sector,
 			bdevname(tmp_dev->rdev->bdev, b),
-			(unsigned long long)tmp_dev->size,
-		        (unsigned long long)tmp_dev->offset);
+			(unsigned long long)tmp_dev->num_sectors,
+			(unsigned long long)tmp_dev->start_sector);
 		bio_io_error(bio);
 		return 0;
 	}
 	if (unlikely(bio->bi_sector + (bio->bi_size >> 9) >
-		     (tmp_dev->offset + tmp_dev->size)<<1)) {
+			tmp_dev->start_sector + tmp_dev->num_sectors)) {
 		/* This bio crosses a device boundary, so we have to
 		 * split it.
 		 */
 		struct bio_pair *bp;
 		bp = bio_split(bio, bio_split_pool,
-			       ((tmp_dev->offset + tmp_dev->size)<<1) - bio->bi_sector);
+			tmp_dev->start_sector + tmp_dev->num_sectors
+			- bio->bi_sector);
 		if (linear_make_request(q, &bp->bio1))
 			generic_make_request(&bp->bio1);
 		if (linear_make_request(q, &bp->bio2))
@@ -359,7 +359,8 @@ static int linear_make_request (struct request_queue *q, struct bio *bio)
 	}
 		    
 	bio->bi_bdev = tmp_dev->rdev->bdev;
-	bio->bi_sector = bio->bi_sector - (tmp_dev->offset << 1) + tmp_dev->rdev->data_offset;
+	bio->bi_sector = bio->bi_sector - tmp_dev->start_sector
+		+ tmp_dev->rdev->data_offset;
 
 	return 1;
 }
diff --git a/include/linux/raid/linear.h b/include/linux/raid/linear.h
index 7e37511..87090e9 100644
--- a/include/linux/raid/linear.h
+++ b/include/linux/raid/linear.h
@@ -5,8 +5,8 @@
 
 struct dev_info {
 	mdk_rdev_t	*rdev;
-	sector_t	size;
-	sector_t	offset;
+	sector_t	num_sectors;
+	sector_t	start_sector;
 };
 
 typedef struct dev_info dev_info_t;
-- 
1.5.3.8

[PATCH 5/6] md: linear.c: Make two local variables sector-based.

[Andre Noll]

This is a preparation for representing also the remaining fields of struct
linear_private_data as sectors.

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

diff --git a/drivers/md/linear.c b/drivers/md/linear.c
index f2d38e1..76a8a44 100644
--- a/drivers/md/linear.c
+++ b/drivers/md/linear.c
@@ -112,7 +112,7 @@ static linear_conf_t *linear_conf(mddev_t *mddev, int raid_disks)
 	dev_info_t **table;
 	mdk_rdev_t *rdev;
 	int i, nb_zone, cnt;
-	sector_t min_spacing;
+	sector_t min_sectors;
 	sector_t curr_sector;
 	struct list_head *tmp;
 
@@ -155,23 +155,23 @@ static linear_conf_t *linear_conf(mddev_t *mddev, int raid_disks)
 		goto out;
 	}
 
-	min_spacing = conf->array_sectors / 2;
-	sector_div(min_spacing, PAGE_SIZE/sizeof(struct dev_info *));
+	min_sectors = conf->array_sectors;
+	sector_div(min_sectors, PAGE_SIZE/sizeof(struct dev_info *));
 
-	/* min_spacing is the minimum spacing that will fit the hash
+	/* min_sectors is the minimum spacing that will fit the hash
 	 * table in one PAGE.  This may be much smaller than needed.
 	 * We find the smallest non-terminal set of consecutive devices
-	 * that is larger than min_spacing and use the size of that as
+	 * that is larger than min_sectors and use the size of that as
 	 * the actual spacing
 	 */
 	conf->hash_spacing = conf->array_sectors / 2;
 	for (i=0; i < cnt-1 ; i++) {
-		sector_t sz = 0;
+		sector_t tmp = 0;
 		int j;
-		for (j = i; j < cnt - 1 && sz < min_spacing; j++)
-			sz += conf->disks[j].num_sectors / 2;
-		if (sz >= min_spacing && sz < conf->hash_spacing)
-			conf->hash_spacing = sz;
+		for (j = i; j < cnt - 1 && tmp < min_sectors; j++)
+			tmp += conf->disks[j].num_sectors;
+		if (tmp >= min_sectors && tmp < conf->hash_spacing * 2)
+			conf->hash_spacing = tmp / 2;
 	}
 
 	/* hash_spacing may be too large for sector_div to work with,
-- 
1.5.3.8

[PATCH 6/6] md: Convert remaining 1k representations in linear.c to sectors.

[Andre Noll]

This patch renames hash_spacing and preshift to  spacing and
sector_shift respectively with the following change of semantics:

Case 1: (sizeof(sector_t) <= sizeof(u32)).
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

In this case, we have sector_shift = preshift = 0 and spacing =
2 * hash_spacing.

Hence, the index for the hash table which is computed by the new code
in which_dev() as sector / spacing equals the old value which was
(sector/2) / hash_spacing.

Note also that the value of nb_zone stays the same because both sz
and base double.

Case 2: (sizeof(sector_t) > sizeof(u32)).
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

(aka the shifting dance case). Here we have sector_shift = preshift +
1 and

spacing = 2 * hash_spacing

during the computation of nb_zone and curr_sector, but

spacing = hash_spacing

in which_dev() because in the last hunk of the patch for linear.c we
shift down conf->spacing (= 2 * hash_spacing) by one more bit than
in the old code.

Hence in the computation of nb_zone, sz and base have the same value
as before, so nb_zone is not affected. Also curr_sector in the next
hunk stays the same.

In which_dev() the hash table index is computed as

(sector >> sector_shift) / spacing

In view of sector_shift = preshift + 1 and spacing = hash_spacing,
this equals

((sector/2) >> preshift) / hash_spacing

which is the value computed by the old code.

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

diff --git a/drivers/md/linear.c b/drivers/md/linear.c
index 76a8a44..23bdad4 100644
--- a/drivers/md/linear.c
+++ b/drivers/md/linear.c
@@ -33,14 +33,13 @@ static inline dev_info_t *which_dev(mddev_t *mddev, sector_t sector)
 {
 	dev_info_t *hash;
 	linear_conf_t *conf = mddev_to_conf(mddev);
-	sector_t block = sector >> 1;
 
 	/*
 	 * sector_div(a,b) returns the remainer and sets a to a/b
 	 */
-	block >>= conf->preshift;
-	(void)sector_div(block, conf->hash_spacing);
-	hash = conf->hash_table[block];
+	sector >>= conf->sector_shift;
+	(void)sector_div(sector, conf->spacing);
+	hash = conf->hash_table[sector];
 
 	while (sector >= hash->num_sectors + hash->start_sector)
 		hash++;
@@ -164,25 +163,25 @@ static linear_conf_t *linear_conf(mddev_t *mddev, int raid_disks)
 	 * that is larger than min_sectors and use the size of that as
 	 * the actual spacing
 	 */
-	conf->hash_spacing = conf->array_sectors / 2;
+	conf->spacing = conf->array_sectors;
 	for (i=0; i < cnt-1 ; i++) {
 		sector_t tmp = 0;
 		int j;
 		for (j = i; j < cnt - 1 && tmp < min_sectors; j++)
 			tmp += conf->disks[j].num_sectors;
-		if (tmp >= min_sectors && tmp < conf->hash_spacing * 2)
-			conf->hash_spacing = tmp / 2;
+		if (tmp >= min_sectors && tmp < conf->spacing)
+			conf->spacing = tmp;
 	}
 
-	/* hash_spacing may be too large for sector_div to work with,
+	/* spacing may be too large for sector_div to work with,
 	 * so we might need to pre-shift
 	 */
-	conf->preshift = 0;
+	conf->sector_shift = 0;
 	if (sizeof(sector_t) > sizeof(u32)) {
-		sector_t space = conf->hash_spacing;
+		sector_t space = conf->spacing;
 		while (space > (sector_t)(~(u32)0)) {
 			space >>= 1;
-			conf->preshift++;
+			conf->sector_shift++;
 		}
 	}
 	/*
@@ -194,9 +193,9 @@ static linear_conf_t *linear_conf(mddev_t *mddev, int raid_disks)
 		unsigned round;
 		unsigned long base;
 
-		sz = conf->array_sectors >> (conf->preshift + 1);
+		sz = conf->array_sectors >> conf->sector_shift;
 		sz += 1; /* force round-up */
-		base = conf->hash_spacing >> conf->preshift;
+		base = conf->spacing >> conf->sector_shift;
 		round = sector_div(sz, base);
 		nb_zone = sz + (round ? 1 : 0);
 	}
@@ -221,7 +220,7 @@ static linear_conf_t *linear_conf(mddev_t *mddev, int raid_disks)
 	i = 0;
 	for (curr_sector = 0;
 	     curr_sector < conf->array_sectors;
-	     curr_sector += conf->hash_spacing * 2) {
+	     curr_sector += conf->spacing) {
 
 		while (i < raid_disks-1 &&
 		       curr_sector >= conf->disks[i+1].start_sector)
@@ -230,12 +229,12 @@ static linear_conf_t *linear_conf(mddev_t *mddev, int raid_disks)
 		*table ++ = conf->disks + i;
 	}
 
-	if (conf->preshift) {
-		conf->hash_spacing >>= conf->preshift;
-		/* round hash_spacing up so that when we divide by it,
+	if (conf->sector_shift) {
+		conf->spacing >>= conf->sector_shift;
+		/* round spacing up so that when we divide by it,
 		 * we err on the side of "too-low", which is safest.
 		 */
-		conf->hash_spacing++;
+		conf->spacing++;
 	}
 
 	BUG_ON(table - conf->hash_table > nb_zone);
diff --git a/include/linux/raid/linear.h b/include/linux/raid/linear.h
index 87090e9..f38b9c5 100644
--- a/include/linux/raid/linear.h
+++ b/include/linux/raid/linear.h
@@ -15,9 +15,11 @@ struct linear_private_data
 {
 	struct linear_private_data *prev;	/* earlier version */
 	dev_info_t		**hash_table;
-	sector_t		hash_spacing;
+	sector_t		spacing;
 	sector_t		array_sectors;
-	int			preshift; /* shift before dividing by hash_spacing */
+	int			sector_shift;	/* shift before dividing
+						 * by spacing
+						 */
 	dev_info_t		disks[0];
 };
 
-- 
1.5.3.8