Re: [PATCH v15 04/12] block: add emulation for copy

[Nitesh Shetty <nj.shetty@samsung.com>]

[-- Attachment #1: Type: text/plain, Size: 2896 bytes --]

On 11/09/23 09:39AM, Hannes Reinecke wrote:
>On 9/11/23 09:09, Nitesh Shetty wrote:
>>On Fri, Sep 08, 2023 at 08:06:38AM +0200, Hannes Reinecke wrote:
>>>On 9/6/23 18:38, Nitesh Shetty wrote:
>>>>For the devices which does not support copy, copy emulation is added.
>>>>It is required for in-kernel users like fabrics, where file descriptor is
>>>>not available and hence they can't use copy_file_range.
>>>>Copy-emulation is implemented by reading from source into memory and
>>>>writing to the corresponding destination.
>>>>Also emulation can be used, if copy offload fails or partially completes.
>>>>At present in kernel user of emulation is NVMe fabrics.
>>>>
>>>Leave out the last sentence; I really would like to see it enabled for SCSI,
>>>too (we do have copy offload commands for SCSI ...).
>>>
>>Sure, will do that
>>
>>>And it raises all the questions which have bogged us down right from the
>>>start: where is the point in calling copy offload if copy offload is not
>>>implemented or slower than copying it by hand?
>>>And how can the caller differentiate whether copy offload bring a benefit to
>>>him?
>>>
>>>IOW: wouldn't it be better to return -EOPNOTSUPP if copy offload is not
>>>available?
>>
>>Present approach treats copy as a background operation and the idea is to
>>maximize the chances of achieving copy by falling back to emulation.
>>Having said that, it should be possible to return -EOPNOTSUPP,
>>in case of offload IO failure or device not supporting offload.
>>We will update this in next version.
>>
>That is also what I meant with my comments to patch 09/12: I don't see 
>it as a benefit to _always_ fall back to a generic copy-offload 
>emulation. After all, that hardly brings any benefit.

Agreed, we will correct this by returning error to user in case copy offload
fails, instead of falling back to block layer emulation.

We do need block layer emulation for fabrics, where we call emulation
if target doesn't support offload. In fabrics scenarios sending
offload command from host and achieve copy using block layer
emulation on target is better than sending read+write from host.

>Where I do see a benefit is to tie in the generic copy-offload 
>_infrastructure_ to existing mechanisms (like dm-kcopyd).
>But if there is no copy-offload infrastructure available then we 
>really should return -EOPNOTSUPP as it really is not supported.
>
Agreed, we will add this in next phase, once present series gets merged.

>In the end, copy offload is not a command which 'always works'.
>It's a command which _might_ deliver benefits (ie better performance) 
>if dedicated implementations are available and certain parameters are 
>met. If not then copy offload is not the best choice, and applications 
>will need to be made aware of that.

Agreed. We will leave the choice to user, to use either block layer offload
or emulation.


Thank you,
Nitesh Shetty

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]