[PATCH v4 0/2] dm unstriped: add new target

[PATCH v4 0/2] dm unstriped: add new target

[Heinz Mauelshagen]

This is a quick rework of the new "unstripe" target
(undoes a striped/raid0 mapping) v3 patch series
authored by Scott Bauer addressing some points
discussed here and more.

Addressed:
- rename target to "unstriped"
- enhance constructor header
- add more consistency checks to constructor imposing
  divisibility of device and drive length by chunk size;
  device length has to be divisible by total number of drives
- use destructor in constructor error pathes
- move a few local variables to 'struct unstripe'
- add missing unstripe_status() function using
  new variables in ^ struct to address requirement
  for dm targets
- tweaked config variable name Kconfig text

Not addressed:
- documentation (kept as is in this series for completeness) 

Example of 'dmsetup table' output:
# dmsetup table nvm-s s1 s2
nvm-s: 0 262144 raid raid0 1 8 2 - 254:2 - 254:3
s1: 0 131072 unstriped 254:4 0 2 8
s2: 0 131072 unstriped 254:4 1 2 8


Scott,
please check, test and tell, if the additonal constraints
on length divisibility are too restrictive and how to go
about the 2 FIXMEs.


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

Heinz Mauelshagen (2):
  dm unstriped: new "unstriped" raid0/striped device
  dm unstriped: add documentation for unstriped target

 Documentation/device-mapper/dm-unstripe.txt | 130 ++++++++++++++++
 drivers/md/Kconfig                          |  11 ++
 drivers/md/Makefile                         |   1 +
 drivers/md/dm-unstripe.c                    | 233 ++++++++++++++++++++++++++++
 4 files changed, 375 insertions(+)
 create mode 100644 Documentation/device-mapper/dm-unstripe.txt
 create mode 100644 drivers/md/dm-unstripe.c

-- 
2.14.3

[PATCH v4 1/2] dm unstriped: the target

[Heinz Mauelshagen]

Signed-off-by: Heinz Mauelshagen <heinzm@redhat.com>
---
 drivers/md/Kconfig       |  11 +++
 drivers/md/Makefile      |   1 +
 drivers/md/dm-unstripe.c | 233 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 245 insertions(+)
 create mode 100644 drivers/md/dm-unstripe.c

diff --git a/drivers/md/Kconfig b/drivers/md/Kconfig
index 83b9362be09c..7f8099f53ad6 100644
--- a/drivers/md/Kconfig
+++ b/drivers/md/Kconfig
@@ -269,6 +269,17 @@ config DM_BIO_PRISON
 
 source "drivers/md/persistent-data/Kconfig"
 
+config DM_UNSTRIPED
+       tristate "Transpose IO to individual drives on a raid device"
+       depends on BLK_DEV_DM
+       ---help---
+         Enable this feature if you with to unstripe I/O on a RAID 0
+	 device to the respective drive. If your hardware has physical
+	 RAID 0 this module can unstripe the I/O to respective controllers
+	 on the stripes.
+
+	 If unsure say N.
+
 config DM_CRYPT
 	tristate "Crypt target support"
 	depends on BLK_DEV_DM
diff --git a/drivers/md/Makefile b/drivers/md/Makefile
index f701bb211783..63255f3ebd97 100644
--- a/drivers/md/Makefile
+++ b/drivers/md/Makefile
@@ -43,6 +43,7 @@ obj-$(CONFIG_BCACHE)		+= bcache/
 obj-$(CONFIG_BLK_DEV_MD)	+= md-mod.o
 obj-$(CONFIG_BLK_DEV_DM)	+= dm-mod.o
 obj-$(CONFIG_BLK_DEV_DM_BUILTIN) += dm-builtin.o
+obj-$(CONFIG_DM_UNSTRIPED)	+= dm-unstripe.o
 obj-$(CONFIG_DM_BUFIO)		+= dm-bufio.o
 obj-$(CONFIG_DM_BIO_PRISON)	+= dm-bio-prison.o
 obj-$(CONFIG_DM_CRYPT)		+= dm-crypt.o
diff --git a/drivers/md/dm-unstripe.c b/drivers/md/dm-unstripe.c
new file mode 100644
index 000000000000..a590e1544dcf
--- /dev/null
+++ b/drivers/md/dm-unstripe.c
@@ -0,0 +1,233 @@
+/*
+ * Copyright © 2017 Intel Corporation
+ *
+ * Authors:
+ *    Scott  Bauer      <scott.bauer@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ */
+
+#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 unstripe {
+	struct dm_dev *ddisk;
+	sector_t chunk_sectors;
+	sector_t stripe_sectors;
+	sector_t cur_drive_chunk_sectors;
+	unsigned int chunk_size;
+	u8 chunk_shift;
+	u8 cur_drive;
+	u8 total_drives;
+};
+
+#define DM_MSG_PREFIX "dm-unstriped"
+
+static void unstripe_dtr(struct dm_target *ti)
+{
+	struct unstripe *ut = ti->private;
+
+	if (ut->ddisk)
+		dm_put_device(ti, ut->ddisk);
+	kfree(ut);
+}
+
+/*
+ * Construct an "unstriped" mapping:
+ *
+ * Args:
+ *	<unstriped_dev> <extract_dev#> <#total_devs> <chunk_size>
+ *
+ * <unstriped_dev> device path of the raid0 device to unstripe (e.g. /dev/nvme0)
+ * <extract_dev#>  index starting at 0 of device to extract
+ * <#total_devs>   number of total, extractable devices
+ */
+static int unstripe_ctr(struct dm_target *ti, unsigned int argc, char **argv)
+{
+	int r = -EINVAL;
+	sector_t sectors;
+	struct unstripe *ut;
+
+	if (argc != 4) {
+		ti->error = "Invalid numver of arguments!";
+		return -EINVAL;
+	}
+
+	/* FIXME: interesting to allow multi-segment mappings for some testing? */
+	if (ti->begin) {
+		ti->error = "Non-zero target begin!";
+		return -EINVAL;
+	}
+
+	ut = kzalloc(sizeof(*ut), GFP_KERNEL);
+	if (!ut) {
+		ti->error = "Failed to allocate space for target!";
+		return -ENOMEM;
+	}
+
+	if (sscanf(argv[1], "%hhu", &ut->cur_drive) != 1 ||
+	    sscanf(argv[2], "%hhu", &ut->total_drives) != 1 ||
+	    sscanf(argv[3], "%u", &ut->chunk_size) != 1) {
+		ti->error = "Error parsing drive/chunk arguments!";
+		goto err;
+	}
+
+	if (!ut->total_drives || (ut->cur_drive > ut->total_drives && ut->total_drives > 1)) {
+		ti->error = "Please provide drive between [0,total_devices]";
+		goto err;
+	}
+
+	if (ut->chunk_size && !is_power_of_2(ut->chunk_size)) {
+		ti->error = "Non power of 2 chunk size!";
+		goto err;
+	}
+
+	r = dm_get_device(ti, argv[0], dm_table_get_mode(ti->table), &ut->ddisk);
+	if (r) {
+		ti->error = "dm-unstripe dev lookup failure!";
+		goto err;
+	}
+
+	/* FIXME: uspace should provide proper chunk size! */
+	/* Given chunk sectors or max hw sectors if 0 */
+	ut->chunk_sectors = ut->chunk_size ?: queue_max_hw_sectors(bdev_get_queue(ut->ddisk->bdev));
+	ut->cur_drive_chunk_sectors = ut->cur_drive * ut->chunk_sectors;
+	ut->stripe_sectors = (ut->total_drives - 1) * ut->chunk_sectors;
+	ut->chunk_shift = fls(ut->chunk_sectors) - 1;
+
+	/* Check uspace provided proper device length divisible by total drives */
+	sectors = i_size_read(ut->ddisk->bdev->bd_inode) >> SECTOR_SHIFT;
+	if (do_div(sectors, ut->total_drives)) {
+		ti->error = "Device length not divisible by total number of drives!";
+		goto err;
+	}
+
+	/* Check uspace provided proper target length! */
+	if (sectors != ti->len) {
+		ti->error = "Drive length not equal to target length!";
+		goto err;
+	}
+
+	/* Check uspace provided proper drive length divisible by chunk size */
+	if (do_div(sectors, ut->chunk_sectors)) {
+		ti->error = "Drive length not divisible by chunk size";
+		goto err;
+	}
+
+	r = dm_set_target_max_io_len(ti, ut->chunk_sectors);
+	if (r) {
+		ti->error = "Failed to set max io len!";
+		goto err;
+	}
+
+	ti->private = ut;
+	return 0;
+err:
+	unstripe_dtr(ti);
+	return r;
+}
+
+static sector_t map_to_core(struct dm_target *ti, struct bio *bio)
+{
+	struct unstripe *ut = ti->private;
+	sector_t sec = bio->bi_iter.bi_sector;
+
+	/* Account for what drive we're operating on */
+	sec += ut->cur_drive_chunk_sectors;
+
+	/* Shift us up to the right "row" on the drive*/
+	sec += ut->stripe_sectors * (sec >> ut->chunk_shift);
+	return sec;
+}
+
+static int unstripe_map(struct dm_target *ti, struct bio *bio)
+{
+	struct unstripe *ut = ti->private;
+
+	bio_set_dev(bio, ut->ddisk->bdev);
+	bio->bi_iter.bi_sector = map_to_core(ti, bio);
+	return DM_MAPIO_REMAPPED;
+}
+
+static void unstripe_status(struct dm_target *ti, status_type_t type,
+			    unsigned int status_flags, char *result, unsigned int maxlen)
+{
+	struct unstripe *ut = ti->private;
+	unsigned int sz = 0;
+
+	switch (type) {
+	case STATUSTYPE_INFO:
+		break;
+
+	case STATUSTYPE_TABLE:
+		DMEMIT("%s %hhu %hhu %u",
+		       ut->ddisk->name, ut->cur_drive, ut->total_drives, ut->chunk_size);
+	}
+}
+
+static void unstripe_iohints(struct dm_target *ti,
+			struct queue_limits *limits)
+{
+	struct unstripe *ut = ti->private;
+	struct queue_limits *lim = &bdev_get_queue(ut->ddisk->bdev)->limits;
+
+	blk_limits_io_min(limits, lim->io_min);
+	blk_limits_io_opt(limits, lim->io_opt);
+	limits->chunk_sectors = ut->chunk_sectors;
+}
+
+static int unstripe_iterate(struct dm_target *ti, iterate_devices_callout_fn fn,
+		       void *data)
+{
+	struct unstripe *ut = ti->private;
+
+	return fn(ti, ut->ddisk, 0, ti->len, data);
+}
+
+static struct target_type unstripe_target = {
+	.name = "unstriped",
+	.version = {1, 0, 0},
+	.module = THIS_MODULE,
+	.ctr = unstripe_ctr,
+	.dtr = unstripe_dtr,
+	.map = unstripe_map,
+	.status = unstripe_status,
+	.iterate_devices = unstripe_iterate,
+	.io_hints = unstripe_iohints,
+};
+
+static int __init dm_unstripe_init(void)
+{
+	int r = dm_register_target(&unstripe_target);
+
+	if (r < 0)
+		DMERR("register failed %d", r);
+
+	return r;
+}
+
+static void __exit dm_unstripe_exit(void)
+{
+	dm_unregister_target(&unstripe_target);
+}
+
+module_init(dm_unstripe_init);
+module_exit(dm_unstripe_exit);
+
+MODULE_DESCRIPTION(DM_NAME " DM unstripe");
+MODULE_ALIAS("dm-unstripe");
+MODULE_AUTHOR("Scott Bauer <scott.bauer@intel.com>");
+MODULE_LICENSE("GPL");
-- 
2.14.3

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[PATCH v4 2/2] dm unstriped: add documentation for target

[Heinz Mauelshagen]

---
 Documentation/device-mapper/dm-unstripe.txt | 130 ++++++++++++++++++++++++++++
 1 file changed, 130 insertions(+)
 create mode 100644 Documentation/device-mapper/dm-unstripe.txt

