Re: scsi: convert discard to REQ_TYPE_FS instead of REQ_TYPE_BLOCK_PC

[Douglas Gilbert <dgilbert@interlog.com>]

On 10-07-06 05:31 PM, Mike Snitzer wrote:
> On Tue, Jul 06 2010 at  3:01am -0400,
> FUJITA Tomonori<fujita.tomonori@lab.ntt.co.jp>  wrote:
>
>> I confirmed that mkfs.xfs worked with Intel X25-M (trim) and
>> scsi_debug (write same and unmap).
>>
>> REQ_TYPE_FS should give the same scsi_cmnd struct as REQ_TYPE_BLOCK_PC.
>>
>> This can be applied to block's for-2.6.36.
>>
>> The git tree is also available:
>>
>> git://git.kernel.org/pub/scm/linux/kernel/git/tomo/linux-2.6-misc.git fs-discard
>>
>> =
>> From: FUJITA Tomonori<fujita.tomonori@lab.ntt.co.jp>
>> Subject: [PATCH] scsi: convert discard to REQ_TYPE_FS instead of REQ_TYPE_BLOCK_PC
>>
>> The block layer (file systems) sends discard requests as REQ_TYPE_FS
>> (the role of REQ_TYPE_FS is that setting up commands and interpreting
>> the results). But SCSI-ml treats discard requests as
>> REQ_TYPE_BLOCK_PC.
>>
>> scsi-ml can handle discard requests as REQ_TYPE_FS
>> easily. scsi_setup_discard_cmnd() sets up struct request and the bio
>> nicely. Only remaining issue is that discard requests can't be
>> completed partially so we need to modify sd_done.
>>
>> This conversion also fixes the problem that discard requests aren't
>> retried when possible (e.g. UNIT ATTENTION).
>>
>> Signed-off-by: FUJITA Tomonori<fujita.tomonori@lab.ntt.co.jp>
>
> Unfortunately this patch causes 'mkfs.ext4 -F /dev/sda' to fail against
> a device whose discard support is implemented using WRITE SAME 16 w/
> discard bit set.  This is with recent e2fsprogs that issues BLKDISCARD
> ioctl at start of mkfs:
>
> sd 2:0:0:0: [sda] Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE
> sd 2:0:0:0: [sda] Sense Key : Illegal Request [current]
> Info fld=0x0
> sd 2:0:0:0: [sda] Add. Sense: Parameter value invalid
> sd 2:0:0:0: [sda] CDB: Write same(16): 93 08 00 00 00 00 00 00 00 00 00 7f ff ff 00 00
> end_request: I/O error, dev sda, sector 0

That is 0x7fffff (over 8 million) blocks (4 GB) being unmapped
in one operation! That may exceed the "maximum unmap lba
count" field in the Block Limits VPD page.
The latest SBC draft (sbc3r22.pdf) says that field applies to
the SCSI UNMAP command and does not mention the WRITE SAME (16)
command but that is probably an oversight.

One also wonders how long that would take if permitted and what
is an appropriate command timeout.

Also the additional sense of "Parameter value invalid" (26h,2h)
is not mentioned in the latest drafts of SPC or SBC, apart from
being defined. So it might be a good one for an implementer to
pull out of the bag for special occasions. And the SBC draft
does say what is the correct additional sense in this case.


On a slightly related matter, when the target device is ATA
(e.g. a SSD with an SATA interface) then the SAT used can
influence whether WRITE SAME (16) gets translated into DATA
SET MANAGEMENT with the TRIM bit set. libata does it in recent
kernels but the LSI SAS controller that I use has its own SAT
which does not translate WRITE SAME. [It might with more
recent firmware.]

Doug Gilbert