Re: [PATCH v20 02/12] Add infrastructure for copy offload in block and request layer.

[Nitesh Shetty]

On 24/05/24 01:33PM, Bart Van Assche wrote:
> On 5/20/24 03:20, Nitesh Shetty wrote:
> > We add two new opcode REQ_OP_COPY_DST, REQ_OP_COPY_SRC.
> > Since copy is a composite operation involving src and dst sectors/lba,
> > each needs to be represented by a separate bio to make it compatible
> > with device mapper.
> > We expect caller to take a plug and send bio with destination information,
> > followed by bio with source information.
> > Once the dst bio arrives we form a request and wait for source
> > bio. Upon arrival of source bio we merge these two bio's and send
> > corresponding request down to device driver.
> > Merging non copy offload bio is avoided by checking for copy specific
> > opcodes in merge function.
>
> In this patch I don't see any changes for blk_attempt_bio_merge(). Does
> this mean that combining REQ_OP_COPY_DST and REQ_OP_COPY_SRC will never
> happen if the QUEUE_FLAG_NOMERGES request queue flag has been set?
Yes, in this case copy won't work, as both src and dst bio reach driver
as part of separate requests.
We will add this as part of documentation.

> Can it happen that the REQ_NOMERGE flag is set by __bio_split_to_limits()
> for REQ_OP_COPY_DST or REQ_OP_COPY_SRC bios? Will this happen if the
> following condition is met?
>
> dst_bio->nr_phys_segs + src_bio->nr_phys_segs > max_segments
No, this should not happen. We don't use bio_split_rw for copy.
We have added a separate function to check for split incase of
copy(bio_split_copy), which doesn't allow copy bio splits,
hence REQ_NOMERGE flag won't be set.

> Is it allowed to set REQ_PREFLUSH or REQ_FUA for REQ_OP_COPY_DST or
> REQ_OP_COPY_SRC bios? I'm asking this because these flags disable merging.
>
> From include/linux/blk_types.h:
>
> #define REQ_NOMERGE_FLAGS (REQ_NOMERGE | REQ_PREFLUSH | REQ_FUA)
No, setting these flags won't allow copy bio's to merge and hence copy
won't work.
We suggest to use helper API blkdev_copy_offload to achieve the
copy which won't be setting these flags.

Thank You,
Nitesh Shetty