[PATCH 0/4] dm unstriped: support non power of 2 chunk size...

[PATCH 0/4] dm unstriped: support non power of 2 chunk size...

[Heinz Mauelshagen]

Patch series adds support for non power of 2 chunk size to
the "unstriped" target to better suit the striped target
in patch 1.

Patches 2-4 remove the module init error path message,
rename symbols to conform to the target name and avoid
unnecessary headers.


Test script:
------------
#!/bin/sh
#
# Test "unstriped" target non power of 2 chunk size support.
#
# Requires precreated "striped" logical volume as backing device.
# Device will be overwritten!
#
po2=0 # adjust to 1 = power of2, 0 = non-po2
dev=/dev/$vg/$lv # "striped" backing device; adjust to exisiting "striped" LV

table=`dmsetup table $dev`
size=`echo "$table"|awk '{print $2}'`
stripes=`echo "$table"|awk '{print $4}'`
stripesize=`echo "$table"|awk '{print $5}'`
bdstripesize=$stripesize

[ $po2 -eq 0 ] && stripesize=$(($stripesize - 9))
len=$(($size / $stripes / $stripesize * $stripesize))

if [ $po2 -eq 0 ]
then
	dmsetup table $dev | sed "s/$size/$(($len * $stripes))/;s/$bdstripesize/$stripesize/" | dmsetup load $dev
	dmsetup suspend $dev ; dmsetup resume $dev
fi

echo y | mkfs -t ext4 $dev
fsck -fn $dev

devs=""
for i in `seq 0 $(($stripes - 1))`
do
	echo "0 $len unstriped $stripes $stripesize $i $dev 0" | dmsetup create s-$i
	devs="$devs /dev/mapper/s-$i 0"
done

dmsetup create `basename $dev` --table "0 $(($len * $stripes)) striped $stripes $stripesize $devs"
fsck -fn /dev/mapper/`basename $dev`


Example mapping tables resulting from script:
---------------------------------------------
# dmsetup table|egrep "^(nvm-s|s)"
s: 0 270249 striped 3 119 254:7 0 254:8 0 254:9 0
s-2: 0 90083 unstriped 3 119 2 254:6 0
s-1: 0 90083 unstriped 3 119 1 254:6 0
s-0: 0 90083 unstriped 3 119 0 254:6 0
nvm-s: 0 270249 striped 3 119 66:96 13854720 66:64 2048 66:48 2048



Signed-off-by: Heinz Mauelshagen <heinzm@redhat.com>

Heinz Mauelshagen (4):
  dm unstriped: support non power of 2 chunk size
  dm unstriped: remove superfluous module init error path message
  dm unstriped: rename symbols to conform to target name
  dm unstriped: remove unnecessary header includes

 Documentation/device-mapper/unstriped.txt |   5 ++
 drivers/md/dm-unstripe.c                  | 109 +++++++++++++-----------------
 2 files changed, 53 insertions(+), 61 deletions(-)

-- 
2.14.3

[PATCH 1/4] dm unstriped: support non power of 2 chunk size

[Heinz Mauelshagen]

Address "FIXME: must support non power of 2 chunk_size, dm-stripe.c does".

Bump target version to indicate change.

Allow for module loading adding alias as long as we keep dm-unstripe.ko.

Signed-off-by: Heinz Mauelshagen <heinzm@redhat.com>
---
 Documentation/device-mapper/unstriped.txt |  5 +++++
 drivers/md/dm-unstripe.c                  | 23 +++++++++++------------
 2 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/Documentation/device-mapper/unstriped.txt b/Documentation/device-mapper/unstriped.txt
index 0b2a306c54ee..00c904f74a4f 100644
--- a/Documentation/device-mapper/unstriped.txt
+++ b/Documentation/device-mapper/unstriped.txt
@@ -122,3 +122,8 @@ dmsetup create raid_disk0 --table '0 512 unstriped 4 256 0 /dev/mapper/striped 0
 dmsetup create raid_disk1 --table '0 512 unstriped 4 256 1 /dev/mapper/striped 0'
 dmsetup create raid_disk2 --table '0 512 unstriped 4 256 2 /dev/mapper/striped 0'
 dmsetup create raid_disk3 --table '0 512 unstriped 4 256 3 /dev/mapper/striped 0'
+
+Version history
+---------------
+1.0.0	- initial version
+1.0.1	- support non power of 2 chunk size
diff --git a/drivers/md/dm-unstripe.c b/drivers/md/dm-unstripe.c
index 65f838fa2e99..c06a386bcc79 100644
--- a/drivers/md/dm-unstripe.c
+++ b/drivers/md/dm-unstripe.c
@@ -69,12 +69,6 @@ static int unstripe_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 		goto err;
 	}
 