diff --git a/Documentation/device-mapper/dm-unstripe.txt b/Documentation/device-mapper/dm-unstripe.txt
new file mode 100644
index 000000000000..01d7194b9075
--- /dev/null
+++ b/Documentation/device-mapper/dm-unstripe.txt
@@ -0,0 +1,130 @@
+Device-Mapper Unstripe
+=====================
+
+The device-mapper unstripe (dm-unstripe) target provides a transparent
+mechanism to unstripe a device-mapper "striped" target to access the
+underlying disks without having to touch the true backing block-device.
+It can also be used to unstripe a hardware RAID-0 to access backing disks
+as well.
+
+
+Parameters:
+<drive (ex: /dev/nvme0n1)> <drive #> <# of drives> <chunk sectors>
+
+
+<drive>
+	The block device you wish to unstripe.
+
+<drive #>
+        The physical drive you wish to expose via this "virtual" device
+	mapper target. This must be 0 indexed.
+
+<# of drives>
+        The number of drives in the RAID 0.
+
+<chunk sectors>
+	The amount of 512B sectors in the chunk striping, or zero, if you
+	wish you use max_hw_sector_size.
+
+
+Why use this module?
+=====================
+
+    An example of undoing an existing dm-stripe:
+
+    This small bash script will setup 4 loop devices and use the existing
+    dm-stripe target to combine the 4 devices into one. It then will use
+    the unstripe target on the new combined stripe device to access the
+    individual backing loop devices. We write data to the newly exposed
+    unstriped devices and verify the data written matches the correct
+    underlying device on the striped array.
+
+    #!/bin/bash
+
+    MEMBER_SIZE=$((128 * 1024 * 1024))
+    NUM=4
+    SEQ_END=$((${NUM}-1))
+    CHUNK=256
+    BS=4096
+
+    RAID_SIZE=$((${MEMBER_SIZE}*${NUM}/512))
+    DM_PARMS="0 ${RAID_SIZE} striped ${NUM} ${CHUNK}"
+    COUNT=$((${MEMBER_SIZE} / ${BS}))
+
+    for i in $(seq 0 ${SEQ_END}); do
+      dd if=/dev/zero of=member-${i} bs=${MEMBER_SIZE} count=1 oflag=direct
+      losetup /dev/loop${i} member-${i}
+      DM_PARMS+=" /dev/loop${i} 0"
+    done
+
+    echo $DM_PARMS | dmsetup create raid0
+    for i in $(seq 0 ${SEQ_END}); do
+      echo "0 1 unstripe /dev/mapper/raid0 ${i} ${NUM} ${CHUNK}" | dmsetup create set-${i}
+    done;
+
+    for i in $(seq 0 ${SEQ_END}); do
+      dd if=/dev/urandom of=/dev/mapper/set-${i} bs=${BS} count=${COUNT} oflag=direct
+      diff /dev/mapper/set-${i} member-${i}
+    done;
+
+    for i in $(seq 0 ${SEQ_END}); do
+      dmsetup remove set-${i}
+    done
+
+    dmsetup remove raid0
+
+    for i in $(seq 0 ${SEQ_END}); do
+      losetup -d /dev/loop${i}
+      rm -f member-${i}
+    done
+
+==============
+
+
+    Another example:
+
+    Intel NVMe drives contain two cores on the physical device.
+    Each core of the drive has segregated access to its LBA range.
+    The current LBA model has a RAID 0 128k chunk on each core, resulting
+    in a 256k stripe across the two cores:
+
+       Core 0:                Core 1:
+      __________            __________
+      | LBA 512|            | LBA 768|
+      | LBA 0  |            | LBA 256|
+      ⎻⎻⎻⎻⎻⎻⎻⎻⎻⎻            ⎻⎻⎻⎻⎻⎻⎻⎻⎻⎻
+
+    The purpose of this unstriping is to provide better QoS in noisy
+    neighbor environments. When two partitions are created on the
+    aggregate drive without this unstriping, reads on one partition
+    can affect writes on another partition. This is because the partitions
+    are striped across the two cores. When we unstripe this hardware RAID 0
+    and make partitions on each new exposed device the two partitions are now
+    physically separated.
+
+    With the module we were able to segregate a fio script that has read and
+    write jobs that are independent of each other. Compared to when we run
+    the test on a combined drive with partitions, we were able to get a 92%
+    reduction in five-9ths read latency using this device mapper target.
+
+
+====================
+Example scripts:
+
+
+dmsetup create nvmset1 --table '0 1 unstripe /dev/nvme0n1 1 2 0'
+dmsetup create nvmset0 --table '0 1 unstripe /dev/nvme0n1 0 2 0'
+
+There will now be two mappers:
+/dev/mapper/nvmset1
+/dev/mapper/nvmset0
+
+that will expose core 0 and core 1.
+
+
+# In a dm-stripe with 4 drives of chunk size 128K:
+dmsetup create raid_disk0 --table '0 1 unstripe /dev/mapper/striped 0 4 256'
+dmsetup create raid_disk1 --table '0 1 unstripe /dev/mapper/striped 1 4 256'
+dmsetup create raid_disk2 --table '0 1 unstripe /dev/mapper/striped 2 4 256'
+dmsetup create raid_disk3 --table '0 1 unstripe /dev/mapper/striped 3 4 256'
+
-- 
2.14.3

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [PATCH v5] dm: add unstriped target

[Scott Bauer]

On Mon, Dec 18, 2017 at 06:22:33PM -0500, Mike Snitzer wrote:
> From: Scott Bauer <scott.bauer@intel.com>
> 
> [Preamble: I started with v3 and then folded in v4 and then refactored.
> I've only compile tested... ran out of time to test today.  Please
> review and let me know what you think, if you're OK with this I'll get
> it staged for 4.16. Thanks!]

I will pull this in and test tonight, hope to have an okay/comments by
mid-morning tomorrow.

Thanks for the help.

[PATCH v5] dm: add unstriped target

[Mike Snitzer]

From: Scott Bauer <scott.bauer@intel.com>

[Preamble: I started with v3 and then folded in v4 and then refactored.
I've only compile tested... ran out of time to test today.  Please
review and let me know what you think, if you're OK with this I'll get
it staged for 4.16. Thanks!]

This device mapper unstriped remaps and unstripes IO so it lands
solely on a single drive in a HW RAID 0 or dm-striped target.

In a 4 drive HW RAID 0 the striped target exposes 1/4th of the LBA range
as a virtual drive.  Each IO to that virtual drive will only be issued
to the 1 drive that was selected of the 4 drives in the HW RAID 0.

This unstriped target is most useful for Intel NVMe drives that have
multiple cores but that do not have firmware control to pin separate LBA
ranges to each discrete cpu core.

Signed-off-by: Scott Bauer <scott.bauer@intel.com>
Signed-off-by: Heinz Mauelshagen <heinzm@redhat.com>
Acked-by: Keith Busch <keith.busch@intel.com>
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
---
 Documentation/device-mapper/unstriped.txt | 124 ++++++++++++++++
 drivers/md/Kconfig                        |   7 +
 drivers/md/Makefile                       |   1 +
 drivers/md/dm-unstripe.c                  | 227 ++++++++++++++++++++++++++++++
 4 files changed, 359 insertions(+)
 create mode 100644 Documentation/device-mapper/unstriped.txt
 create mode 100644 drivers/md/dm-unstripe.c

diff --git a/Documentation/device-mapper/unstriped.txt b/Documentation/device-mapper/unstriped.txt
new file mode 100644
index 000000000000..0b2a306c54ee
--- /dev/null
+++ b/Documentation/device-mapper/unstriped.txt
@@ -0,0 +1,124 @@
+Introduction
+============
+
+The device-mapper "unstriped" target provides a transparent mechanism to
+unstripe a device-mapper "striped" target to access the underlying disks
+without having to touch the true backing block-device.  It can also be
+used to unstripe a hardware RAID-0 to access backing disks.
+
+Parameters:
+<number of stripes> <chunk size> <stripe #> <dev_path> <offset>
+
+<number of stripes>
+        The number of stripes in the RAID 0.
+
+<chunk size>
+	The amount of 512B sectors in the chunk striping.
+
+<dev_path>
+	The block device you wish to unstripe.
+
+<stripe #>
+        The stripe number within the device that corresponds to physical
+        drive you wish to unstripe.  This must be 0 indexed.
+
+
+Why use this module?
+====================
+
+An example of undoing an existing dm-stripe
+-------------------------------------------
+
+This small bash script will setup 4 loop devices and use the existing
+striped target to combine the 4 devices into one.  It then will use
+the unstriped target ontop of the striped device to access the
+individual backing loop devices.  We write data to the newly exposed
+unstriped devices and verify the data written matches the correct
+underlying device on the striped array.
+
+#!/bin/bash
+
+MEMBER_SIZE=$((128 * 1024 * 1024))
+NUM=4
+SEQ_END=$((${NUM}-1))
+CHUNK=256
+BS=4096
+
+RAID_SIZE=$((${MEMBER_SIZE}*${NUM}/512))
+DM_PARMS="0 ${RAID_SIZE} striped ${NUM} ${CHUNK}"
+COUNT=$((${MEMBER_SIZE} / ${BS}))
+
+for i in $(seq 0 ${SEQ_END}); do
+  dd if=/dev/zero of=member-${i} bs=${MEMBER_SIZE} count=1 oflag=direct
+  losetup /dev/loop${i} member-${i}
+  DM_PARMS+=" /dev/loop${i} 0"
+done
+
+echo $DM_PARMS | dmsetup create raid0
+for i in $(seq 0 ${SEQ_END}); do
+  echo "0 1 unstriped ${NUM} ${CHUNK} ${i} /dev/mapper/raid0 0" | dmsetup create set-${i}
+done;
+
+for i in $(seq 0 ${SEQ_END}); do
+  dd if=/dev/urandom of=/dev/mapper/set-${i} bs=${BS} count=${COUNT} oflag=direct
+  diff /dev/mapper/set-${i} member-${i}
+done;
+
+for i in $(seq 0 ${SEQ_END}); do
+  dmsetup remove set-${i}
+done
+
+dmsetup remove raid0
+
+for i in $(seq 0 ${SEQ_END}); do
+  losetup -d /dev/loop${i}
+  rm -f member-${i}
+done
+
+Another example
+---------------
+
+Intel NVMe drives contain two cores on the physical device.
+Each core of the drive has segregated access to its LBA range.
+The current LBA model has a RAID 0 128k chunk on each core, resulting
+in a 256k stripe across the two cores:
+
+   Core 0:       Core 1:
+  __________    __________
+  | LBA 512|    | LBA 768|
+  | LBA 0  |    | LBA 256|
+  ----------    ----------
+
+The purpose of this unstriping is to provide better QoS in noisy
+neighbor environments. When two partitions are created on the
+aggregate drive without this unstriping, reads on one partition
+can affect writes on another partition.  This is because the partitions
+are striped across the two cores.  When we unstripe this hardware RAID 0
+and make partitions on each new exposed device the two partitions are now
+physically separated.
+
+With the dm-unstriped target we're able to segregate an fio script that
+has read and write jobs that are independent of each other.  Compared to
+when we run the test on a combined drive with partitions, we were able
+to get a 92% reduction in read latency using this device mapper target.
+
+
+Example dmsetup usage
+=====================
+
+unstriped ontop of Intel NVMe device that has 2 cores
+-----------------------------------------------------
+dmsetup create nvmset0 --table '0 512 unstriped 2 256 0 /dev/nvme0n1 0'
+dmsetup create nvmset1 --table '0 512 unstriped 2 256 1 /dev/nvme0n1 0'
+
+There will now be two devices that expose Intel NVMe core 0 and 1
+respectively:
+/dev/mapper/nvmset0
+/dev/mapper/nvmset1
+
+unstriped ontop of striped with 4 drives using 128K chunk size
+--------------------------------------------------------------
+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'
diff --git a/drivers/md/Kconfig b/drivers/md/Kconfig
index 83b9362be09c..7843cf86f74a 100644
--- a/drivers/md/Kconfig
+++ b/drivers/md/Kconfig
@@ -269,6 +269,13 @@ config DM_BIO_PRISON
 
 source "drivers/md/persistent-data/Kconfig"
 
+config DM_UNSTRIPED
+       tristate "Unstriped target"
+       depends on BLK_DEV_DM
+       ---help---
+	  Issue I/O to individual drives of a striped RAID device.
+	  Provides direct access to 1/N of an N-way RAID 0 device.
+
 config DM_CRYPT
 	tristate "Crypt target support"
 	depends on BLK_DEV_DM
diff --git a/drivers/md/Makefile b/drivers/md/Makefile
index f701bb211783..63255f3ebd97 100644
--- a/drivers/md/Makefile
+++ b/drivers/md/Makefile
@@ -43,6 +43,7 @@ obj-$(CONFIG_BCACHE)		+= bcache/
 obj-$(CONFIG_BLK_DEV_MD)	+= md-mod.o
 obj-$(CONFIG_BLK_DEV_DM)	+= dm-mod.o
 obj-$(CONFIG_BLK_DEV_DM_BUILTIN) += dm-builtin.o
+obj-$(CONFIG_DM_UNSTRIPED)	+= dm-unstripe.o
 obj-$(CONFIG_DM_BUFIO)		+= dm-bufio.o
 obj-$(CONFIG_DM_BIO_PRISON)	+= dm-bio-prison.o
 obj-$(CONFIG_DM_CRYPT)		+= dm-crypt.o
diff --git a/drivers/md/dm-unstripe.c b/drivers/md/dm-unstripe.c
new file mode 100644
index 000000000000..b6f641dcbdee
--- /dev/null
+++ b/drivers/md/dm-unstripe.c
@@ -0,0 +1,227 @@
+/*
+ * Copyright (C) 2017 Intel Corporation.
+ *
+ * This file is released under the GPL.
+ */
+
+#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 unstripe_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 chunk_size;
+	u8 chunk_shift;
+};
+
+#define DM_MSG_PREFIX "unstriped"
+
+void cleanup_unstripe(struct unstripe_c *uc, struct dm_target *ti)
+{
+	if (uc->dev)
+		dm_put_device(ti, uc->dev);
+	kfree(uc);
+}
+
+/*
+ * 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)
+{
+	struct unstripe_c *uc;
+	sector_t width, tmp_len;
+	unsigned long long start;
+	char dummy;
+	int r = -EINVAL;
+
+	if (argc != 5) {
+		ti->error = "Invalid number of arguments";
+		return -EINVAL;
+	}
+
+	uc = kzalloc(sizeof(*uc), GFP_KERNEL);
+	if (!uc) {
+		ti->error = "Memory allocation for unstriped context failed";
+		return -ENOMEM;
+	}
+
+	if (kstrtouint(argv[0], 10, &uc->stripes) || !uc->stripes) {
+		ti->error = "Invalid stripe count";
+		goto err;
+	}
+
+	if (kstrtouint(argv[1], 10, &uc->chunk_size) || !uc->chunk_size) {
+		ti->error = "Invalid chunk_size";
+		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;
+	}
+
+	if (uc->unstripe > uc->stripes && uc->stripes > 1) {
+		ti->error = "Please provide stripe between [0, # of stripes]";
+		goto err;
+	}
+
+	r = dm_get_device(ti, argv[3], dm_table_get_mode(ti->table), &uc->dev);
+	if (r) {
+		ti->error = "Couldn't get striped device";
+		goto err;
+	}
+
+	if (sscanf(argv[1], "%llu%c", &start, &dummy) != 1) {
+		ti->error = "Invalid striped device offset";
+		goto err;
+	}
+	uc->physical_start = start;
+
+	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;
+
+	width = ti->len;
+	if (sector_div(width, uc->stripes)) {
+		ti->error = "Target length not divisible by number of stripes";
+		goto err;
+	}
+
+	tmp_len = width;
+	if (sector_div(tmp_len, uc->chunk_size)) {
+		ti->error = "Target length not divisible by chunk size";
+		goto err;
+	}
+
+	r = dm_set_target_max_io_len(ti, uc->chunk_size);
+	if (r) {
+		ti->error = "Failed to set max io len";
+		goto err;
+	}
+
+	ti->private = uc;
+	return 0;
+err:
+	cleanup_unstripe(uc, ti);
+	return r;
+}
+
+static void unstripe_dtr(struct dm_target *ti)
+{
+	struct unstripe_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;
+	sector_t sector = bio->bi_iter.bi_sector;
+
+	/* Account for what stripe we're operating on */
+	sector += uc->unstripe_offset;
+
+	/* Shift us up to the right "row" on the stripe */
+	sector += uc->unstripe_width * (sector >> uc->chunk_shift);
+	return sector;
+}
+
+static int unstripe_map(struct dm_target *ti, struct bio *bio)
+{
+	struct unstripe_c *uc = ti->private;
+
+	bio_set_dev(bio, uc->dev->bdev);
+	bio->bi_iter.bi_sector = map_to_core(ti, bio) + uc->physical_start;
+
+	return DM_MAPIO_REMAPPED;
+}
+
+static void unstripe_status(struct dm_target *ti, status_type_t type,
+			    unsigned int status_flags, char *result, unsigned int maxlen)
+{
+	struct unstripe_c *uc = ti->private;
+	unsigned int sz = 0;
+
+	switch (type) {
+	case STATUSTYPE_INFO:
+		break;
+
+	case STATUSTYPE_TABLE:
+		DMEMIT("%d %llu %d %s %llu",
+		       uc->stripes, (unsigned long long)uc->chunk_size, uc->unstripe,
+		       uc->dev->name, (unsigned long long)uc->physical_start);
+		break;
+	}
+}
+
+static int unstripe_iterate_devices(struct dm_target *ti,
+				    iterate_devices_callout_fn fn, void *data)
+{
+	struct unstripe_c *uc = ti->private;
+
+	return fn(ti, uc->dev, uc->physical_start, ti->len, data);
+}
+
+static void unstripe_io_hints(struct dm_target *ti,
+			       struct queue_limits *limits)
+{
+	struct unstripe_c *uc = ti->private;
+
+	limits->chunk_sectors = uc->chunk_size;
+}
+
+static struct target_type unstripe_target = {
+	.name = "unstriped",
+	.version = {1, 0, 0},
+	.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,
+};
+
+static int __init dm_unstripe_init(void)
+{
+	int r;
+
+	r = dm_register_target(&unstripe_target);
+	if (r < 0)
+		DMERR("target registration failed");
+
+	return r;
+}
+
+static void __exit dm_unstripe_exit(void)
+{
+	dm_unregister_target(&unstripe_target);
+}
+
+module_init(dm_unstripe_init);
+module_exit(dm_unstripe_exit);
+
+MODULE_DESCRIPTION(DM_NAME " unstriped target");
+MODULE_AUTHOR("Scott Bauer <scott.bauer@intel.com>");
+MODULE_LICENSE("GPL");
-- 
2.15.0

Re: [PATCH v5] dm: add unstriped target

[Scott Bauer]

> +#define DM_MSG_PREFIX "unstriped"
> +
> +void cleanup_unstripe(struct unstripe_c *uc, struct dm_target *ti)
> +{
> +	if (uc->dev)
> +		dm_put_device(ti, uc->dev);
> +	kfree(uc);
> +}
> +
> +/*
> + * 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)
> +{

[snip]

> +	tmp_len = width;
> +	if (sector_div(tmp_len, uc->chunk_size)) {
> +		ti->error = "Target length not divisible by chunk size";
> +		goto err;
> +	}
> +
> +	r = dm_set_target_max_io_len(ti, uc->chunk_size);
> +	if (r) {
> +		ti->error = "Failed to set max io len";
> +		goto err;
> +	}
> +
> +	ti->private = uc;
> +	return 0;
> +err:
> +	cleanup_unstripe(uc, ti);
> +	return r;
> +}
[snip]
> +static int unstripe_iterate_devices(struct dm_target *ti,
> +				    iterate_devices_callout_fn fn, void *data)
> +{
> +	struct unstripe_c *uc = ti->private;
> +
> +	return fn(ti, uc->dev, uc->physical_start, ti->len, data);
> +}


I was testing some error conditions:

#1 set up legitimate unstripe.
#2 remove it.
#3 set up an unstripe that fails the

> +     tmp_len = width;
> +     if (sector_div(tmp_len, uc->chunk_size)) {
> +             ti->error = "Target length not divisible by chunk size";
> +             goto err;
> +     }

check in the constructor (so give it an odd length):

I'm greeted by this splat:


[   76.349033] BUG: unable to handle kernel NULL pointer dereference at 0000000066b57171
[   76.349051] IP: unstripe_iterate_devices+0x13/0x20 [dm_unstripe]
[   76.349055] PGD 0 P4D 0 
[   76.349062] Oops: 0000 [#1] SMP
[   76.349065] Modules linked in: dm_unstripe binfmt_misc nls_iso8859_1 snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic snd_hda_intel snd_hda_codec intel_rapl x86_pkg_temp_thermal snd_hda_core intel_powerclamp kvm_intel snd_hwdep snd_pcm kvm snd_seq_midi snd_seq_midi_event snd_rawmidi irqbypass crct10dif_pclmul snd_seq crc32_pclmul ghash_clmulni_intel pcbc snd_seq_device snd_timer aesni_intel snd mei_me aes_x86_64 crypto_simd glue_helper cryptd shpchp mei soundcore mac_hid tpm_crb acpi_pad coretemp parport_pc ppdev lp parport ip_tables x_tables autofs4 hid_generic usbhid hid i915 drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops drm igb mxm_wmi e1000e dca ptp ahci pps_core i2c_algo_bit libahci video wmi
[   76.349153] CPU: 2 PID: 1688 Comm: dmsetup Not tainted 4.15.0-rc2+ #7
[   76.349157] Hardware name: Gigabyte Technology Co., Ltd. Z170X-UD5/Z170X-UD5-CF, BIOS F5 03/07/2016
[   76.349160] task: 00000000dfd78dfc task.stack: 00000000fadda433
[   76.349166] RIP: 0010:unstripe_iterate_devices+0x13/0x20 [dm_unstripe]
[   76.349169] RSP: 0018:ffffb89842d13c00 EFLAGS: 00010286
[   76.349173] RAX: 0000000000000000 RBX: 0000000000000000 RCX: 000000001749c7ff
[   76.349176] RDX: ffffb89842d13c0c RSI: ffffffff902ad590 RDI: ffffb89841ed1040
[   76.349179] RBP: ffffffff902ad590 R08: ffffb89842d13c0c R09: ffffffff902ad590
[   76.349182] R10: ffffb89842d13c68 R11: ffffa020daef0000 R12: ffffa0208ec63600
[   76.349184] R13: 0000000000000001 R14: ffffffff902b29a0 R15: ffffb89842d13d20
[   76.349189] FS:  00007faa3ee63400(0000) GS:ffffa020f1c80000(0000) knlGS:0000000000000000
[   76.349192] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   76.349195] CR2: 0000000000000008 CR3: 000000045c342004 CR4: 00000000003606e0
[   76.349200] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[   76.349203] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[   76.349205] Call Trace:
[   76.349217]  dm_table_has_no_data_devices+0x91/0xc0
[   76.349225]  dm_swap_table+0x6b/0x2e0
[   76.349230]  ? table_load+0x350/0x350
[   76.349234]  ? __dm_suspend+0x7c/0x1d0
[   76.349239]  ? table_load+0x350/0x350
[   76.349243]  dev_suspend+0x95/0x220
[   76.349247]  ctl_ioctl+0x1bf/0x450
[   76.349257]  ? yield_to+0x130/0x1b0
[   76.349261]  dm_ctl_ioctl+0xa/0x10
[   76.349268]  do_vfs_ioctl+0x9f/0x5f0
[   76.349274]  ? SYSC_semctl+0xf9/0x130
[   76.349279]  ? _cond_resched+0x16/0x40
[   76.349285]  ? task_work_run+0x38/0xa0
[   76.349291]  SyS_ioctl+0x74/0x80
[   76.349296]  entry_SYSCALL_64_fastpath+0x1e/0x81
[   76.349301] RIP: 0033:0x7faa3e766587
[   76.349304] RSP: 002b:00007ffcba093b88 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
[   76.349309] RAX: ffffffffffffffda RBX: 000000000d4d1614 RCX: 00007faa3e766587
[   76.349312] RDX: 0000563b73da7190 RSI: 00000000c138fd06 RDI: 0000000000000003
[   76.349314] RBP: 0000000000000004 R08: 00007faa3ea72ba8 R09: 00007ffcba0939f0
[   76.349317] R10: 00007faa3ea720b3 R11: 0000000000000246 R12: 0000563b72eb1190
[   76.349320] R13: 00007ffcba093f14 R14: 00007ffcba093ebc R15: 0000000000018000
[   76.349324] Code: <48> 8b 50 08 48 8b 30 41 ff e1 0f 1f 00 0f 1f 44 00 00 48 8b 47 38 
[   76.349359] RIP: unstripe_iterate_devices+0x13/0x20 [dm_unstripe] RSP: ffffb89842d13c00
[   76.349362] CR2: 0000000000000008
[   76.349366] ---[ end trace f92285c94fd36893 ]---


Two questions:

Do we need to set ti-private = NULL in "cleanup_unstripe"? Also, do we
need to have :


static int unstripe_iterate_devices(struct dm_target *ti,
                                    iterate_devices_callout_fn fn, void *data)
{
        struct unstripe_c *uc = ti->private;

	if (uc)
           return fn(ti, uc->dev, uc->physical_start, ti->len, data);
}

~

Re: [PATCH v5] dm: add unstriped target

[Scott Bauer]

> +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;
> +
> +	/* Account for what stripe we're operating on */
> +	sector += uc->unstripe_offset;
> +
> +	/* Shift us up to the right "row" on the stripe */
> +	sector += uc->unstripe_width * (sector >> uc->chunk_shift);
> +	return sector;
> +}

This doesn't work anymore, previously we had a "group" local variable,
but removed it out. Now we're calculating the group based *after* we
switch up the stripe.

we need to swap the two sector +=  then it will work:

     /* Shift us up to the right "row" on the stripe */
     sector += uc->unstripe_width * (sector >> uc->chunk_shift);

     /* Account for what stripe we're operating on */
     sector += uc->unstripe_offset;

Re: [PATCH v5] dm: add unstriped target

[Scott Bauer]

On Mon, Dec 18, 2017 at 06:22:33PM -0500, Mike Snitzer wrote:

> +	if (sscanf(argv[1], "%llu%c", &start, &dummy) != 1) {
                        ^ should be argv[4]
> +		ti->error = "Invalid striped device offset";
> +		goto err;
> +	}
> +	uc->physical_start = start;

Hi Mike,
Sorry for the bombardment of emails. I think I've fixed the last
problem. Above is the last issue.

Below is a patch you can apply that will fix up the sector switch, I
had mentioned in the previous mail, as well as the wrong argv usage
from above.

I still have not solved the NULL pointer issue, i'll continue to
investigate that. Unless you have an idea of why that is occuring.

You can trigger it without having to create/remove/create. Just a
creation with a bad (odd length) target length will do it.

If you don't want this patch but want me to do a v6 I can do that as
well.

Thank you

diff --git a/drivers/md/dm-unstripe.c b/drivers/md/dm-unstripe.c
index b6f641dcbdee..df62b8a51299 100644
--- a/drivers/md/dm-unstripe.c
+++ b/drivers/md/dm-unstripe.c
@@ -92,7 +92,7 @@ static int unstripe_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 		goto err;
 	}
 
-	if (sscanf(argv[1], "%llu%c", &start, &dummy) != 1) {
+	if (sscanf(argv[4], "%llu%c", &start, &dummy) != 1) {
 		ti->error = "Invalid striped device offset";
 		goto err;
 	}
@@ -139,11 +139,12 @@ 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;
 
+	/* Shift us up to the right "row" on the stripe */
+	sector += uc->unstripe_width * (sector >> uc->chunk_shift);
+
 	/* Account for what stripe we're operating on */
 	sector += uc->unstripe_offset;
 
-	/* Shift us up to the right "row" on the stripe */
-	sector += uc->unstripe_width * (sector >> uc->chunk_shift);
 	return sector;
 }
 

Re: [PATCH v5] dm: add unstriped target

[Mike Snitzer]

On Tue, Dec 19 2017 at  1:35P -0500,
Scott Bauer <scott.bauer@intel.com> wrote:

> On Mon, Dec 18, 2017 at 06:22:33PM -0500, Mike Snitzer wrote:
> 
> > +	if (sscanf(argv[1], "%llu%c", &start, &dummy) != 1) {
>                         ^ should be argv[4]
> > +		ti->error = "Invalid striped device offset";
> > +		goto err;
> > +	}
> > +	uc->physical_start = start;
> 
> Hi Mike,
> Sorry for the bombardment of emails. I think I've fixed the last
> problem. Above is the last issue.
> 
> Below is a patch you can apply that will fix up the sector switch, I
> had mentioned in the previous mail, as well as the wrong argv usage
> from above.
> 
> I still have not solved the NULL pointer issue, i'll continue to
> investigate that. Unless you have an idea of why that is occuring.

See below for incremental patch that should fix the NULL pointer, please
test and I'll fold it in, along with your incremental.

Thanks!

> You can trigger it without having to create/remove/create. Just a
> creation with a bad (odd length) target length will do it.
> 
> If you don't want this patch but want me to do a v6 I can do that as
> well.

I'll take it, no worries on sending out v6.

diff --git a/drivers/md/dm-unstripe.c b/drivers/md/dm-unstripe.c
index b6f641dcbdee..27a8400a96f2 100644
--- a/drivers/md/dm-unstripe.c
+++ b/drivers/md/dm-unstripe.c
@@ -47,7 +47,6 @@ static int unstripe_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 	sector_t width, tmp_len;
 	unsigned long long start;
 	char dummy;
-	int r = -EINVAL;
 
 	if (argc != 5) {
 		ti->error = "Invalid number of arguments";
@@ -86,8 +85,7 @@ static int unstripe_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 		goto err;
 	}
 
-	r = dm_get_device(ti, argv[3], dm_table_get_mode(ti->table), &uc->dev);
-	if (r) {
+	if (dm_get_device(ti, argv[3], dm_table_get_mode(ti->table), &uc->dev))
 		ti->error = "Couldn't get striped device";
 		goto err;
 	}
@@ -124,7 +122,7 @@ static int unstripe_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 	return 0;
 err:
 	cleanup_unstripe(uc, ti);
-	return r;
+	return -EINVAL;
 }
 
 static void unstripe_dtr(struct dm_target *ti)

Re: [PATCH v5] dm: add unstriped target

