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;
Message-ID: <66b4c377-1b17-1972-847e-207620cc9364@mailbox.org>
Date:   Sun, 19 Sep 2021 16:27:22 +0000
MIME-Version: 1.0
Subject: Re: [PATCH v5] libata: Add ATA_HORKAGE_NO_NCQ_ON_AMD for Samsung 860
 and 870 SSD.
Content-Language: en-US
To:     Hans de Goede <hdegoede@redhat.com>,
        "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>
From:   Tor Vic <torvic9@mailbox.org>
In-Reply-To: <967d7639-fc31-a209-8c21-ea8ab3718de6@redhat.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


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?
Or is there a way to check that discard is effectively enabled?

Not sure if relevant, but here are a couple of lines from the syslog:

   ata4.00: 976773168 sectors, multi 1: LBA48 NCQ (depth 32), AA
   [...]
   ata4.00: Enabling discard_zeroes_data

Thanks!

> 
> Also note that AFAIK all major distros do not enable immediate
> discard, instead relying on fstrim runs from a cronjob, which
> again means this change will not impact users of those distros.
> 
> So chances are that your workload simply never triggered the issue;
> and this is the cause of everything always having worked fine for
> you.
> 
> Regards,
> 
> Hans
>