-	// FIXME: must support non power of 2 chunk_size, dm-stripe.c does
-	if (!is_power_of_2(uc->chunk_size)) {
-		ti->error = "Non power of 2 chunk_size is not supported yet";
-		goto err;
-	}
-
 	if (kstrtouint(argv[2], 10, &uc->unstripe)) {
 		ti->error = "Invalid stripe number";
 		goto err;
@@ -98,7 +92,7 @@ static int unstripe_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 
 	uc->unstripe_offset = uc->unstripe * uc->chunk_size;
 	uc->unstripe_width = (uc->stripes - 1) * uc->chunk_size;
-	uc->chunk_shift = fls(uc->chunk_size) - 1;
+	uc->chunk_shift = is_power_of_2(uc->chunk_size) ? fls(uc->chunk_size) - 1 : 0;
 
 	tmp_len = ti->len;
 	if (sector_div(tmp_len, uc->chunk_size)) {
@@ -129,14 +123,18 @@ static sector_t map_to_core(struct dm_target *ti, struct bio *bio)
 {
 	struct unstripe_c *uc = ti->private;
 	sector_t sector = bio->bi_iter.bi_sector;
+	sector_t tmp_sector = sector;
 
 	/* Shift us up to the right "row" on the stripe */
-	sector += uc->unstripe_width * (sector >> uc->chunk_shift);
+	if (uc->chunk_shift)
+		tmp_sector >>= uc->chunk_shift;
+	else
+		sector_div(tmp_sector, uc->chunk_size);
 
-	/* Account for what stripe we're operating on */
-	sector += uc->unstripe_offset;
+	sector += uc->unstripe_width * tmp_sector;
 
-	return sector;
+	/* Account for what stripe we're operating on */
+	return sector + uc->unstripe_offset;
 }
 
 static int unstripe_map(struct dm_target *ti, struct bio *bio)
@@ -185,7 +183,7 @@ static void unstripe_io_hints(struct dm_target *ti,
 
 static struct target_type unstripe_target = {
 	.name = "unstriped",
-	.version = {1, 0, 0},
+	.version = {1, 0, 1},
 	.module = THIS_MODULE,
 	.ctr = unstripe_ctr,
 	.dtr = unstripe_dtr,
@@ -215,5 +213,6 @@ module_init(dm_unstripe_init);
 module_exit(dm_unstripe_exit);
 
 MODULE_DESCRIPTION(DM_NAME " unstriped target");
+MODULE_ALIAS("dm-unstriped");
 MODULE_AUTHOR("Scott Bauer <scott.bauer@intel.com>");
 MODULE_LICENSE("GPL");
-- 
2.14.3

[PATCH 2/4] dm unstriped: remove superfluous module init error path message

[Heinz Mauelshagen]

Signed-off-by: Heinz Mauelshagen <heinzm@redhat.com>
---
 drivers/md/dm-unstripe.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/md/dm-unstripe.c b/drivers/md/dm-unstripe.c
index c06a386bcc79..790b534c67d8 100644
--- a/drivers/md/dm-unstripe.c
+++ b/drivers/md/dm-unstripe.c
@@ -195,13 +195,7 @@ static struct target_type unstripe_target = {
 
 static int __init dm_unstripe_init(void)
 {
-	int r;
-
-	r = dm_register_target(&unstripe_target);
-	if (r < 0)
-		DMERR("target registration failed");
-
-	return r;
+	return dm_register_target(&unstripe_target);
 }
 
 static void __exit dm_unstripe_exit(void)
-- 
2.14.3

[PATCH 3/4] dm unstriped: rename symbols to conform to target name

[Heinz Mauelshagen]

Rename all "unstripe_" symbol substrings to "unstriped_".

Rename 'struct unstriped_c' member 'unstripe' to
more meaningful 'unstriped_idx'

Signed-off-by: Heinz Mauelshagen <heinzm@redhat.com>
---
 drivers/md/dm-unstripe.c | 76 ++++++++++++++++++++++++------------------------
 1 file changed, 38 insertions(+), 38 deletions(-)

diff --git a/drivers/md/dm-unstripe.c b/drivers/md/dm-unstripe.c
index 790b534c67d8..c1eddd1f7972 100644
--- a/drivers/md/dm-unstripe.c
+++ b/drivers/md/dm-unstripe.c
@@ -14,15 +14,15 @@
 #include <linux/bitops.h>
 #include <linux/device-mapper.h>
 
-struct unstripe_c {
+struct unstriped_c {
 	struct dm_dev *dev;
 	sector_t physical_start;
 
 	uint32_t stripes;
 
-	uint32_t unstripe;
-	sector_t unstripe_width;
-	sector_t unstripe_offset;
+	uint32_t unstriped_idx;
+	sector_t unstriped_width;
+	sector_t unstriped_offset;
 
 	uint32_t chunk_size;
 	u8 chunk_shift;
@@ -30,7 +30,7 @@ struct unstripe_c {
 
 #define DM_MSG_PREFIX "unstriped"
 
-static void cleanup_unstripe(struct unstripe_c *uc, struct dm_target *ti)
+static void cleanup_unstripe(struct unstriped_c *uc, struct dm_target *ti)
 {
 	if (uc->dev)
 		dm_put_device(ti, uc->dev);
@@ -41,9 +41,9 @@ static void cleanup_unstripe(struct unstripe_c *uc, struct dm_target *ti)
  * Contruct an unstriped mapping.
  * <number of stripes> <chunk size> <stripe #> <dev_path> <offset>
  */
-static int unstripe_ctr(struct dm_target *ti, unsigned int argc, char **argv)
+static int unstriped_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 {
-	struct unstripe_c *uc;
+	struct unstriped_c *uc;
 	sector_t tmp_len;
 	unsigned long long start;
 	char dummy;
@@ -69,12 +69,12 @@ static int unstripe_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 		goto err;
 	}
 
-	if (kstrtouint(argv[2], 10, &uc->unstripe)) {
+	if (kstrtouint(argv[2], 10, &uc->unstriped_idx)) {
 		ti->error = "Invalid stripe number";
 		goto err;
 	}
 
-	if (uc->unstripe > uc->stripes && uc->stripes > 1) {
+	if (uc->unstriped_idx > uc->stripes && uc->stripes > 1) {
 		ti->error = "Please provide stripe between [0, # of stripes]";
 		goto err;
 	}
@@ -90,8 +90,8 @@ static int unstripe_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 	}
 	uc->physical_start = start;
 
-	uc->unstripe_offset = uc->unstripe * uc->chunk_size;
-	uc->unstripe_width = (uc->stripes - 1) * uc->chunk_size;
+	uc->unstriped_offset = uc->unstriped_idx * uc->chunk_size;
+	uc->unstriped_width = (uc->stripes - 1) * uc->chunk_size;
 	uc->chunk_shift = is_power_of_2(uc->chunk_size) ? fls(uc->chunk_size) - 1 : 0;
 
 	tmp_len = ti->len;
@@ -112,16 +112,16 @@ static int unstripe_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 	return -EINVAL;
 }
 
-static void unstripe_dtr(struct dm_target *ti)
+static void unstriped_dtr(struct dm_target *ti)
 {
-	struct unstripe_c *uc = ti->private;
+	struct unstriped_c *uc = ti->private;
 
 	cleanup_unstripe(uc, ti);
 }
 
 static sector_t map_to_core(struct dm_target *ti, struct bio *bio)
 {
-	struct unstripe_c *uc = ti->private;
+	struct unstriped_c *uc = ti->private;
 	sector_t sector = bio->bi_iter.bi_sector;
 	sector_t tmp_sector = sector;
 
@@ -131,15 +131,15 @@ static sector_t map_to_core(struct dm_target *ti, struct bio *bio)
 	else
 		sector_div(tmp_sector, uc->chunk_size);
 
-	sector += uc->unstripe_width * tmp_sector;
+	sector += uc->unstriped_width * tmp_sector;
 
 	/* Account for what stripe we're operating on */
-	return sector + uc->unstripe_offset;
+	return sector + uc->unstriped_offset;
 }
 
-static int unstripe_map(struct dm_target *ti, struct bio *bio)
+static int unstriped_map(struct dm_target *ti, struct bio *bio)
 {
-	struct unstripe_c *uc = ti->private;
+	struct unstriped_c *uc = ti->private;
 
 	bio_set_dev(bio, uc->dev->bdev);
 	bio->bi_iter.bi_sector = map_to_core(ti, bio) + uc->physical_start;
@@ -147,10 +147,10 @@ static int unstripe_map(struct dm_target *ti, struct bio *bio)
 	return DM_MAPIO_REMAPPED;
 }
 
-static void unstripe_status(struct dm_target *ti, status_type_t type,
+static void unstriped_status(struct dm_target *ti, status_type_t type,
 			    unsigned int status_flags, char *result, unsigned int maxlen)
 {
-	struct unstripe_c *uc = ti->private;
+	struct unstriped_c *uc = ti->private;
 	unsigned int sz = 0;
 
 	switch (type) {
@@ -159,52 +159,52 @@ static void unstripe_status(struct dm_target *ti, status_type_t type,
 
 	case STATUSTYPE_TABLE:
 		DMEMIT("%d %llu %d %s %llu",
-		       uc->stripes, (unsigned long long)uc->chunk_size, uc->unstripe,
+		       uc->stripes, (unsigned long long)uc->chunk_size, uc->unstriped_idx,
 		       uc->dev->name, (unsigned long long)uc->physical_start);
 		break;
 	}
 }
 
-static int unstripe_iterate_devices(struct dm_target *ti,
+static int unstriped_iterate_devices(struct dm_target *ti,
 				    iterate_devices_callout_fn fn, void *data)
 {
-	struct unstripe_c *uc = ti->private;
+	struct unstriped_c *uc = ti->private;
 
 	return fn(ti, uc->dev, uc->physical_start, ti->len, data);
 }
 
-static void unstripe_io_hints(struct dm_target *ti,
+static void unstriped_io_hints(struct dm_target *ti,
 			       struct queue_limits *limits)
 {
-	struct unstripe_c *uc = ti->private;
+	struct unstriped_c *uc = ti->private;
 
 	limits->chunk_sectors = uc->chunk_size;
 }
 
-static struct target_type unstripe_target = {
+static struct target_type unstriped_target = {
 	.name = "unstriped",
 	.version = {1, 0, 1},
 	.module = THIS_MODULE,
-	.ctr = unstripe_ctr,
-	.dtr = unstripe_dtr,
-	.map = unstripe_map,
-	.status = unstripe_status,
-	.iterate_devices = unstripe_iterate_devices,
-	.io_hints = unstripe_io_hints,
+	.ctr = unstriped_ctr,
+	.dtr = unstriped_dtr,
+	.map = unstriped_map,
+	.status = unstriped_status,
+	.iterate_devices = unstriped_iterate_devices,
+	.io_hints = unstriped_io_hints,
 };
 
-static int __init dm_unstripe_init(void)
+static int __init dm_unstriped_init(void)
 {
-	return dm_register_target(&unstripe_target);
+	return dm_register_target(&unstriped_target);
 }
 
-static void __exit dm_unstripe_exit(void)
+static void __exit dm_unstriped_exit(void)
 {
-	dm_unregister_target(&unstripe_target);
+	dm_unregister_target(&unstriped_target);
 }
 
-module_init(dm_unstripe_init);
-module_exit(dm_unstripe_exit);
+module_init(dm_unstriped_init);
+module_exit(dm_unstriped_exit);
 
 MODULE_DESCRIPTION(DM_NAME " unstriped target");
 MODULE_ALIAS("dm-unstriped");
-- 
2.14.3

[PATCH 4/4] dm unstriped: remove unnecessary header includes

[Heinz Mauelshagen]

Signed-off-by: Heinz Mauelshagen <heinzm@redhat.com>
---
 drivers/md/dm-unstripe.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/md/dm-unstripe.c b/drivers/md/dm-unstripe.c
index c1eddd1f7972..41ed30819e58 100644
--- a/drivers/md/dm-unstripe.c
+++ b/drivers/md/dm-unstripe.c
@@ -7,12 +7,6 @@
 #include "dm.h"
 
 #include <linux/module.h>
-#include <linux/init.h>
-#include <linux/blkdev.h>
-#include <linux/bio.h>
-#include <linux/slab.h>
-#include <linux/bitops.h>
-#include <linux/device-mapper.h>
 
 struct unstriped_c {
 	struct dm_dev *dev;
-- 
2.14.3

Re: [PATCH 1/4] dm unstriped: support non power of 2 chunk size

[Scott Bauer]

On Thu, Feb 01, 2018 at 07:09:46PM +0100, Heinz Mauelshagen wrote:
> Address "FIXME: must support non power of 2 chunk_size, dm-stripe.c does".
> 
> Bump target version to indicate change.
> 
> Allow for module loading adding alias as long as we keep dm-unstripe.ko.
> 
> Signed-off-by: Heinz Mauelshagen <heinzm@redhat.com>
Looks good:

Tested-by: Scott Bauer <Scott.Bauer@intel.com>
Reviewed-by: Scott Bauer <Scott.Bauer@intel.com>

Re: [PATCH 2/4] dm unstriped: remove superfluous module init error path message

[Scott Bauer]

esOn Thu, Feb 01, 2018 at 07:09:47PM +0100, Heinz Mauelshagen wrote:
> Signed-off-by: Heinz Mauelshagen <heinzm@redhat.com>
Looks fine
Reviewed-by: Scott Bauer <Scott.Bauer@intel.com>

Re: [PATCH 3/4] dm unstriped: rename symbols to conform to target name

[Mike Snitzer]

On Thu, Feb 01 2018 at  1:06pm -0500,
Heinz Mauelshagen <heinzm@redhat.com> wrote:

> Rename all "unstripe_" symbol substrings to "unstriped_".
> 
> Rename 'struct unstriped_c' member 'unstripe' to
> more meaningful 'unstriped_idx'
> 
> Signed-off-by: Heinz Mauelshagen <heinzm@redhat.com>

dm-stripe.c has comparable differences.  I'd prefer to just leave it.
Not seeing a big benefit to these renames.

Mike