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

[Hannes Reinecke]

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@xxxxxxx                              +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