Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v5] libata: Add ATA_HORKAGE_NO_NCQ_ON_AMD for Samsung 860
 and 870 SSD.
To:     Tor Vic <torvic9@mailbox.org>,
        "Martin K. Petersen" <martin.petersen@oracle.com>
Cc:     Kate Hsuan <hpa@redhat.com>, Jens Axboe <axboe@kernel.dk>,
        Damien Le Moal <damien.lemoal@wdc.com>,
        linux-ide@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20210901151643.13562-1-hpa@redhat.com>
 <3e26e7a5-0d99-b993-d5ce-aa517e1bf1bb@redhat.com>
 <yq1h7f24y6f.fsf@ca-mkp.ca.oracle.com>
 <238d0841-0f03-928f-5441-89d5c9dcf9b9@redhat.com>
 <cd75fa32-8c4d-664e-5adb-f2f325d3c58e@redhat.com>
 <yq14kb24e97.fsf@ca-mkp.ca.oracle.com>
 <f071dfb3-1aad-003c-00bc-6b7ecf103e91@mailbox.org>
 <967d7639-fc31-a209-8c21-ea8ab3718de6@redhat.com>
 <66b4c377-1b17-1972-847e-207620cc9364@mailbox.org>
From:   Hans de Goede <hdegoede@redhat.com>
Message-ID: <361d702e-44c4-625f-5b6b-121a4dc07105@redhat.com>
Date:   Sun, 19 Sep 2021 18:51:18 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.11.0
MIME-Version: 1.0
In-Reply-To: <66b4c377-1b17-1972-847e-207620cc9364@mailbox.org>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Precedence: bulk

Hi,

On 9/19/21 6:27 PM, Tor Vic wrote:
> 
> 
> On 19.09.21 15:27, Hans de Goede wrote:
>> Hi Tor,
>>
>> On 9/19/21 4:24 PM, Tor Vic wrote:
>>> Hi,
>>>
>>> I saw that v2 (?) of this patch has made it into stable, which
>>> is quite reasonable given the number of bug reports.
>>> Are there any plans to "enhance" this patch once sufficient data
>>> on controller support/drive combinations has been collected?
>>
>> ATM there are no plans to limit these quirks, we have bug
>> reports of queued trims being an issue over all usual chip-vendors
>> of sata controllers (including more recent AMD models).
>>
>> Note that unless you have immediate "discard" enabled as an option
>> on all layers of your storage stack (dmcrypt, device-mapper/raid,
>> filesystem) then this change will not impact you at all.
> 
> Is that the "discard" mount option?
> I added this to one of the partitions residing on my 860 Evo,
> reverted the patch, and it still seems to work just fine.
> 
>   $ mount | grep sdb
> 
> 
>   /dev/sdb1 on /mnt/vbox type ext4 (rw,nosuid,nodev,noatime,discard)
> 
> Is there another place where discard has to be enabled?

No since you do not seem to be using dmcrypt/raid/lvm that should
do the trick.

Except that it sounds like this is a partition carrying vm
images. Those never delete storage, they only grow, to
effectively trim you need to either punch holes in files,
or remove files. Discard only comes in to play when used
diskspace becomes unused.

To test preferably remove several large files at once while
also generating a whole bunch of other diskio (e.g.
compile the kernel while also deleting several large files
from the same disk, with discard enable).

But even if that works for you, that is 1 report that this
works in some cases, vs many that it does not work; and also
note that you had to manually enable this, it was not
enabled before. So this really is going to impact the
performance of very few users, while looking at the amount
of bugreports about hangs / disk-corruption the problem
of having queued-trim support enabled is much much bigger,
so I see very little reason to re-enable this even if it
happens to work in your case.

> Or is there a way to check that discard is effectively enabled?

There probably is, but I don't know where / how to check this.

Regards,

Hans