| Message ID | 20230522104146.2856-1-nj.shetty@samsung.com |
|---|---|
| Headers | show |
| Series | Implement copy offload support | expand |
On Mon, May 22, 2023 at 08:45:44PM +0900, Damien Le Moal wrote: > On 5/22/23 19:41, Nitesh Shetty wrote: > > Add device limits as sysfs entries, > > - copy_offload (RW) > > - copy_max_bytes (RW) > > - copy_max_bytes_hw (RO) > > > > Above limits help to split the copy payload in block layer. > > copy_offload: used for setting copy offload(1) or emulation(0). > > copy_max_bytes: maximum total length of copy in single payload. > > copy_max_bytes_hw: Reflects the device supported maximum limit. > > > > Reviewed-by: Hannes Reinecke <hare@suse.de> > > Signed-off-by: Nitesh Shetty <nj.shetty@samsung.com> > > Signed-off-by: Kanchan Joshi <joshi.k@samsung.com> > > Signed-off-by: Anuj Gupta <anuj20.g@samsung.com> > > --- > > Documentation/ABI/stable/sysfs-block | 33 ++++++++++++++ > > block/blk-settings.c | 24 +++++++++++ > > block/blk-sysfs.c | 64 ++++++++++++++++++++++++++++ > > include/linux/blkdev.h | 12 ++++++ > > include/uapi/linux/fs.h | 3 ++ > > 5 files changed, 136 insertions(+) > > > > diff --git a/Documentation/ABI/stable/sysfs-block b/Documentation/ABI/stable/sysfs-block > > index c57e5b7cb532..e4d31132f77c 100644 > > --- a/Documentation/ABI/stable/sysfs-block > > +++ b/Documentation/ABI/stable/sysfs-block > > @@ -155,6 +155,39 @@ Description: > > last zone of the device which may be smaller. > > > > > > +What: /sys/block/<disk>/queue/copy_offload > > +Date: April 2023 > > +Contact: linux-block@vger.kernel.org > > +Description: > > + [RW] When read, this file shows whether offloading copy to a > > + device is enabled (1) or disabled (0). Writing '0' to this > > + file will disable offloading copies for this device. > > + Writing any '1' value will enable this feature. If the device > > + does not support offloading, then writing 1, will result in > > + error. > > will result is an error. > acked > > + > > + > > +What: /sys/block/<disk>/queue/copy_max_bytes > > +Date: April 2023 > > +Contact: linux-block@vger.kernel.org > > +Description: > > + [RW] This is the maximum number of bytes, that the block layer > > Please drop the comma after block. you mean after bytes ?, acked. > > > + will allow for copy request. This will be smaller or equal to > > will allow for a copy request. This value is always smaller... > acked > > + the maximum size allowed by the hardware, indicated by > > + 'copy_max_bytes_hw'. Attempt to set value higher than > > An attempt to set a value higher than... > acked > > + 'copy_max_bytes_hw' will truncate this to 'copy_max_bytes_hw'. > > + > > + > > +What: /sys/block/<disk>/queue/copy_max_bytes_hw > > +Date: April 2023 > > +Contact: linux-block@vger.kernel.org > > +Description: > > + [RO] This is the maximum number of bytes, that the hardware > > drop the comma after bytes > acked > > + will allow in a single data copy request. > > will allow for > acked > > + A value of 0 means that the device does not support > > + copy offload. > > Given that you do have copy emulation for devices that do not support hw > offload, how is the user supposed to know the maximum size of a copy request > when it is emulated ? This is not obvious from looking at these parameters. > This was little tricky for us as well. There are multiple limits (such as max_hw_sectors, max_segments, buffer allocation size), which decide what emulated copy size is. Moreover this limit was supposed to reflect device copy offload size/capability. Let me know if you something in mind, which can make this look better. > > + > > + > > What: /sys/block/<disk>/queue/crypto/ > > Date: February 2022 > > Contact: linux-block@vger.kernel.org > > diff --git a/block/blk-settings.c b/block/blk-settings.c > > index 896b4654ab00..23aff2d4dcba 100644 > > --- a/block/blk-settings.c > > +++ b/block/blk-settings.c > > @@ -59,6 +59,8 @@ void blk_set_default_limits(struct queue_limits *lim) > > lim->zoned = BLK_ZONED_NONE; > > lim->zone_write_granularity = 0; > > lim->dma_alignment = 511; > > + lim->max_copy_sectors_hw = 0; > > + lim->max_copy_sectors = 0; > > } > > > > /** > > @@ -82,6 +84,8 @@ void blk_set_stacking_limits(struct queue_limits *lim) > > lim->max_dev_sectors = UINT_MAX; > > lim->max_write_zeroes_sectors = UINT_MAX; > > lim->max_zone_append_sectors = UINT_MAX; > > + lim->max_copy_sectors_hw = ULONG_MAX; > > + lim->max_copy_sectors = ULONG_MAX; > > UINT_MAX is not enough ? acked > > > } > > EXPORT_SYMBOL(blk_set_stacking_limits); > > > > @@ -183,6 +187,22 @@ void blk_queue_max_discard_sectors(struct request_queue *q, > > } > > EXPORT_SYMBOL(blk_queue_max_discard_sectors); > > > > +/** > > + * blk_queue_max_copy_sectors_hw - set max sectors for a single copy payload > > + * @q: the request queue for the device > > + * @max_copy_sectors: maximum number of sectors to copy > > + **/ > > +void blk_queue_max_copy_sectors_hw(struct request_queue *q, > > + unsigned int max_copy_sectors) > > +{ > > + if (max_copy_sectors > (COPY_MAX_BYTES >> SECTOR_SHIFT)) > > + max_copy_sectors = COPY_MAX_BYTES >> SECTOR_SHIFT; > > + > > + q->limits.max_copy_sectors_hw = max_copy_sectors; > > + q->limits.max_copy_sectors = max_copy_sectors; > > +} > > +EXPORT_SYMBOL_GPL(blk_queue_max_copy_sectors_hw); > > + > > /** > > * blk_queue_max_secure_erase_sectors - set max sectors for a secure erase > > * @q: the request queue for the device > > @@ -578,6 +598,10 @@ int blk_stack_limits(struct queue_limits *t, struct queue_limits *b, > > t->max_segment_size = min_not_zero(t->max_segment_size, > > b->max_segment_size); > > > > + t->max_copy_sectors = min(t->max_copy_sectors, b->max_copy_sectors); > > + t->max_copy_sectors_hw = min(t->max_copy_sectors_hw, > > + b->max_copy_sectors_hw); > > + > > t->misaligned |= b->misaligned; > > > > alignment = queue_limit_alignment_offset(b, start); > > diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c > > index a64208583853..826ab29beba3 100644 > > --- a/block/blk-sysfs.c > > +++ b/block/blk-sysfs.c > > @@ -212,6 +212,63 @@ static ssize_t queue_discard_zeroes_data_show(struct request_queue *q, char *pag > > return queue_var_show(0, page); > > } > > > > +static ssize_t queue_copy_offload_show(struct request_queue *q, char *page) > > +{ > > + return queue_var_show(blk_queue_copy(q), page); > > +} > > + > > +static ssize_t queue_copy_offload_store(struct request_queue *q, > > + const char *page, size_t count) > > +{ > > + s64 copy_offload; > > + ssize_t ret = queue_var_store64(©_offload, page); > > + > > + if (ret < 0) > > + return ret; > > + > > + if (copy_offload && !q->limits.max_copy_sectors_hw) > > + return -EINVAL; > > + > > + if (copy_offload) > > + blk_queue_flag_set(QUEUE_FLAG_COPY, q); > > + else > > + blk_queue_flag_clear(QUEUE_FLAG_COPY, q); > > + > > + return count; > > +} > > + > > +static ssize_t queue_copy_max_hw_show(struct request_queue *q, char *page) > > +{ > > + return sprintf(page, "%llu\n", (unsigned long long) > > + q->limits.max_copy_sectors_hw << SECTOR_SHIFT); > > +} > > + > > +static ssize_t queue_copy_max_show(struct request_queue *q, char *page) > > +{ > > + return sprintf(page, "%llu\n", (unsigned long long) > > + q->limits.max_copy_sectors << SECTOR_SHIFT); > > +} > > + > > +static ssize_t queue_copy_max_store(struct request_queue *q, > > + const char *page, size_t count) > > +{ > > + s64 max_copy; > > + ssize_t ret = queue_var_store64(&max_copy, page); > > + > > + if (ret < 0) > > + return ret; > > + > > + if (max_copy & (queue_logical_block_size(q) - 1)) > > + return -EINVAL; > > + > > + max_copy >>= SECTOR_SHIFT; > > + if (max_copy > q->limits.max_copy_sectors_hw) > > + max_copy = q->limits.max_copy_sectors_hw; > > + > > + q->limits.max_copy_sectors = max_copy; > > q->limits.max_copy_sectors = > min(max_copy, q->limits.max_copy_sectors_hw); > > To remove the if above. acked > > > + return count; > > +} > > + > > static ssize_t queue_write_same_max_show(struct request_queue *q, char *page) > > { > > return queue_var_show(0, page); > > @@ -590,6 +647,10 @@ QUEUE_RO_ENTRY(queue_nr_zones, "nr_zones"); > > QUEUE_RO_ENTRY(queue_max_open_zones, "max_open_zones"); > > QUEUE_RO_ENTRY(queue_max_active_zones, "max_active_zones"); > > > > +QUEUE_RW_ENTRY(queue_copy_offload, "copy_offload"); > > +QUEUE_RO_ENTRY(queue_copy_max_hw, "copy_max_bytes_hw"); > > +QUEUE_RW_ENTRY(queue_copy_max, "copy_max_bytes"); > > + > > QUEUE_RW_ENTRY(queue_nomerges, "nomerges"); > > QUEUE_RW_ENTRY(queue_rq_affinity, "rq_affinity"); > > QUEUE_RW_ENTRY(queue_poll, "io_poll"); > > @@ -637,6 +698,9 @@ static struct attribute *queue_attrs[] = { > > &queue_discard_max_entry.attr, > > &queue_discard_max_hw_entry.attr, > > &queue_discard_zeroes_data_entry.attr, > > + &queue_copy_offload_entry.attr, > > + &queue_copy_max_hw_entry.attr, > > + &queue_copy_max_entry.attr, > > &queue_write_same_max_entry.attr, > > &queue_write_zeroes_max_entry.attr, > > &queue_zone_append_max_entry.attr, > > diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h > > index b441e633f4dd..c9bf11adccb3 100644 > > --- a/include/linux/blkdev.h > > +++ b/include/linux/blkdev.h > > @@ -295,6 +295,9 @@ struct queue_limits { > > unsigned int discard_alignment; > > unsigned int zone_write_granularity; > > > > + unsigned long max_copy_sectors_hw; > > + unsigned long max_copy_sectors; > > Why unsigned long ? unsigned int gives you 4G * 512 = 2TB max copy. Isn't that a > large enough max limit ? Should be enough. Will update this. > > > + > > unsigned short max_segments; > > unsigned short max_integrity_segments; > > unsigned short max_discard_segments; > > @@ -561,6 +564,7 @@ struct request_queue { > > #define QUEUE_FLAG_NOWAIT 29 /* device supports NOWAIT */ > > #define QUEUE_FLAG_SQ_SCHED 30 /* single queue style io dispatch */ > > #define QUEUE_FLAG_SKIP_TAGSET_QUIESCE 31 /* quiesce_tagset skip the queue*/ > > +#define QUEUE_FLAG_COPY 32 /* supports copy offload */ > > Nope. That is misleading. This flag is cleared in queue_copy_offload_store() if > the user writes 0. So this flag indicates that copy offload is enabled or > disabled, not that the device supports it. For the device support, one has to > look at max copy sectors hw being != 0. > Agree. Will changing it to "flag to enable/disable copy offload", will it be better? > > > > #define QUEUE_FLAG_MQ_DEFAULT ((1UL << QUEUE_FLAG_IO_STAT) | \ > > (1UL << QUEUE_FLAG_SAME_COMP) | \ > > @@ -581,6 +585,7 @@ bool blk_queue_flag_test_and_set(unsigned int flag, struct request_queue *q); > > test_bit(QUEUE_FLAG_STABLE_WRITES, &(q)->queue_flags) > > #define blk_queue_io_stat(q) test_bit(QUEUE_FLAG_IO_STAT, &(q)->queue_flags) > > #define blk_queue_add_random(q) test_bit(QUEUE_FLAG_ADD_RANDOM, &(q)->queue_flags) > > +#define blk_queue_copy(q) test_bit(QUEUE_FLAG_COPY, &(q)->queue_flags) > > #define blk_queue_zone_resetall(q) \ > > test_bit(QUEUE_FLAG_ZONE_RESETALL, &(q)->queue_flags) > > #define blk_queue_dax(q) test_bit(QUEUE_FLAG_DAX, &(q)->queue_flags) > > @@ -899,6 +904,8 @@ extern void blk_queue_chunk_sectors(struct request_queue *, unsigned int); > > extern void blk_queue_max_segments(struct request_queue *, unsigned short); > > extern void blk_queue_max_discard_segments(struct request_queue *, > > unsigned short); > > +extern void blk_queue_max_copy_sectors_hw(struct request_queue *q, > > + unsigned int max_copy_sectors); > > void blk_queue_max_secure_erase_sectors(struct request_queue *q, > > unsigned int max_sectors); > > extern void blk_queue_max_segment_size(struct request_queue *, unsigned int); > > @@ -1218,6 +1225,11 @@ static inline unsigned int bdev_discard_granularity(struct block_device *bdev) > > return bdev_get_queue(bdev)->limits.discard_granularity; > > } > > > > +static inline unsigned int bdev_max_copy_sectors(struct block_device *bdev) > > +{ > > + return bdev_get_queue(bdev)->limits.max_copy_sectors; > > +} > > + > > static inline unsigned int > > bdev_max_secure_erase_sectors(struct block_device *bdev) > > { > > diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h > > index b7b56871029c..8879567791fa 100644 > > --- a/include/uapi/linux/fs.h > > +++ b/include/uapi/linux/fs.h > > @@ -64,6 +64,9 @@ struct fstrim_range { > > __u64 minlen; > > }; > > > > +/* maximum total copy length */ > > +#define COPY_MAX_BYTES (1 << 27) > > My brain bit shifter is not as good as a CPU one :) So it would be nice to > mention what value that is in MB and also explain where this magic value comes from. > Agree. Even I had similar experience :). Will update the comments to reflect in MB. About the limit, we faced a dilemma, 1. not set limit: Allow copy which is huge(ssize_t) and which starts consuming/hogging system resources 2. set limit: what the limit should be We went with 2, and limit is based on our internel testing, mainly desktop systems had memory limitation for emulation scenario. Feel free to suggest, if you have some other thoughts. Thanks, Nitesh Shetty
The patch series covers the points discussed in past and most recently in LSFMM'23[0]. We have covered the initial agreed requirements in this patchset and further additional features suggested by community. Patchset borrows Mikulas's token based approach for 2 bdev implementation. This is next iteration of our previous patchset v10[1]. Overall series supports: ======================== 1. Driver - NVMe Copy command (single NS, TP 4065), including support in nvme-target (for block and file backend). 2. Block layer - Block-generic copy (REQ_COPY flag), with interface accommodating two block-devs - Emulation, for in-kernel user when offload is natively absent - dm-linear support (for cases not requiring split) 3. User-interface - copy_file_range Testing ======= Copy offload can be tested on: a. QEMU: NVME simple copy (TP 4065). By setting nvme-ns parameters mssrl,mcl, msrc. For more info [2]. b. Null block device c. NVMe Fabrics loopback. d. blktests[3] (tests block/034-037, nvme/050-053) Emulation can be tested on any device. fio[4]. Infra and plumbing: =================== We populate copy_file_range callback in def_blk_fops. For devices that support copy-offload, use blkdev_copy_offload to achieve in-device copy. However for cases, where device doesn't support offload, fallback to generic_copy_file_range. For in-kernel users (like NVMe fabrics), we use blkdev_issue_copy which implements its own emulation, as fd is not available. Modify checks in generic_copy_file_range to support block-device. Performance: ============ The major benefit of this copy-offload/emulation framework is observed in fabrics setup, for copy workloads across the network. The host will send offload command over the network and actual copy can be achieved using emulation on the target. This results in higher performance and lower network consumption, as compared to read and write travelling across the network. With async-design of copy-offload/emulation we are able to see the following improvements as compared to userspace read + write on a NVMeOF TCP setup: Setup1: Network Speed: 1000Mb/s Host PC: Intel(R) Core(TM) i7-8700 CPU @ 3.20GHz Target PC: AMD Ryzen 9 5900X 12-Core Processor block size 8k: 710% improvement in IO BW (108 MiB/s to 876 MiB/s). Network utilisation drops from 97% to 15%. block-size 1M: 2532% improvement in IO BW (101 MiB/s to 2659 MiB/s). Network utilisation drops from 89% to 0.62%. Setup2: Network Speed: 100Gb/s Server: Intel(R) Xeon(R) Gold 6240 CPU @ 2.60GHz, 72 cores (host and target have the same configuration) block-size 8k: 17.5% improvement in IO BW (794 MiB/s to 933 MiB/s). Network utilisation drops from 6.75% to 0.16%. Blktests[3] ====================== tests/block/034,035: Runs copy offload and emulation on block device. tests/block/035,036: Runs copy offload and emulation on null block device. tests/nvme/050-053: Create a loop backed fabrics device and run copy offload and emulation. Future Work =========== - loopback device copy offload support - upstream fio to use copy offload - upstream blktest to use copy offload These are to be taken up after this minimal series is agreed upon. Additional links: ================= [0] https://lore.kernel.org/linux-nvme/CA+1E3rJ7BZ7LjQXXTdX+-0Edz=zT14mmPGMiVCzUgB33C60tbQ@mail.gmail.com/ https://lore.kernel.org/linux-nvme/f0e19ae4-b37a-e9a3-2be7-a5afb334a5c3@nvidia.com/ https://lore.kernel.org/linux-nvme/20230113094648.15614-1-nj.shetty@samsung.com/ [1] https://lore.kernel.org/all/20230419114320.13674-1-nj.shetty@samsung.com/ [2] https://qemu-project.gitlab.io/qemu/system/devices/nvme.html#simple-copy [3] https://github.com/nitesh-shetty/blktests/tree/feat/copy_offload/v11 [4] https://github.com/vincentkfu/fio/commits/copyoffload-3.34-v11 Changes since v10: ================= - NVMeOF: optimization in NVMe fabrics (Chaitanya Kulkarni) - NVMeOF: sparse warnings (kernel test robot) Changes since v9: ================= - null_blk, improved documentation, minor fixes(Chaitanya Kulkarni) - fio, expanded testing and minor fixes (Vincent Fu) Changes since v8: ================= - null_blk, copy_max_bytes_hw is made config fs parameter (Damien Le Moal) - Negative error handling in copy_file_range (Christian Brauner) - minor fixes, better documentation (Damien Le Moal) - fio upgraded to 3.34 (Vincent Fu) Changes since v7: ================= - null block copy offload support for testing (Damien Le Moal) - adding direct flag check for copy offload to block device, as we are using generic_copy_file_range for cached cases. - Minor fixes Changes since v6: ================= - copy_file_range instead of ioctl for direct block device - Remove support for multi range (vectored) copy - Remove ioctl interface for copy. - Remove offload support in dm kcopyd. Changes since v5: ================= - Addition of blktests (Chaitanya Kulkarni) - Minor fix for fabrics file backed path - Remove buggy zonefs copy file range implementation. Changes since v4: ================= - make the offload and emulation design asynchronous (Hannes Reinecke) - fabrics loopback support - sysfs naming improvements (Damien Le Moal) - use kfree() instead of kvfree() in cio_await_completion (Damien Le Moal) - use ranges instead of rlist to represent range_entry (Damien Le Moal) - change argument ordering in blk_copy_offload suggested (Damien Le Moal) - removed multiple copy limit and merged into only one limit (Damien Le Moal) - wrap overly long lines (Damien Le Moal) - other naming improvements and cleanups (Damien Le Moal) - correctly format the code example in description (Damien Le Moal) - mark blk_copy_offload as static (kernel test robot) Changes since v3: ================= - added copy_file_range support for zonefs - added documentation about new sysfs entries - incorporated review comments on v3 - minor fixes Changes since v2: ================= - fixed possible race condition reported by Damien Le Moal - new sysfs controls as suggested by Damien Le Moal - fixed possible memory leak reported by Dan Carpenter, lkp - minor fixes Changes since v1: ================= - sysfs documentation (Greg KH) - 2 bios for copy operation (Bart Van Assche, Mikulas Patocka, Martin K. Petersen, Douglas Gilbert) - better payload design (Darrick J. Wong) Nitesh Shetty (9): block: Introduce queue limits for copy-offload support block: Add copy offload support infrastructure block: add emulation for copy fs, block: copy_file_range for def_blk_ops for direct block device nvme: add copy offload support nvmet: add copy command support for bdev and file ns dm: Add support for copy offload dm: Enable copy offload for dm-linear target null_blk: add support for copy offload Documentation/ABI/stable/sysfs-block | 33 ++ block/blk-lib.c | 431 +++++++++++++++++++++++++++ block/blk-map.c | 4 +- block/blk-settings.c | 24 ++ block/blk-sysfs.c | 64 ++++ block/blk.h | 2 + block/fops.c | 20 ++ drivers/block/null_blk/main.c | 108 ++++++- drivers/block/null_blk/null_blk.h | 8 + drivers/md/dm-linear.c | 1 + drivers/md/dm-table.c | 41 +++ drivers/md/dm.c | 7 + drivers/nvme/host/constants.c | 1 + drivers/nvme/host/core.c | 103 ++++++- drivers/nvme/host/fc.c | 5 + drivers/nvme/host/nvme.h | 7 + drivers/nvme/host/pci.c | 27 +- drivers/nvme/host/rdma.c | 7 + drivers/nvme/host/tcp.c | 16 + drivers/nvme/host/trace.c | 19 ++ drivers/nvme/target/admin-cmd.c | 9 +- drivers/nvme/target/io-cmd-bdev.c | 62 ++++ drivers/nvme/target/io-cmd-file.c | 52 ++++ drivers/nvme/target/loop.c | 6 + drivers/nvme/target/nvmet.h | 1 + fs/read_write.c | 11 +- include/linux/blk_types.h | 25 ++ include/linux/blkdev.h | 21 ++ include/linux/device-mapper.h | 5 + include/linux/nvme.h | 43 ++- include/uapi/linux/fs.h | 3 + mm/filemap.c | 11 +- 32 files changed, 1155 insertions(+), 22 deletions(-) base-commit: dbd91ef4e91c1ce3a24429f5fb3876b7a0306733