Re: [PATCH 1/2] ata: libata-scsi: fix DSM TRIM for sector sizes larger than 2048 bytes

[Hannes Reinecke <hare@suse.de>]

On 7/2/26 12:46 AM, Niklas Cassel wrote:
> ata_scsi_write_same_xlat() translates a SCSI WRITE SAME command with the
> UNMAP bit set into an ATA DATA SET MANAGEMENT TRIM command.  The TRIM
> descriptor is built by ata_format_dsm_trim_descr() into the 2048-byte
> ata_scsi_rbuf staging buffer, and the number of bytes copied is compared
> against the logical sector size by the caller:
> 
> 	size = ata_format_dsm_trim_descr(scmd, trmax, block, n_block);
> 	if (size != len)		/* len == sdp->sector_size */
> 		goto invalid_param_len;
> 
> ata_format_dsm_trim_descr() clamps the copy length to ATA_SCSI_RBUF_SIZE
> (2048).  On a device whose logical sector size exceeds that (e.g. a 4Kn
> device, where sector_size == 4096) the function can never return more than
> 2048, while the caller expects it to return sector_size.  The comparison
> therefore always fails, so every TRIM is rejected with "Parameter list
> length error" and WARN_ON() splats on each attempt.  TRIM / discard is
> thus completely broken on such devices.
> 
> The descriptor was incorrectly sized from the logical sector size.  A DSM
> TRIM payload is a list of 512-byte pages, each holding up to
> ATA_MAX_TRIM_RNUM (64) LBA Range Entries, and is independent of the logical
> sector size.  The Block Limits VPD page already advertises a single such
> page as the maximum WRITE SAME length (65535 * ATA_MAX_TRIM_RNUM logical
> blocks), so the block layer never sends a request that needs more than one
> page.
> 
> Emit exactly one 512-byte page, independent of the logical sector size,
> and transfer only that page (COUNT == 1).  For a 512-byte-sector device
> this is unchanged; devices with larger logical sectors now work instead of
> failing every TRIM.
> 
> Fixes: ef2d7392c4ec ("libata: SCT Write Same / DSM Trim")
> Cc: stable@vger.kernel.org
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
>   drivers/ata/libata-scsi.c | 28 ++++++++++++++--------------
>   1 file changed, 14 insertions(+), 14 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@kernel.org>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich