On Thu, Aug 25, 2016 at 1:31 AM, Tom Yan [off-list ref] wrote:

On 25 August 2016 at 05:28, Shaun Tancheff [off-list ref] wrote:

quoted

On Wed, Aug 24, 2016 at 12:31 AM, Tom Yan [off-list ref] wrote:

quoted

On 24 August 2016 at 11:33, Martin K. Petersen
[off-list ref] wrote:

quoted

quoted

quoted

quoted

quoted

quoted

"Tom" == Tom Yan [off-list ref] writes:

Tom> Nope, SCSI Write Same commands does not have payload (or in SCSI
Tom> terms, parameter list / data-out buffer).

WRITE SAME has a a payload of 1 logical block (unless NDOB is set but we
have had no good reason to support that yet).

Interesting, I wasn't aware of the bit. I just didn't see any
parameter list defined for any of the Write Same commands. Ah wait, it
carries the pattern (the "same") and so.

Hmm, it doesn't seem like the translation implemented in this patch
series cares about the payload though?

As repeated here and elsewhere the payload is:
    scsi_sglist(cmd)
and it was put there by scsi_init_io() when it called scsi_init_sgtable()

What I mean is:

put_unaligned_le32(0u,      &sctpg[10]);

So even if the payload of the SCSI Write Same commands instruct a
non-zero pattern writing, SCT Write Same will conveniently ignore that
do zero-filling anyway. That's what I mean by "doesn't care about the
payload".

If you would like to add support for that it would be nice. I am not
planning to do so here.

Though that would only be case with SG_IO though. SCSI Write Same
issued from block layer (BLKZEROOUT) will be instructing zero-filling
anyway.

quoted

quoted

quoted

UNMAP has a payload that varies based on the number of range
descriptors. The SCSI disk driver only ever issues a single descriptor
but since libata doesn't support UNMAP this doesn't really come into
play.

Ideally there would be a way to distinguish between device limits for
WRITE SAME with the UNMAP bit and for "regular" WRITE SAME. One way to
do that would be to transition the libata discard implementation over to
single-range UNMAP, fill out the relevant VPD page B0 fields and leave
the WRITE SAME bits for writing zeroes.

One reason that has not been particularly compelling is that the WRITE
SAME payload buffer does double duty to hold the ATA DSM TRIM range

Huh? Why would the SATL care about its payload buffer for TRIM (i.e.
when the UNMAP bit is set)? Doesn't it just read the LBA and NUMBER OF
BLOCKS field and pack TRIM ranges/payload according to that?

quoted

descriptors and matches the required ATA payload size. Whereas the UNMAP

Why would it need to "matches the required ATA payload size"?

quoted

command would only provide 24 bytes of TRIM range space.

I don't really follow. The UNMAP descriptor has LBA (8 bytes / 64-bit)
and NUMBER OF BLOCKS (4 bytes / 32-bit) field of the same length as
Write Same (16). Even if the SCSI disk driver will only send one
descriptor, it should work as good as Write Same (16).

The "payload" is the data block transferred with the command.
The "descriptor" is, in this context, the contents of the payload as
it "describes" what the action the command is supposed to perform.

I know right.

quoted

The "payload" contains the "descriptor" that "describes" how
DSM TRIM should determine which logical blocks it should UNMAP.

This should only be the case of UNMAP command, but not SCSI Write Same
with UNMAP bit set. And either way it should not affect how large the
ATA TRIM payload can be.

The current SATL does not report support for UNMAP.
If you think it should be added please submit a patch.

If you would like to extend the current translate to support sending
multiple blocks of trim data please submit a patch.

quoted

quoted

quoted

Also, please be careful with transfer lengths, __data_len, etc. As
mentioned, the transfer length WRITE SAME is typically 512 bytes and
that's the number of bytes that need to be DMA'ed and transferred over
the wire by the controller. But from a command completion perspective we
need to complete however many bytes the command acted upon. Unlike reads
and writes there is not a 1:1 mapping between the transfer length and
the affected area. So we do a bit of magic after the buffer has been
mapped to ensure that the completion byte count matches the number of
blocks that were affected by the command rather than the size of the
data buffer in bytes.

--
Martin K. Petersen      Oracle Linux Engineering

--
Shaun Tancheff

-- 
Shaun Tancheff