illumos gate

I attempted a little testing with TRIM enabled on one of my systems (with a couple Crucial MX500's connected to motherboard SATA ports) and discovered some questionable code in uts/common/io/sata/impl/sata.c's implementation of READ CAPACITY 16.

If I'm reading the spec fragments I've found correctly, SATA devices advertise "trim" capability via the low order bit of "word 169" (which sata.c refers to as "satadrv_id.ad_dsm"), and make commitments about the behavior of reads after trim with two bits in "word 69" (satadrv_id.ad_addsupported) -- either "always zero", "always the same thing", or "subject to change at any time").

As best as I can tell, none of this additional detail matters to ZFS -- everything it writes is checksummed, and it's never going to attempt to read a block that's been trimmed until it's been rewritten with new contents.

Meanwhile, the SCSI command set communicates a subset of this information in the high order two bits of one of the bytes of the READ CAPACITY 16 response - the high order bit for TPE ("thin provisioning enabled"?) and the next one for "TPRZ" ("thin provisioning read zero"?).

Above this code, sd.c looks only for the TPE bit and ignores the TPRZ bit in implementing the DKIOC_CANFREE and DKIOCFREE ioctls.

However when we look at the code in common/io/sata/impl/sata.c's sata_txlt_read_capacity16(), which fills in the READ CAPACITY 16 response we find:

if (sdinfo->satadrv_id.ai_addsupported &
                    SATA_DETERMINISTIC_READ) {
                        if (sdinfo->satadrv_id.ai_addsupported &
                            SATA_READ_ZERO) {
                                rbuf[14] |= TPRZ;
                        } else {
                                rbuf[14] |= TPE;
                        }
                }

This only sets TPE (which is what sd.c is looking for) if SATA_DETERMINISTIC_READ is set and SATA_READ_ZERO is clear!

I'd think that to meet ZFS's minimal expectations for TRIM, it should set TPE if the low order bit of ai_dsm is set, and then add in TPRZ only if all three bits (ai_dsm&1, SATA_DETERMINISTIC_READ, and SATA_READ_ZERO) are set, and when I tweak the code accordingly, zpool trim appears to be functional for the SATA-connected ssd's mentioned above.