Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
IronPort-SDR: cXErdpdBjE/u4mAhYhHQkwgPeJJHfkr+8HczKsoNr7u3Nzjyh3abA9lXv+bZIjHGcPZ9i930RC
 Ptxhp97V7aKhYdIQI/5Wfsm+NXM1qkrHEp28uVBa5ZT3u0XXSeYaLf5lQbefVjmfYrcAgpVgpt
 HI0dcGwSfSc5lLz9v4UMYiJiayOeb6qr3POvsFaj8cLCqwD3o3OqxaOBwtuMy1JvCGg4TZCaC1
 jZ9qtsvzEGb5LILeUWbKA2gbMIcff2ycReqXOyLOFeqeDXF/WKKudiotfC4nOcUxctNTWYxkyX
 kdVH26PQKz7F9SCuR7epQV+d
IronPort-SDR: qfVH/bvYrlex066ZOzDbvXZ0SePVyTqacb+ZAKVTojgSIK7uGgCLac4IPAN4+8jdNJHpK5sE9F
 mZIiMLLBOdSZRjqa7j/ACscSFYSKD1hXZTda2pLNeAW38Q59YMvn3ZXHpD1RtohHmIVE2JNgdN
 yUmME+GeDqpXBjz3LuRI4VSX82pNvYsLNuq71Bh3u2098WvgAIC781bOvhP74XbZVxv/ekm1SF
 qj+yvqinc/7kAEqRKVdXCeu1w079SxME2OslfgXmiSioOXlLlD4JirZ6xV7Bae5gGgi3CMNx1G
 wAU=
WDCIronportException: Internal
Message-ID: <5dad0eb6-10e3-8319-ca86-f5059a7bd185@opensource.wdc.com>
Date:   Thu, 23 Jun 2022 18:55:42 +0900
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.10.0
Subject: Re: [PATCH] libata: add horkage for M88V29
Content-Language: en-US
To:     =?UTF-8?B?QsO2c3rDtnJtw6lueWkgWm9sdMOhbg==?= <zboszor@gmail.com>,
        zboszor@pr.hu, linux-kernel@vger.kernel.org,
        linux-ide@vger.kernel.org
References: <20220204125750.1771303-1-zboszor@pr.hu>
 <f726e9e1-bdad-ac8a-368b-aae423a00676@opensource.wdc.com>
 <78c29f43-3b67-8e70-0711-14e997f3efb1@gmail.com>
 <09091cc4-f652-0978-bb6a-b63f24fdcf49@opensource.wdc.com>
 <bbfeb862-9e1f-79c2-89dd-7db9515471e4@gmail.com>
 <7f086930-ff6c-da99-212b-46c4479247cb@opensource.wdc.com>
 <c678589c-cb30-35a3-cc7f-ad4065863640@gmail.com>
