Re: [PATCH v15 09/12] dm: Add support for copy offload

[Hannes Reinecke <hare@suse.de>]

On 9/11/23 09:07, Nitesh Shetty wrote:
> On Fri, Sep 08, 2023 at 08:13:37AM +0200, Hannes Reinecke wrote:
>> On 9/6/23 18:38, Nitesh Shetty wrote:
>>> Before enabling copy for dm target, check if underlying devices and
>>> dm target support copy. Avoid split happening inside dm target.
>>> Fail early if the request needs split, currently splitting copy
>>> request is not supported.
>>>
>> And here is where I would have expected the emulation to take place;
>> didn't you have it in one of the earlier iterations?
> 
> No, but it was the other way round.
> In dm-kcopyd we used device offload, if that was possible, before using default
> dm-mapper copy. It was dropped in the current series,
> to streamline the patches and make the series easier to review.
> 
>> After all, device-mapper already has the infrastructure for copying
>> data between devices, so adding a copy-offload emulation for device-mapper
>> should be trivial.
> I did not understand this, can you please elaborate ?
> 
Please see my comments to patch 04.
We should only implement copy-offload if there is a dedicated 
infrastructure in place. But we should not have a 'generic' copy-offload 
emulation.
Problem is that 'real' copy-offload functionalities (ie for NVMe or 
SCSI) are riddled with corner-cases where copy-offload does _not_ work,
and where commands might fail if particular conditions are not met.
Falling back to a generic implementation will cause applications to 
assume that copy-offload worked, and that it gained performance as
the application just had to issue a single command.
Whereas in fact the opposite is true; it wasn't a single command, and 
the application might have performed better by issuing the commands
itself.
Returning -EOPNOTSUPP in these cases will inform the application that 
the attempt didn't work, and that it will have to fall back to the
'normal' copy.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman