Changes from v2->v3:
1) Renamed variables in-code to reflect correct terminology with respect
to the dm-stripe target.
2) Fixed a __must_check missing check in the constructor.
3) Used correct types for working with sector remaping.
3) Fixed documentation to reflect the correct termonology, like the code.
Added the test script Keith sent out in an email that makes a striped
device and uses the un-stripe target to access the underlying loop
devices.
Signed-off-by: Scott Bauer <[email protected]>
---
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.11.0
This device mapper module remaps and unstripes IO so it lands
solely on a single drive in a RAID 0/dm-stripe target.
In a 4 drive RAID 0 the mapper exposes 1/4th of the LBA range
as a virtual drive. Each IO to that virtual drive will land on
only one of the 4 drives, selected by the user.
Signed-off-by: Scott Bauer <[email protected]>
Acked-by: Keith Busch <[email protected]>
---
drivers/md/Kconfig | 10 +++
drivers/md/Makefile | 1 +
drivers/md/dm-unstripe.c | 204 +++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 215 insertions(+)
create mode 100644 drivers/md/dm-unstripe.c
diff --git a/drivers/md/Kconfig b/drivers/md/Kconfig
index 83b9362be09c..948874fcc67c 100644
--- a/drivers/md/Kconfig
+++ b/drivers/md/Kconfig
@@ -269,6 +269,16 @@ config DM_BIO_PRISON
source "drivers/md/persistent-data/Kconfig"
+config DM_UN_STRIPE
+ 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 sides.
+
+ 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..2cc380b71319 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_UN_STRIPE) += 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..d1105e92bb3f
--- /dev/null
+++ b/drivers/md/dm-unstripe.c
@@ -0,0 +1,204 @@
+/*
+ * Copyright © 2017 Intel Corporation
+ *
+ * Authors:
+ * Scott Bauer <[email protected]>
+ *
+ * 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;
+ u8 chunk_shift;
+ u8 cur_drive;
+};
+
+
+#define DM_MSG_PREFIX "dm-unstripe"
+static const char *parse_err = "Please provide the necessary information:"
+ "<drive> <device (0 indexed)> <total_devices>"
+ " <chunk size in 512B sectors || 0 to use max hw sector size>";
+
+/*
+ * Argument layout:
+ * <drive> <stripe/drive to extract (0 indexed)>
+ * <total_devices> <chunk size in 512B sect>
+ */
+static int set_ctr(struct dm_target *ti, unsigned int argc, char **argv)
+{
+ struct block_device *bbdev;
+ struct unstripe *target;
+ unsigned int chunk_size;
+ u64 tot_sec, mod;
+ u8 cur_drive, tot_drives;
+ char dummy;
+ int ret;
+
+ if (argc != 4) {
+ DMERR("%s", parse_err);
+ return -EINVAL;
+ }
+
+ if (sscanf(argv[1], "%hhu%c", &cur_drive, &dummy) != 1 ||
+ sscanf(argv[2], "%hhu%c", &tot_drives, &dummy) != 1 ||
+ sscanf(argv[3], "%u%c", &chunk_size, &dummy) != 1) {
+ DMERR("%s", parse_err);
+ return -EINVAL;
+ }
+
+ if (tot_drives == 0 || (cur_drive > tot_drives && tot_drives > 1)) {
+ DMERR("Please provide a drive between [0,%hhu)", tot_drives);
+ return -EINVAL;
+ }
+
+ target = kzalloc(sizeof(*target), GFP_KERNEL);
+
+ if (!target) {
+ DMERR("Failed to allocate space for DM unstripe!");
+ return -ENOMEM;
+ }
+
+ ret = dm_get_device(ti, argv[0], dm_table_get_mode(ti->table),
+ &target->ddisk);
+ if (ret) {
+ kfree(target);
+ DMERR("dm-unstripe dev lookup failure! for drive %s", argv[0]);
+ return ret;
+ }
+
+ bbdev = target->ddisk->bdev;
+
+ target->cur_drive = cur_drive;
+ if (chunk_size)
+ target->chunk_sectors = chunk_size;
+ else
+ target->chunk_sectors =
+ queue_max_hw_sectors(bdev_get_queue(bbdev));
+
+ target->stripe_sectors = (tot_drives - 1) * target->chunk_sectors;
+ target->chunk_shift = fls(target->chunk_sectors) - 1;
+
+ ret = dm_set_target_max_io_len(ti, target->chunk_sectors);
+ if (ret) {
+ dm_put_device(ti, target->ddisk);
+ kfree(target);
+ DMERR("Failed to set max io len!");
+ return ret;
+ }
+ ti->private = target;
+
+ tot_sec = i_size_read(bbdev->bd_inode) >> SECTOR_SHIFT;
+ mod = tot_sec % target->chunk_sectors;
+
+ if (ti->len == 1)
+ ti->len = (tot_sec / tot_drives) - mod;
+ ti->begin = 0;
+ return 0;
+}
+
+static void set_dtr(struct dm_target *ti)
+{
+ struct unstripe *target = ti->private;
+
+ dm_put_device(ti, target->ddisk);
+ kfree(target);
+}
+
+
+static sector_t map_to_core(struct dm_target *ti, struct bio *bio)
+{
+ struct unstripe *target = ti->private;
+ unsigned long long sec = bio->bi_iter.bi_sector;
+ unsigned long long group;
+
+ group = (sec >> target->chunk_shift);
+ /* Account for what drive we're operating on */
+ sec += (target->cur_drive * target->chunk_sectors);
+ /* Shift us up to the right "row" on the drive*/
+ sec += target->stripe_sectors * group;
+ return sec;
+}
+
+static int set_map_bio(struct dm_target *ti, struct bio *bio)
+{
+ struct unstripe *target = ti->private;
+
+ if (bio_sectors(bio))
+ bio->bi_iter.bi_sector = map_to_core(ti, bio);
+
+ bio_set_dev(bio, target->ddisk->bdev);
+ submit_bio(bio);
+ return DM_MAPIO_SUBMITTED;
+}
+
+static void set_iohints(struct dm_target *ti,
+ struct queue_limits *limits)
+{
+ struct unstripe *target = ti->private;
+ struct queue_limits *lim = &bdev_get_queue(target->ddisk->bdev)->limits;
+
+ blk_limits_io_min(limits, lim->io_min);
+ blk_limits_io_opt(limits, lim->io_opt);
+ limits->chunk_sectors = target->chunk_sectors;
+}
+
+static int set_iterate(struct dm_target *ti, iterate_devices_callout_fn fn,
+ void *data)
+{
+ struct unstripe *target = ti->private;
+
+ return fn(ti, target->ddisk, 0, ti->len, data);
+}
+
+static struct target_type iset_target = {
+ .name = "unstripe",
+ .version = {1, 0, 0},
+ .module = THIS_MODULE,
+ .ctr = set_ctr,
+ .dtr = set_dtr,
+ .map = set_map_bio,
+ .iterate_devices = set_iterate,
+ .io_hints = set_iohints,
+};
+
+static int __init dm_unstripe_init(void)
+{
+ int r = dm_register_target(&iset_target);
+
+ if (r < 0)
+ DMERR("register failed %d", r);
+
+ return r;
+}
+
+static void __exit dm_unstripe_exit(void)
+{
+ dm_unregister_target(&iset_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 <[email protected]>");
+MODULE_LICENSE("GPL");
--
2.11.0
On 12/13/2017 01:33 PM, Scott Bauer wrote:
> This device mapper module remaps and unstripes IO so it lands
> solely on a single drive in a RAID 0/dm-stripe target.
> In a 4 drive RAID 0 the mapper exposes 1/4th of the LBA range
> as a virtual drive. Each IO to that virtual drive will land on
> only one of the 4 drives, selected by the user.
>
> Signed-off-by: Scott Bauer <[email protected]>
> Acked-by: Keith Busch <[email protected]>
> ---
> drivers/md/Kconfig | 10 +++
> drivers/md/Makefile | 1 +
> drivers/md/dm-unstripe.c | 204 +++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 215 insertions(+)
> create mode 100644 drivers/md/dm-unstripe.c
>
> diff --git a/drivers/md/Kconfig b/drivers/md/Kconfig
> index 83b9362be09c..948874fcc67c 100644
> --- a/drivers/md/Kconfig
> +++ b/drivers/md/Kconfig
> @@ -269,6 +269,16 @@ config DM_BIO_PRISON
>
> source "drivers/md/persistent-data/Kconfig"
>
> +config DM_UN_STRIPE
> + 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 sides.
Please use tabs (consistenly) at the beginning of each line (after "config").
> +
> + If unsure say N.
> +
> config DM_CRYPT
> tristate "Crypt target support"
> depends on BLK_DEV_DM
> diff --git a/drivers/md/dm-unstripe.c b/drivers/md/dm-unstripe.c
> new file mode 100644
> index 000000000000..d1105e92bb3f
> --- /dev/null
> +++ b/drivers/md/dm-unstripe.c
> @@ -0,0 +1,204 @@
> +/*
> + * Copyright © 2017 Intel Corporation
> + *
> + * Authors:
> + * Scott Bauer <[email protected]>
> + *
> + * 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;
> + u8 chunk_shift;
> + u8 cur_drive;
> +};
> +
> +
> +#define DM_MSG_PREFIX "dm-unstripe"
> +static const char *parse_err = "Please provide the necessary information:"
> + "<drive> <device (0 indexed)> <total_devices>"
> + " <chunk size in 512B sectors || 0 to use max hw sector size>";
> +
> +/*
> + * Argument layout:
> + * <drive> <stripe/drive to extract (0 indexed)>
> + * <total_devices> <chunk size in 512B sect>
> + */
> +static int set_ctr(struct dm_target *ti, unsigned int argc, char **argv)
> +{
> + struct block_device *bbdev;
> + struct unstripe *target;
> + unsigned int chunk_size;
> + u64 tot_sec, mod;
> + u8 cur_drive, tot_drives;
> + char dummy;
> + int ret;
> +
> + if (argc != 4) {
> + DMERR("%s", parse_err);
> + return -EINVAL;
> + }
> +
> + if (sscanf(argv[1], "%hhu%c", &cur_drive, &dummy) != 1 ||
> + sscanf(argv[2], "%hhu%c", &tot_drives, &dummy) != 1 ||
> + sscanf(argv[3], "%u%c", &chunk_size, &dummy) != 1) {
> + DMERR("%s", parse_err);
> + return -EINVAL;
> + }
> +
> + if (tot_drives == 0 || (cur_drive > tot_drives && tot_drives > 1)) {
>=
> + DMERR("Please provide a drive between [0,%hhu)", tot_drives);
> + return -EINVAL;
> + }
> +
> + target = kzalloc(sizeof(*target), GFP_KERNEL);
> +
> + if (!target) {
> + DMERR("Failed to allocate space for DM unstripe!");
> + return -ENOMEM;
> + }
> +
> + ret = dm_get_device(ti, argv[0], dm_table_get_mode(ti->table),
> + &target->ddisk);
> + if (ret) {
> + kfree(target);
> + DMERR("dm-unstripe dev lookup failure! for drive %s", argv[0]);
> + return ret;
> + }
> +
> + bbdev = target->ddisk->bdev;
> +
> + target->cur_drive = cur_drive;
> + if (chunk_size)
> + target->chunk_sectors = chunk_size;
> + else
> + target->chunk_sectors =
> + queue_max_hw_sectors(bdev_get_queue(bbdev));
> +
> + target->stripe_sectors = (tot_drives - 1) * target->chunk_sectors;
> + target->chunk_shift = fls(target->chunk_sectors) - 1;
> +
> + ret = dm_set_target_max_io_len(ti, target->chunk_sectors);
> + if (ret) {
> + dm_put_device(ti, target->ddisk);
> + kfree(target);
> + DMERR("Failed to set max io len!");
> + return ret;
> + }
> + ti->private = target;
> +
> + tot_sec = i_size_read(bbdev->bd_inode) >> SECTOR_SHIFT;
> + mod = tot_sec % target->chunk_sectors;
Did you build this on 32-bit also? Is that '%' OK on 32-bit?
> +
> + if (ti->len == 1)
> + ti->len = (tot_sec / tot_drives) - mod;
> + ti->begin = 0;
> + return 0;
> +}
--
~Randy
On 12/13/2017 01:33 PM, Scott Bauer wrote:
> Signed-off-by: Scott Bauer <[email protected]>
> ---
> 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
> +=====================
> +
[snip]
> +==============
> +
> +
> + 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|
> + ⎻⎻⎻⎻⎻⎻⎻⎻⎻⎻ ⎻⎻⎻⎻⎻⎻⎻⎻⎻⎻
Use ASCII characters ___ or ---, not whatever those bottom block characters are.
> +
> + 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.
5/9ths
although I can't quite parse that sentence.
--
~Randy
On Wed, Dec 13, 2017 at 04:18:06PM -0800, Randy Dunlap wrote:
> Use ASCII characters ___ or ---, not whatever those bottom block characters are.
Argh, sorry, I removed those for an internal review but forgot to remove them here.
>
> > +
> > + 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.
>
> 5/9ths
> although I can't quite parse that sentence.
I'll reword it, thanks.
[snip]
On Wed, Dec 13, 2017 at 04:11:44PM -0800, Randy Dunlap wrote:
>
> >=
>
Thanks, good catch.
> > + tot_sec = i_size_read(bbdev->bd_inode) >> SECTOR_SHIFT;
> > + mod = tot_sec % target->chunk_sectors;
>
> Did you build this on 32-bit also? Is that '%' OK on 32-bit?
I've looked at this a bit and still can't figure out why this
modulo operation would operate differently on a 32 versus a 64
bit platform? I know sector_t is config dependent but the
sector_t should be promoted to 64 bit width during the modulo
operation.
Are you wondering whether sector_t is the right type for any of
the math in this file? Perhaps we should be safe and only use
u64s?
On 12/15/2017 07:27 AM, Scott Bauer wrote:
> [snip]
> On Wed, Dec 13, 2017 at 04:11:44PM -0800, Randy Dunlap wrote:
>>
>> >=
>>
>
> Thanks, good catch.
>
>
>>> + tot_sec = i_size_read(bbdev->bd_inode) >> SECTOR_SHIFT;
>>> + mod = tot_sec % target->chunk_sectors;
>>
>> Did you build this on 32-bit also? Is that '%' OK on 32-bit?
>
> I've looked at this a bit and still can't figure out why this
> modulo operation would operate differently on a 32 versus a 64
> bit platform? I know sector_t is config dependent but the
> sector_t should be promoted to 64 bit width during the modulo
> operation.
>
> Are you wondering whether sector_t is the right type for any of
> the math in this file? Perhaps we should be safe and only use
> u64s?
Just wondering if it causes a call to some glibc __mod() function
and if so, the code should be using sector_div() -- oops, that's
a divide and you want a modulus. Oh well, we can address it if
it becomes a problem.
--
~Randy