Re: [PATCH 2/5] block: add support for copy offload

Hannes Reinecke
2025-05-22T13:49:24Z

On 5/22/25 00:31, Keith Busch wrote:
> From: Keith Busch <kbusch@kernel.org>
>
> Various storage protocols can support offloading block data copies.
> Enhance the block layer to know about the device's copying capabilities,
> introduce the new REQ_OP_COPY operation, and provide the infrastructure
> to iterate, split, and merge these kinds of requests.
>
> A copy command must provide the device with a list of source LBAs and
> their lengths, and a destination LBA. The 'struct bio' type doesn't
> readily have a way to describe such a thing. But a copy request doesn't
> use host memory for data, so the bio's bio_vec is unused space. This
> patch adds a new purpose to the bio_vec where it can provide a vector of
> sectors instead of memory pages.
>
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
> block/bio.c | 25 ++++++++++++++
> block/blk-core.c | 4 +++
> block/blk-lib.c | 47 ++++++++++++++++++++++-----
> block/blk-merge.c | 28 +++++++++++++++-
> block/blk-sysfs.c | 9 ++++++
> block/blk.h | 17 +++++++++-
> include/linux/bio.h | 20 ++++++++++++
> include/linux/blk-mq.h | 5 +++
> include/linux/blk_types.h | 2 ++
> include/linux/blkdev.h | 14 ++++++++
> include/linux/bvec.h | 68 +++++++++++++++++++++++++++++++++++++--
> 11 files changed, 226 insertions(+), 13 deletions(-)
>
> diff --git a/block/bio.c b/block/bio.c
> index 3c0a558c90f52..9c73a895c987b 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -1156,6 +1156,31 @@ void bio_iov_bvec_set(struct bio *bio, const struct iov_iter *iter)
> bio_set_flag(bio, BIO_CLONED);
> }
>
> +static int bvec_try_merge_copy_src(struct bio *bio, struct bio_vec *src)
> +{
> + struct bio_vec *bv;
> +
> + if (!bio->bi_vcnt)
> + return false;
> +
> + bv = &bio->bi_io_vec[bio->bi_vcnt - 1];
> + if (bv->bv_sector + src->bv_sectors != src->bv_sector)
> + return false;
> +
> + bv->bv_sectors += src->bv_sectors;
> + return true;
> +}
> +
> +int bio_add_copy_src(struct bio *bio, struct bio_vec *src)
> +{
> + if (bvec_try_merge_copy_src(bio, src))
> + return 0;
> + if (bio->bi_vcnt >= bio->bi_max_vecs)
> + return -EINVAL;
> + bio->bi_io_vec[bio->bi_vcnt++] = *src;
> + return 0;
> +}
> +
> static unsigned int get_contig_folio_len(unsigned int *num_pages,
> struct page **pages, unsigned int i,
> struct folio *folio, size_t left,
> diff --git a/block/blk-core.c b/block/blk-core.c
> index b862c66018f25..cb3d9879e2d65 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -837,6 +837,10 @@ void submit_bio_noacct(struct bio *bio)
> if (!bdev_max_discard_sectors(bdev))
> goto not_supported;
> break;
> + case REQ_OP_COPY:
> + if (!bdev_copy_sectors(bdev))
> + goto not_supported;
> + break;
> case REQ_OP_SECURE_ERASE:
> if (!bdev_max_secure_erase_sectors(bdev))
> goto not_supported;
> diff --git a/block/blk-lib.c b/block/blk-lib.c
> index a819ded0ed3a9..a538acbaa2cd7 100644
> --- a/block/blk-lib.c
> +++ b/block/blk-lib.c
> @@ -369,14 +369,7 @@ int blkdev_issue_secure_erase(struct block_device *bdev, sector_t sector,
> }
> EXPORT_SYMBOL(blkdev_issue_secure_erase);
>
> -/**
> - * blkdev_copy - copy source sectors to a destination on the same block device
> - * @dst_sector: start sector of the destination to copy to
> - * @src_sector: start sector of the source to copy from
> - * @nr_sects: number of sectors to copy
> - * @gfp: allocation flags to use
> - */
> -int blkdev_copy(struct block_device *bdev, sector_t dst_sector,
> +static int __blkdev_copy(struct block_device *bdev, sector_t dst_sector,
> sector_t src_sector, sector_t nr_sects, gfp_t gfp)
> {
> unsigned int nr_vecs = __blkdev_sectors_to_bio_pages(nr_sects);

That's a bit odd, renaming a just introduced function.
But if you must...

Otherwise looks good.

Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich