Re: [PATCH v4] libata: Add ATA_HORKAGE_NO_NCQ_ON_AMD for Samsung 860 and 870 SSD.

(Sorry for not doing a proper reply)

Hello,
Noob here.
I have a Samsung 860 Pro connected to a AMD X570 chipset mainboard and
it just works flawlessly on 5.13 and 5.14.
Are you sure that *every* 860/870 is concerned by this problem on
*every* AMD controller? Isn't this too restrictive?
Or am I simply missing something?

Thanks,
Tor

On 2021/09/01 16:38, torvic9@mailbox.org wrote:

(Sorry for not doing a proper reply)

Hello,
Noob here.
I have a Samsung 860 Pro connected to a AMD X570 chipset mainboard and
it just works flawlessly on 5.13 and 5.14.
Are you sure that *every* 860/870 is concerned by this problem on
*every* AMD controller? Isn't this too restrictive?
Or am I simply missing something?

Is you drive being initialized with NCQ enabled ?

cat /sys/block/sdX/device/queue_depth ?

Thanks,
Tor

-- 
Damien Le Moal
Western Digital Research

On 01.09.21 08:13, Damien Le Moal wrote:

On 2021/09/01 16:38, torvic9@mailbox.org wrote:

quoted

(Sorry for not doing a proper reply)

Hello,
Noob here.
I have a Samsung 860 Pro connected to a AMD X570 chipset mainboard and
it just works flawlessly on 5.13 and 5.14.
Are you sure that *every* 860/870 is concerned by this problem on
*every* AMD controller? Isn't this too restrictive?
Or am I simply missing something?

Is you drive being initialized with NCQ enabled ?

cat /sys/block/sdX/device/queue_depth ?

Samsung 860 Pro:
$ cat /sys/block/sda/device/queue_depth


32

Samsung 860 Evo:
$ cat /sys/block/sdb/device/queue_depth


32

quoted

Thanks,
Tor

Hi Tor,

On 9/1/21 9:37 AM, torvic9@mailbox.org wrote:

(Sorry for not doing a proper reply)

Hello,
Noob here.
I have a Samsung 860 Pro connected to a AMD X570 chipset mainboard and
it just works flawlessly on 5.13 and 5.14.
Are you sure that *every* 860/870 is concerned by this problem on
*every* AMD controller?

I am pretty sure that every 860 / 870 EVO is affected,
I am not sure if the PRO is also affected.

As for *every* AMD controller, chances are that more recent
AMD controllers are fine.

We have been trying to resolve various issues with this combo
for a long time now, see:

https://bugzilla.kernel.org/show_bug.cgi?id=201693
https://bugzilla.kernel.org/show_bug.cgi?id=203475

Isn't this too restrictive?
Or am I simply missing something?

The problem is that when users are hit by this they end up with
a non functional system and even fs / data  corruption. Where
as OTOH disabling NCQ leads to a (significant) performance
degradation but affected systems will still work fine.

So I believe that it is best to err on the safe side here
and accept the performance degradation as a trade-of for
fixing the fs / data corruption.

With that said, I do believe that we should allow re-enabling
ncq on this combo through libata.force on the kernel cmdline
by adding this extra bit to the patch:

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c

@@ -6136,6 +6136,8 @@ static int __init ata_parse_force_one(char **cur,
 		{ "ncq",	.horkage_off	= ATA_HORKAGE_NONCQ },
 		{ "noncqtrim",	.horkage_on	= ATA_HORKAGE_NO_NCQ_TRIM },
 		{ "ncqtrim",	.horkage_off	= ATA_HORKAGE_NO_NCQ_TRIM },
+		{ "noncqamd",	.horkage_on	= ATA_HORKAGE_NO_NCQ_ON_AMD },
+		{ "ncqamd",	.horkage_off	= ATA_HORKAGE_NO_NCQ_ON_AMD },
 		{ "dump_id",	.horkage_on	= ATA_HORKAGE_DUMP_ID },
 		{ "pio0",	.xfer_mask	= 1 << (ATA_SHIFT_PIO + 0) },
 		{ "pio1",	.xfer_mask	= 1 << (ATA_SHIFT_PIO + 1) },

And I will also add a comment to both linked bugs to see if we can maybe
exclude the pro models from this quirk and if we can maybe narrow it down
to a subset of the AMD SATA controllers.

But that narrowing down is probably best done as a follow up fix, while just
going with this "err on the safe side" approach for now.

Regards,

Hans

Hi,

On 9/1/21 10:55 AM, Hans de Goede wrote:

Hi Tor,

On 9/1/21 9:37 AM, torvic9@mailbox.org wrote:

quoted

(Sorry for not doing a proper reply)

Hello,
Noob here.
I have a Samsung 860 Pro connected to a AMD X570 chipset mainboard and
it just works flawlessly on 5.13 and 5.14.
Are you sure that *every* 860/870 is concerned by this problem on
*every* AMD controller?

I am pretty sure that every 860 / 870 EVO is affected,
I am not sure if the PRO is also affected.

So while reading https://bugzilla.kernel.org/show_bug.cgi?id=201693
again to add a comment asking if anyone was seeing this on a
pro to I found existing comments of both queued-trims being
an issue on the 860 pro, as well as the 860 pro having issues
with some AMD sata controllers.

So it seems safe to say that the 860 pro has the same issues
as the 860 and 870 evo models. Chances are you don't have
discard support enabled causing you to not see the queued-trim
issues (which means you also won't see any difference from
disabling support for queued-trim commands).

So this just leaves your question of:

"concerned by this problem on *every* AMD controller?"

Where "this problem" is needing to completely disable NCQ
and I guess the answer is no, not every AMD controller
is affected. Still the plan is to err on the safe side for now,
allowing overriding this from the kernel cmdline with:

libata.force=ncqamd

I will add a comment to:

https://bugzilla.kernel.org/show_bug.cgi?id=201693

Asking for PCI-ids of the controllers where people are seeing
this and then maybe we can narrow down the "AMD" check in a
future follow up patch.

Regards,

Hans

Hans de Goede [off-list ref] hat am 01.09.2021 11:38 geschrieben:

 
Hi,

On 9/1/21 10:55 AM, Hans de Goede wrote:

quoted

Hi Tor,

On 9/1/21 9:37 AM, torvic9@mailbox.org wrote:

quoted

(Sorry for not doing a proper reply)

Hello,
Noob here.
I have a Samsung 860 Pro connected to a AMD X570 chipset mainboard and
it just works flawlessly on 5.13 and 5.14.
Are you sure that *every* 860/870 is concerned by this problem on
*every* AMD controller?

I am pretty sure that every 860 / 870 EVO is affected,
I am not sure if the PRO is also affected.

So while reading https://bugzilla.kernel.org/show_bug.cgi?id=201693
again to add a comment asking if anyone was seeing this on a
pro to I found existing comments of both queued-trims being
an issue on the 860 pro, as well as the 860 pro having issues
with some AMD sata controllers.

So it seems safe to say that the 860 pro has the same issues
as the 860 and 870 evo models. Chances are you don't have
discard support enabled causing you to not see the queued-trim
issues (which means you also won't see any difference from
disabling support for queued-trim commands).

Thanks for your answer, Hans.

If you mean the "discard" mount option, then yes, you're correct,
I don't use this because some (apparently) knowledgeable people
recommended against using it especially on LUKS partitions.
I don't know whether that's true though, it might be outdated.
I do however do manual TRIMs with "fstrim".

So this just leaves your question of:

"concerned by this problem on *every* AMD controller?"

Where "this problem" is needing to completely disable NCQ
and I guess the answer is no, not every AMD controller
is affected. Still the plan is to err on the safe side for now,
allowing overriding this from the kernel cmdline with:

libata.force=ncqamd

I agree to do it in a safe way, that sounds like a good solution.

I will add a comment to:

https://bugzilla.kernel.org/show_bug.cgi?id=201693

Asking for PCI-ids of the controllers where people are seeing
this and then maybe we can narrow down the "AMD" check in a
future follow up patch.

I can send you PCI and device IDs later if it helps.

Regards,

Hans

On 01.09.21 09:38, Hans de Goede wrote:

Hi,

On 9/1/21 10:55 AM, Hans de Goede wrote:

quoted

Hi Tor,

On 9/1/21 9:37 AM, torvic9@mailbox.org wrote:

quoted

(Sorry for not doing a proper reply)

Hello,
Noob here.
I have a Samsung 860 Pro connected to a AMD X570 chipset mainboard and
it just works flawlessly on 5.13 and 5.14.
Are you sure that *every* 860/870 is concerned by this problem on
*every* AMD controller?

I am pretty sure that every 860 / 870 EVO is affected,
I am not sure if the PRO is also affected.

So while reading https://bugzilla.kernel.org/show_bug.cgi?id=201693
again to add a comment asking if anyone was seeing this on a
pro to I found existing comments of both queued-trims being
an issue on the 860 pro, as well as the 860 pro having issues
with some AMD sata controllers.

So it seems safe to say that the 860 pro has the same issues
as the 860 and 870 evo models. Chances are you don't have
discard support enabled causing you to not see the queued-trim
issues (which means you also won't see any difference from
disabling support for queued-trim commands).

So this just leaves your question of:

"concerned by this problem on *every* AMD controller?"

Where "this problem" is needing to completely disable NCQ
and I guess the answer is no, not every AMD controller
is affected. Still the plan is to err on the safe side for now,
allowing overriding this from the kernel cmdline with:

libata.force=ncqamd

I will add a comment to:

https://bugzilla.kernel.org/show_bug.cgi?id=201693

Asking for PCI-ids of the controllers where people are seeing
this and then maybe we can narrow down the "AMD" check in a
future follow up patch.

$ lspci -nn | grep -i sata


06:00.0 SATA controller [0106]: Advanced Micro Devices, Inc. [AMD] FCH
SATA Controller [AHCI mode] [1022:7901] (rev 51)
07:00.0 SATA controller [0106]: Advanced Micro Devices, Inc. [AMD] FCH
SATA Controller [AHCI mode] [1022:7901] (rev 51)

Both Samsung 860 Pro and 860 Evo are connected to these.

Regards,

Hans