ata: libata-core: Fix NO_FUA quirks shadowed by earlier quirk entries

PATCH Applied Niklas Cassel 2 participants 5 messages Oct 13 – Oct 14 libata

Share & tools

Niklas Cassel

author maintainer

2026-10-13 19:39 UTC #0 lore raw

PATCH 1/1 ata: libata-core: Fix NO_FUA quirks shadowed by earlier quirk entries

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 <[email protected]>

---
 drivers/ata/libata-core.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

drivers/ata/libata-core.c +6 -3

index 92233fb25051..cff4ededafad 100644
@@ -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 */
 	{ }

Signature · 1 line

2.56.0

Damien Le Moal

maintainer

2026-10-14 14:34 UTC #1 lore raw

Quoted 22 lines · 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.

Quoted 6 lines

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 <[email protected]>

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:

drivers/ata/libata-core.c +3 -2

index 92233fb25051..980a6a6256b1 100644
@@ -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.

Quoted 45 lines

---
 drivers/ata/libata-core.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

drivers/ata/libata-core.c +6 -3

index 92233fb25051..cff4ededafad 100644
@@ -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 */
 	{ }

Signature · 2 lines

Damien Le Moal
Western Digital Research

Niklas Cassel

author maintainer

2026-10-14 14:44 UTC #2 lore raw

Quoted 47 lines · Damien Le Moal

Quoted 22 lines · 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.
Quoted 6 lines
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 <[email protected]>
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:
drivers/ata/libata-core.c +1 -0
index 92233fb25051..980a6a6256b1 100644
@@ -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))) {

drivers/ata/libata-core.c static u64 ata_dev_quirks(const struct ata_device *dev) · line 4536

Quoted 10 lines

drivers/ata/libata-core.c +2 -2

                        ata_dev_print_quirks(dev, model_num, model_rev,
                                             ad->quirks);
-                       return ad->quirks;
+                       quirks |= ad->quirks;
                }
                ad++;
        }
-       return 0;
+       return quirks;
 }

I like the idea.

Send it as a real patch :)

Kind regards,
Niklas

Damien Le Moal

maintainer Reviewed-by on v1 1/1

2026-10-14 15:22 UTC #3 lore raw

In reply to Niklas Cassel

Quoted 22 lines · 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.

Quoted 6 lines

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 <[email protected]>

Unfortunately, my idea does not work. So:

Reviewed-by Damien Le Moal <[email protected]>

Signature · 2 lines

Damien Le Moal
Western Digital Research

Niklas Cassel

author maintainer Applied

2026-10-14 17:31 UTC #4 lore raw

In reply to Niklas Cassel

Quoted 8 lines · 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:
[...]

Applied to libata/linux.git (for-7.4), thanks!

[1/1] ata: libata-core: Fix NO_FUA quirks shadowed by earlier quirk entries

      https://git.kernel.org/libata/linux/c/a9c6dc51

Kind regards,
Niklas