dm unstriped: add new target
Subsystems (3)
v4 cover dm unstriped: add new target
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.
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
Signature · 1 line
2.14.3
PATCH v4 1/2 dm unstriped: the target
--- 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
drivers/md/Kconfig +11 -0
@@ -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
drivers/md/Makefile +1 -0
@@ -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
drivers/md/dm-unstripe.c new file +233 -0
233 lines not on this page · raw message
Signature · 6 lines
2.14.3 -- dm-devel mailing list [email protected] https://www.redhat.com/mailman/listinfo/dm-devel
PATCH v4 2/2 dm unstriped: add documentation for target
In reply to Heinz Mauelshagen--- Documentation/device-mapper/dm-unstripe.txt | 130 ++++++++++++++++++++++++++++ 1 file changed, 130 insertions(+) create mode 100644 Documentation/device-mapper/dm-unstripe.txt
Documentation/device-mapper/dm-unstripe.txt new file +130 -0
@@ -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' +
Signature · 6 lines
2.14.3 -- dm-devel mailing list [email protected] https://www.redhat.com/mailman/listinfo/dm-devel
PATCH v5 1/1 dm: add unstriped target
In reply to Heinz MauelshagenFrom: Scott Bauer <[email protected]>
[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.
--- 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
Documentation/device-mapper/unstriped.txt new file +124 -0
@@ -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'
drivers/md/Kconfig +7 -0
@@ -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
drivers/md/Makefile +1 -0
@@ -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
drivers/md/dm-unstripe.c new file +227 -0
227 lines not on this page · raw message
Signature · 1 line
2.15.0
Re: [PATCH v5] dm: add unstriped target
Quoted 6 lines · Mike Snitzer
From: Scott Bauer <[email protected]>
[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.
Re: [PATCH v5] dm: add unstriped target
In reply to Mike SnitzerQuoted 15 lines
drivers/md/dm-unstripe.c new file +15
-0+#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]
Quoted 18 lines
drivers/md/dm-unstripe.c new file +18
-0+ 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]
drivers/md/dm-unstripe.c line 184
Quoted 7 lines
drivers/md/dm-unstripe.c new file +7
-0+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
drivers/md/dm-unstripe.c line 115
Quoted 5 lines
drivers/md/dm-unstripe.c new file +5
-0+ 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
In reply to Mike Snitzerdrivers/md/dm-unstripe.c line 148
Quoted 12 lines
drivers/md/dm-unstripe.c new file +12
-0+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
In reply to Mike SnitzerReview
Quoted 1 line · Mike Snitzer
(quoted diff) +1
-0+ if (sscanf(argv[1], "%llu%c", &start, &dummy) != 1) {
^ should be argv[4]
Review
Quoted 4 lines
(quoted diff) +4
-0+ 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
drivers/md/dm-unstripe.c +4 -3
@@ -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
Quoted 13 lines · Scott Bauer
Review
Quoted 1 line · Mike Snitzer
(quoted diff) +1
-0+ if (sscanf(argv[1], "%llu%c", &start, &dummy) != 1) {^ should be argv[4]
Review
Quoted 4 lines
(quoted diff) +4
-0+ 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.
Quoted 6 lines
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!
Quoted 5 lines
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.
drivers/md/dm-unstripe.c +2 -4
@@ -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
Quoted 28 lines · Mike Snitzer
Quoted 13 lines · Scott Bauer
Review
Quoted 1 line · Mike Snitzer
(quoted diff) +1
-0+ if (sscanf(argv[1], "%llu%c", &start, &dummy) != 1) {^ should be argv[4]
Review
Quoted 4 lines
(quoted diff) +4
-0+ 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.Quoted 6 lines
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!
Quoted 7 lines
Quoted 5 lines
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?
drivers/md/dm-unstripe.c +7 -9
@@ -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
Quoted 35 lines · Scott Bauer
Quoted 34 lines · Mike Snitzer
Quoted 13 lines · Scott Bauer
Review
Quoted 1 line · Mike Snitzer
(quoted diff) +1
-0+ if (sscanf(argv[1], "%llu%c", &start, &dummy) != 1) {^ should be argv[4]
Review
Quoted 4 lines
(quoted diff) +4
-0+ 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.Quoted 6 lines
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!
Quoted 5 lines
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.
Quoted 7 lines
Quoted 1 line
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