Re: [PATCH 3/5] nvme: add support for copy offload

Keith Busch
2025-05-22T03:24:05Z

On Wed, May 21, 2025 at 05:51:03PM -0700, Caleb Sander Mateos wrote:
> On Wed, May 21, 2025 at 5:47 PM Caleb Sander Mateos
> >
> > alloc_size should be sizeof(*range) * i? Otherwise this exceeds the
> > amount of data used by the Copy command, which not all controllers
> > support (see bit LLDTS of SGLS in the Identify Controller data
> > structure). We have seen the same behavior with Dataset Management
> > (always specifying 4 KB of data), which also passes the maximum size
> > of the allocation to bvec_set_virt().
>
> I see that was added in commit 530436c45ef2e ("nvme: Discard
> workaround for non-conformant devices"). I would rather wait for
> evidence of non-conformant devices supporting Copy before implementing
> the same spec-noncompliant workaround. It could be a quirk if
> necessary.

Right, that's exactly why I didn't bother allocating tighter to what the
command actually needs. The number of devices that would have needed a
DSM quirk for the full 4k was untenable, so making the quirk behavior
the default was a sane compromise. I suppose Copy is a more enterprisey
feature in comparison, so maybe we can count on devices doing dma
correctly?