[Scott Bauer]

On Tue, Dec 19, 2017 at 03:03:53PM -0500, Mike Snitzer wrote:
> On Tue, Dec 19 2017 at  1:35P -0500,
> Scott Bauer <scott.bauer@intel.com> wrote:
> 
> > On Mon, Dec 18, 2017 at 06:22:33PM -0500, Mike Snitzer wrote:
> > 
> > > +	if (sscanf(argv[1], "%llu%c", &start, &dummy) != 1) {
> >                         ^ should be argv[4]
> > > +		ti->error = "Invalid striped device offset";
> > > +		goto err;
> > > +	}
> > > +	uc->physical_start = start;
> > 
> > Hi Mike,
> > Sorry for the bombardment of emails. I think I've fixed the last
> > problem. Above is the last issue.
> > 
> > Below is a patch you can apply that will fix up the sector switch, I
> > had mentioned in the previous mail, as well as the wrong argv usage
> > from above.
> > 
> > I still have not solved the NULL pointer issue, i'll continue to
> > investigate that. Unless you have an idea of why that is occuring.
> 
> See below for incremental patch that should fix the NULL pointer, please
> test and I'll fold it in, along with your incremental.
> 
> Thanks!
> 
> > You can trigger it without having to create/remove/create. Just a
> > creation with a bad (odd length) target length will do it.
> > 
> > If you don't want this patch but want me to do a v6 I can do that as
> > well.
> 
> I'll take it, no worries on sending out v6.

Functionally the code is good now -- Thank you very much for the help.
The last thing is I need to fix up the bash script in documentation.
I *really* have to work on this other project for the rest of the day.
Do you care if we land this now, and I will just submit a small fixup
of the Documentation tomorrow?

diff --git a/drivers/md/dm-unstripe.c b/drivers/md/dm-unstripe.c
index b6f641dcbdee..7a8fd1bfbdad 100644
--- a/drivers/md/dm-unstripe.c
+++ b/drivers/md/dm-unstripe.c
@@ -47,7 +47,6 @@ static int unstripe_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 	sector_t width, tmp_len;
 	unsigned long long start;
 	char dummy;
-	int r = -EINVAL;
 
 	if (argc != 5) {
 		ti->error = "Invalid number of arguments";
@@ -86,13 +85,12 @@ static int unstripe_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 		goto err;
 	}
 
-	r = dm_get_device(ti, argv[3], dm_table_get_mode(ti->table), &uc->dev);
-	if (r) {
+	if (dm_get_device(ti, argv[3], dm_table_get_mode(ti->table), &uc->dev)) {
 		ti->error = "Couldn't get striped device";
 		goto err;
 	}
 
-	if (sscanf(argv[1], "%llu%c", &start, &dummy) != 1) {
+	if (sscanf(argv[4], "%llu%c", &start, &dummy) != 1) {
 		ti->error = "Invalid striped device offset";
 		goto err;
 	}
@@ -114,8 +112,7 @@ static int unstripe_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 		goto err;
 	}
 
-	r = dm_set_target_max_io_len(ti, uc->chunk_size);
-	if (r) {
+	if (dm_set_target_max_io_len(ti, uc->chunk_size)) {
 		ti->error = "Failed to set max io len";
 		goto err;
 	}
@@ -124,7 +121,7 @@ static int unstripe_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 	return 0;
 err:
 	cleanup_unstripe(uc, ti);
-	return r;
+	return -EINVAL;
 }
 
 static void unstripe_dtr(struct dm_target *ti)
@@ -139,11 +136,12 @@ 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;
 
+	/* Shift us up to the right "row" on the stripe */
+	sector += uc->unstripe_width * (sector >> uc->chunk_shift);
+
 	/* Account for what stripe we're operating on */
 	sector += uc->unstripe_offset;
 
-	/* Shift us up to the right "row" on the stripe */
-	sector += uc->unstripe_width * (sector >> uc->chunk_shift);
 	return sector;
 }
 

Re: [PATCH v5] dm: add unstriped target

[Mike Snitzer]

On Tue, Dec 19 2017 at  3:16pm -0500,
Scott Bauer <scott.bauer@intel.com> wrote:

> On Tue, Dec 19, 2017 at 03:03:53PM -0500, Mike Snitzer wrote:
> > On Tue, Dec 19 2017 at  1:35P -0500,
> > Scott Bauer <scott.bauer@intel.com> wrote:
> > 
> > > On Mon, Dec 18, 2017 at 06:22:33PM -0500, Mike Snitzer wrote:
> > > 
> > > > +	if (sscanf(argv[1], "%llu%c", &start, &dummy) != 1) {
> > >                         ^ should be argv[4]
> > > > +		ti->error = "Invalid striped device offset";
> > > > +		goto err;
> > > > +	}
> > > > +	uc->physical_start = start;
> > > 
> > > Hi Mike,
> > > Sorry for the bombardment of emails. I think I've fixed the last
> > > problem. Above is the last issue.
> > > 
> > > Below is a patch you can apply that will fix up the sector switch, I
> > > had mentioned in the previous mail, as well as the wrong argv usage
> > > from above.
> > > 
> > > I still have not solved the NULL pointer issue, i'll continue to
> > > investigate that. Unless you have an idea of why that is occuring.
> > 
> > See below for incremental patch that should fix the NULL pointer, please
> > test and I'll fold it in, along with your incremental.
> > 
> > Thanks!
> > 
> > > You can trigger it without having to create/remove/create. Just a
> > > creation with a bad (odd length) target length will do it.
> > > 
> > > If you don't want this patch but want me to do a v6 I can do that as
> > > well.
> > 
> > I'll take it, no worries on sending out v6.
> 
> Functionally the code is good now -- Thank you very much for the help.
> The last thing is I need to fix up the bash script in documentation.
> I *really* have to work on this other project for the rest of the day.
> Do you care if we land this now, and I will just submit a small fixup
> of the Documentation tomorrow?

Not sure what docs you think need fixing (since I did update them).
Please see what is now staged in linux-next:
https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-4.16&id=0aaa5aeed6921f65dc1e91e6d6ba43f4a8e1458e

But if you have an incremental patch to fix up something in
Documentation I'll gladly take it.

Thanks,
Mike