All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: torvic9@mailbox.org,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"hpa@redhat.com" <hpa@redhat.com>
Cc: "linux-ide@vger.kernel.org" <linux-ide@vger.kernel.org>,
	"axboe@kernel.dk" <axboe@kernel.dk>,
	"damien.lemoal@wdc.com" <damien.lemoal@wdc.com>
Subject: Re: [PATCH v4] libata: Add ATA_HORKAGE_NO_NCQ_ON_AMD for Samsung 860 and 870 SSD.
Date: Wed, 1 Sep 2021 10:55:37 +0200	[thread overview]
Message-ID: <e6f9921d-0fb6-da30-4dc5-53b4cb7b5270@redhat.com> (raw)
In-Reply-To: <1876334901.51676.1630481868266@office.mailbox.org>

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


  parent reply	other threads:[~2021-09-01  8:55 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-01  7:37 [PATCH v4] libata: Add ATA_HORKAGE_NO_NCQ_ON_AMD for Samsung 860 and 870 SSD torvic9
2021-09-01  8:13 ` Damien Le Moal
2021-09-01 17:56   ` Tor Vic
2021-09-01 18:58     ` Martin K. Petersen
2021-09-02  7:44       ` torvic9
2021-09-02 18:41         ` Martin K. Petersen
2021-09-01  8:55 ` Hans de Goede [this message]
2021-09-01  9:38   ` Hans de Goede
2021-09-01 11:11     ` torvic9
2021-09-01 17:59     ` Tor Vic
  -- strict thread matches above, loose matches on Subject: below --
2021-09-01  4:52 Kate Hsuan
2021-09-01  5:47 ` Damien Le Moal
2021-09-01  6:32 ` Christoph Hellwig
2021-09-01  9:01 ` Hans de Goede
2021-09-01  9:09   ` Kate Hsuan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e6f9921d-0fb6-da30-4dc5-53b4cb7b5270@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=axboe@kernel.dk \
    --cc=damien.lemoal@wdc.com \
    --cc=hpa@redhat.com \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvic9@mailbox.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.