From:   Damien Le Moal <damien.lemoal@opensource.wdc.com>
Organization: Western Digital Research
In-Reply-To: <c678589c-cb30-35a3-cc7f-ad4065863640@gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On 6/23/22 18:32, B=C3=B6sz=C3=B6rm=C3=A9nyi Zolt=C3=A1n wrote:
> 2022. 06. 23. 10:46 keltez=C3=A9ssel, Damien Le Moal =C3=ADrta:
>> On 6/23/22 17:38, B=C3=B6sz=C3=B6rm=C3=A9nyi Zolt=C3=A1n wrote:
>>> 2022. 06. 23. 10:22 keltez=C3=A9ssel, Damien Le Moal =C3=ADrta:
>>>> On 6/23/22 16:47, B=C3=B6sz=C3=B6rm=C3=A9nyi Zolt=C3=A1n wrote:
>>>>> 2022. 02. 08. 9:07 keltez=C3=A9ssel, Damien Le Moal =C3=ADrta:
>>>>>> On 2/4/22 21:57, zboszor@pr.hu wrote:
>>>>>>> From: Zolt=C3=A1n B=C3=B6sz=C3=B6rm=C3=A9nyi <zboszor@gmail.com>
>>>>>>>
>>>>>>> This device is a CF card, or possibly an SSD in CF form factor.
>>>>>>> It supports NCQ and high speed DMA.
>>>>>>>
>>>>>>> While it also advertises TRIM support, I/O errors are reported
>>>>>>> when the discard mount option fstrim is used. TRIM also fails
>>>>>>> when disabling NCQ and not just as an NCQ command.
>>>>>>>
>>>>>>> TRIM must be disabled for this device.
>>>>>>>
>>>>>>> Signed-off-by: Zolt=C3=A1n B=C3=B6sz=C3=B6rm=C3=A9nyi <zboszor@gm=
ail.com>
>>>>>>> ---
>>>>>>>     drivers/ata/libata-core.c | 1 +
>>>>>>>     1 file changed, 1 insertion(+)
>>>>>>>
>>>>>>> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.=
c
>>>>>>> index 67f88027680a..4a7f58fcc411 100644
>>>>>>> --- a/drivers/ata/libata-core.c
>>>>>>> +++ b/drivers/ata/libata-core.c
>>>>>>> @@ -4028,6 +4028,7 @@ static const struct ata_blacklist_entry ata=
_device_blacklist [] =3D {
>>>>>>>    =20
>>>>>>>     	/* devices that don't properly handle TRIM commands */
>>>>>>>     	{ "SuperSSpeed S238*",		NULL,	ATA_HORKAGE_NOTRIM, },
>>>>>>> +	{ "M88V29*",			NULL,	ATA_HORKAGE_NOTRIM, },
>>>>>>>    =20
>>>>>>>     	/*
>>>>>>>     	 * As defined, the DRAT (Deterministic Read After Trim) and =
RZAT
>>>>>> Applied to for-5.17-fixes. Thanks !
>>>>> Thank you. However, I have second thoughts about this patch.
>>>>> The device advertises this:
>>>>>
>>>>> # hdparm -iI /dev/sda
>>>>> ...
>>>>>    =C2=A0Enabled Supported
>>>>>    =C2=A0=C2=A0=C2=A0 *=C2=A0=C2=A0=C2=A0 Data Set Management TRIM =
supported (limit 1 block)
>>>>> ...
>>>>>
>>>>> but the I/O failures always reported higher number of blocks,
>>>>> IIRC the attempted number of block was 8 or so.
>>>>>
>>>>> Can the kernel limit or split TRIM commands according to the
>>>>> advertised limit? If not (or not yet) then the quirk is good for no=
w.
>>>> Yes, the kernel does that. See the sysfs queue attributes
>>>> discard_max_bytes and discard_max_hw_bytes. What are the values for =
your
>>>> device ? I think that the "limit 1 block" indicated by hdparm is sim=
ply to
>>>> say that the DSM command (to trim the device) accept only at most a =
1
>>>> block (512 B) list of sectors to trim. That is not the actual trim l=
imit
>>>> for each sector range in that list.
>>> With the quirk in effect (TRIM disabled) I have these:
>>>
>>> [root@chef queue]# pwd
>>> /sys/block/sda/queue
>>> [root@chef queue]# cat discard_granularity
>>> 0
>>> [root@chef queue]# cat discard_max_bytes
>>> 0
>>> [root@chef queue]# cat discard_max_hw_bytes
>>> 0
>> Yes, expected. What are the values without the quirk applied ?
>=20
> I built 5.18.6 with removing the quirk.
>=20
> [root@chef queue]# pwd
> /sys/block/sda/queue/
> [root@chef queue]# cat discard_granularity
> 512
> [root@chef queue]# cat discard_max_bytes
> 2147450880
> [root@chef queue]# cat discard_max_hw_bytes
> 2147450880
> [root@chef queue]# cat max_discard_segments
> 1

All normal. This is 65535 sectors at most per trim range times 64 ranges
at most (64 range in at most 1 512B range list).
65535*512*64=3D2147450880

So if that is not working, then it means that DSM TRIM are not working
correctly on that device. Have you tried disabling NCQ trim ?
Use libata.force noncqtrim option.

>=20
>> With 5.19, you can use libata.force to disable/enable it. See
>> Documentation/admin-guide/kernel-parameters.txt for details.
>> You could try disabling DSM TRIM (queued trim) and see if the non-ncq =
trim
>> work.
>>
>=20


--=20
Damien Le Moal
Western Digital Research