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

[Bart Van Assche]

On 5/21/24 00:01, Hannes Reinecke wrote:
> On 5/20/24 12: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.
> I am a bit unsure about leveraging 'merge' here. As Bart pointed out, 
> this is arguably as mis-use of the 'merge' functionality as we don't
> actually merge bios, but rather use the information from these bios to
> form the actual request.
> Wouldn't it be better to use bio_chain here, and send out the eventual
> request from the end_io function of the bio chain?

Let me formulate this a bit stronger: I think this patch series abuses
the merge functionality and also that it should use another mechanism
for combining REQ_OP_COPY_DST and REQ_OP_COPY_SRC. See also my email
with concerns about using the merge functionality:
https://lore.kernel.org/linux-block/eda6c198-3a29-4da4-94db-305cfe28d3d6@xxxxxxx/.

Thanks,

Bart.