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

From: Bart Van Assche
Date: Tue May 28 2024 - 10:09:01 EST


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.