* [PATCH] ata: libata-core: Fix NO_FUA quirks shadowed by earlier quirk entries
@ 2026-10-13 19:39 Niklas Cassel
2026-10-14 14:34 ` Damien Le Moal
0 siblings, 1 reply; 2+ messages in thread
From: Niklas Cassel @ 2026-10-13 19:39 UTC (permalink / raw)
To: Damien Le Moal; +Cc: linux-ide, Niklas Cassel
ata_dev_quirks() uses the first entry in __ata_dev_quirks[] that
matches the device and ignores all later entries. Commit 38d431229bfb
("ata: libata: exclude FUA support for known buggy drives") appended
three ATA_QUIRK_NO_FUA entries to the end of the table, with model
patterns meant to cover whole product lines. Earlier entries already
match some or all of these drives, so NO_FUA is not always applied:
- "INTEL*SSDSC2CT*" never takes effect. Every model it matches also
matches the earlier "INTEL*SSD*" entry, so these drives only get
ATA_QUIRK_ZERO_AFTER_TRIM.
- "WDC*WD2500J*" does not take effect for "WDC WD2500JD-*" drives,
which match the earlier ATA_QUIRK_WD_BROKEN_LPM entry.
- "OCZ-VERTEX*" does not take effect for "OCZ-VERTEX" drives with
firmware "1.30", which match the earlier ATA_QUIRK_BROKEN_HPA entry.
Move the Intel entry in front of "INTEL*SSD*" and keep
ATA_QUIRK_ZERO_AFTER_TRIM in it, so these drives stay on the
ZERO_AFTER_TRIM whitelist. Add ATA_QUIRK_NO_FUA to the WD2500JD and
OCZ Vertex 1.30 entries. The quirks of all other drives are unchanged.
libata only uses FUA when it is enabled with libata.fua=1, so systems
using the default setting are not affected.
Fixes: 38d431229bfb ("ata: libata: exclude FUA support for known buggy drives")
Assisted-by: LLM
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
drivers/ata/libata-core.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 92233fb25051..cff4ededafad 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -4323,7 +4323,8 @@ static const struct ata_dev_quirks_entry __ata_dev_quirks[] = {
{ "MAXTOR 6L080L4", "A93.0500", ATA_QUIRK_BROKEN_HPA },
/* this one allows HPA unlocking but fails IOs on the area */
- { "OCZ-VERTEX", "1.30", ATA_QUIRK_BROKEN_HPA },
+ { "OCZ-VERTEX", "1.30", ATA_QUIRK_BROKEN_HPA |
+ ATA_QUIRK_NO_FUA },
/* Devices which report 1 sector over size HPA */
{ "ST340823A", NULL, ATA_QUIRK_HPA_SIZE },
@@ -4459,6 +4460,8 @@ static const struct ata_dev_quirks_entry __ata_dev_quirks[] = {
{ "Crucial*", NULL, ATA_QUIRK_ZERO_AFTER_TRIM },
{ "INTEL SSDSC2KG480G8", "XCV10120", ATA_QUIRK_ZERO_AFTER_TRIM |
ATA_QUIRK_MAX_SEC },
+ { "INTEL*SSDSC2CT*", NULL, ATA_QUIRK_ZERO_AFTER_TRIM |
+ ATA_QUIRK_NO_FUA },
{ "INTEL*SSD*", NULL, ATA_QUIRK_ZERO_AFTER_TRIM },
{ "SSD*INTEL*", NULL, ATA_QUIRK_ZERO_AFTER_TRIM },
{ "Samsung*SSD*", NULL, ATA_QUIRK_ZERO_AFTER_TRIM },
@@ -4479,7 +4482,8 @@ static const struct ata_dev_quirks_entry __ata_dev_quirks[] = {
{ "WDC WD1200JD-*", NULL, ATA_QUIRK_WD_BROKEN_LPM },
{ "WDC WD1600JD-*", NULL, ATA_QUIRK_WD_BROKEN_LPM },
{ "WDC WD2000JD-*", NULL, ATA_QUIRK_WD_BROKEN_LPM },
- { "WDC WD2500JD-*", NULL, ATA_QUIRK_WD_BROKEN_LPM },
+ { "WDC WD2500JD-*", NULL, ATA_QUIRK_WD_BROKEN_LPM |
+ ATA_QUIRK_NO_FUA },
{ "WDC WD3000JD-*", NULL, ATA_QUIRK_WD_BROKEN_LPM },
{ "WDC WD3200JD-*", NULL, ATA_QUIRK_WD_BROKEN_LPM },
@@ -4504,7 +4508,6 @@ static const struct ata_dev_quirks_entry __ata_dev_quirks[] = {
{ "Maxtor", "BANC1G10", ATA_QUIRK_NO_FUA },
{ "WDC*WD2500J*", NULL, ATA_QUIRK_NO_FUA },
{ "OCZ-VERTEX*", NULL, ATA_QUIRK_NO_FUA },
- { "INTEL*SSDSC2CT*", NULL, ATA_QUIRK_NO_FUA },
/* End Marker */
{ }
--
2.56.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] ata: libata-core: Fix NO_FUA quirks shadowed by earlier quirk entries
2026-10-13 19:39 [PATCH] ata: libata-core: Fix NO_FUA quirks shadowed by earlier quirk entries Niklas Cassel
@ 2026-10-14 14:34 ` Damien Le Moal
0 siblings, 0 replies; 2+ messages in thread
From: Damien Le Moal @ 2026-10-14 14:34 UTC (permalink / raw)
To: Niklas Cassel; +Cc: linux-ide
On 2026/10/13 21:39, Niklas Cassel wrote:
> ata_dev_quirks() uses the first entry in __ata_dev_quirks[] that
> matches the device and ignores all later entries. Commit 38d431229bfb
> ("ata: libata: exclude FUA support for known buggy drives") appended
> three ATA_QUIRK_NO_FUA entries to the end of the table, with model
> patterns meant to cover whole product lines. Earlier entries already
> match some or all of these drives, so NO_FUA is not always applied:
>
> - "INTEL*SSDSC2CT*" never takes effect. Every model it matches also
> matches the earlier "INTEL*SSD*" entry, so these drives only get
> ATA_QUIRK_ZERO_AFTER_TRIM.
>
> - "WDC*WD2500J*" does not take effect for "WDC WD2500JD-*" drives,
> which match the earlier ATA_QUIRK_WD_BROKEN_LPM entry.
>
> - "OCZ-VERTEX*" does not take effect for "OCZ-VERTEX" drives with
> firmware "1.30", which match the earlier ATA_QUIRK_BROKEN_HPA entry.
>
> Move the Intel entry in front of "INTEL*SSD*" and keep
> ATA_QUIRK_ZERO_AFTER_TRIM in it, so these drives stay on the
> ZERO_AFTER_TRIM whitelist. Add ATA_QUIRK_NO_FUA to the WD2500JD and
> OCZ Vertex 1.30 entries. The quirks of all other drives are unchanged.
>
> libata only uses FUA when it is enabled with libata.fua=1, so systems
> using the default setting are not affected.
>
> Fixes: 38d431229bfb ("ata: libata: exclude FUA support for known buggy drives")
> Assisted-by: LLM
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
Your solution works, but I think it is very fragile. We will most likely again
make the same mistake with the oredering. So what about something like this:
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 92233fb25051..980a6a6256b1 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -4515,6 +4515,7 @@ static u64 ata_dev_quirks(const struct ata_device *dev)
unsigned char model_num[ATA_ID_PROD_LEN + 1];
unsigned char model_rev[ATA_ID_FW_REV_LEN + 1];
const struct ata_dev_quirks_entry *ad = __ata_dev_quirks;
+ u64 quirks = 0;
/* dev->quirks is an u64. */
BUILD_BUG_ON(__ATA_QUIRK_MAX > 64);
@@ -4527,11 +4528,11 @@ static u64 ata_dev_quirks(const struct ata_device *dev)
(!ad->model_rev || glob_match(ad->model_rev, model_rev))) {
ata_dev_print_quirks(dev, model_num, model_rev,
ad->quirks);
- return ad->quirks;
+ quirks |= ad->quirks;
}
ad++;
}
- return 0;
+ return quirks;
}
static u64 ata_dev_get_max_sec_quirk_value(struct ata_device *dev)
With this, all quirks that apply to a device will apply, even if there are
multiple matching entries. That will solve the issue and make things a lot more
solid.
> ---
> drivers/ata/libata-core.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index 92233fb25051..cff4ededafad 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -4323,7 +4323,8 @@ static const struct ata_dev_quirks_entry __ata_dev_quirks[] = {
> { "MAXTOR 6L080L4", "A93.0500", ATA_QUIRK_BROKEN_HPA },
>
> /* this one allows HPA unlocking but fails IOs on the area */
> - { "OCZ-VERTEX", "1.30", ATA_QUIRK_BROKEN_HPA },
> + { "OCZ-VERTEX", "1.30", ATA_QUIRK_BROKEN_HPA |
> + ATA_QUIRK_NO_FUA },
>
> /* Devices which report 1 sector over size HPA */
> { "ST340823A", NULL, ATA_QUIRK_HPA_SIZE },
> @@ -4459,6 +4460,8 @@ static const struct ata_dev_quirks_entry __ata_dev_quirks[] = {
> { "Crucial*", NULL, ATA_QUIRK_ZERO_AFTER_TRIM },
> { "INTEL SSDSC2KG480G8", "XCV10120", ATA_QUIRK_ZERO_AFTER_TRIM |
> ATA_QUIRK_MAX_SEC },
> + { "INTEL*SSDSC2CT*", NULL, ATA_QUIRK_ZERO_AFTER_TRIM |
> + ATA_QUIRK_NO_FUA },
> { "INTEL*SSD*", NULL, ATA_QUIRK_ZERO_AFTER_TRIM },
> { "SSD*INTEL*", NULL, ATA_QUIRK_ZERO_AFTER_TRIM },
> { "Samsung*SSD*", NULL, ATA_QUIRK_ZERO_AFTER_TRIM },
> @@ -4479,7 +4482,8 @@ static const struct ata_dev_quirks_entry __ata_dev_quirks[] = {
> { "WDC WD1200JD-*", NULL, ATA_QUIRK_WD_BROKEN_LPM },
> { "WDC WD1600JD-*", NULL, ATA_QUIRK_WD_BROKEN_LPM },
> { "WDC WD2000JD-*", NULL, ATA_QUIRK_WD_BROKEN_LPM },
> - { "WDC WD2500JD-*", NULL, ATA_QUIRK_WD_BROKEN_LPM },
> + { "WDC WD2500JD-*", NULL, ATA_QUIRK_WD_BROKEN_LPM |
> + ATA_QUIRK_NO_FUA },
> { "WDC WD3000JD-*", NULL, ATA_QUIRK_WD_BROKEN_LPM },
> { "WDC WD3200JD-*", NULL, ATA_QUIRK_WD_BROKEN_LPM },
>
> @@ -4504,7 +4508,6 @@ static const struct ata_dev_quirks_entry __ata_dev_quirks[] = {
> { "Maxtor", "BANC1G10", ATA_QUIRK_NO_FUA },
> { "WDC*WD2500J*", NULL, ATA_QUIRK_NO_FUA },
> { "OCZ-VERTEX*", NULL, ATA_QUIRK_NO_FUA },
> - { "INTEL*SSDSC2CT*", NULL, ATA_QUIRK_NO_FUA },
>
> /* End Marker */
> { }
--
Damien Le Moal
Western Digital Research
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-10-14 14:34 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-13 19:39 [PATCH] ata: libata-core: Fix NO_FUA quirks shadowed by earlier quirk entries Niklas Cassel
2026-10-14 14:34 ` Damien Le Moal
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox