Skip to contentSkip to conversation

libata: disable queued trim for Micron M500 SSDs

PATCH New Marc Carino 2 participants 4 messages Dec 17, 2013 libata

lore
Share & tools
lore

Marc Carino

author
#0 lore raw

cover libata: disable queued trim for Micron M500 SSDs

There was a report of silent data corruption on Fedora 20 w/ the v3.12 kernel. After some analysis of SATA traces, it appears that in certain queued scenarios, the Micron M500 SSD will exhibit incorrect protocol behavior. A support request was sent to the vendor, and a firmware fix is in the works.

Until a firmware update is deployed for the Micron/Crucial M500 SSD, the ATA driver should avoid issuing the queued TRIM commands to the affected devices.

This patchset adds a new horkage flag to let the queued trim commands be disabled, depending on the drive model string.

Marc Carino (2):
libata: add horkage flag to disable issuance of queued TRIM commands
libata: disable queued TRIMs for Micron M500 SSDs

 drivers/ata/libata-core.c | 11 +++++++++++
 include/linux/libata.h    |  2 ++
 2 files changed, 13 insertions(+)
Signature · 1 line
1.8.1.3
author
#1 lore raw

PATCH 1/2 libata: add horkage flag to disable issuance of queued TRIM commands

Certain drives cannot handle queued TRIM commands properly, even though support is indicated in the IDENTIFY DEVICE buffer. This patch allows for disabling of the commands for the affected drives.

Signed-off-by Marc Carino <[email protected]>
---
 include/linux/libata.h | 2 ++
 1 file changed, 2 insertions(+)
include/linux/libata.h +2 -0
index 0e23c26..59f8f97 100644
@@ -418,6 +418,8 @@ enum {
 	ATA_HORKAGE_DUMP_ID	= (1 << 16),	/* dump IDENTIFY data */
 	ATA_HORKAGE_MAX_SEC_LBA48 = (1 << 17),	/* Set max sects to 65535 */
 	ATA_HORKAGE_ATAPI_DMADIR = (1 << 18),	/* device requires dmadir */
+	ATA_HORKAGE_BROKEN_QUEUED_TRIM = (1 << 19), /* device doesn't handle
+						       queued TRIMs correctly */
 
 	 /* DMA mask for user DMA control: User visible values; DO NOT
 	    renumber */
Signature · 1 line
1.8.1.3
author
#2 lore raw

PATCH 2/2 libata: disable queued TRIMs for Micron M500 SSDs

In reply to Marc Carino

The Micron/Crucial M500 SSDs exhibit incorrect protocol behavior when issuing queued TRIM commands, which could lead to silent data corruption.

Disable issuance of queued TRIM commands for the affected drives.

Signed-off-by Marc Carino <[email protected]>
---
 drivers/ata/libata-core.c | 11 +++++++++++
 1 file changed, 11 insertions(+)
drivers/ata/libata-core.c +11 -0
index ff01584..715ba06 100644
@@ -2152,6 +2152,13 @@ static int ata_dev_config_ncq(struct ata_device *dev,
 			dev->flags |= ATA_DFLAG_NCQ_SEND_RECV;
 			memcpy(dev->ncq_send_recv_cmds, ap->sector_buf,
 				ATA_LOG_NCQ_SEND_RECV_SIZE);
+
+			if (dev->horkage & ATA_HORKAGE_BROKEN_QUEUED_TRIM) {
+				ata_dev_dbg(dev,
+					    "disabling queued TRIM support\n");
+				dev->ncq_send_recv_cmds[ATA_LOG_NCQ_SEND_RECV_DSM_OFFSET]
+					&= ~ATA_LOG_NCQ_SEND_RECV_DSM_TRIM;
+			}
 		}
 	}
 

@@ -4205,6 +4212,10 @@ static const struct ata_blacklist_entry ata_device_blacklist [] = {
 	{ "PIONEER DVD-RW  DVR-212D",	NULL,	ATA_HORKAGE_NOSETXFER },
 	{ "PIONEER DVD-RW  DVR-216D",	NULL,	ATA_HORKAGE_NOSETXFER },
 
+	/* Devices that don't properly handle queued TRIM commands */
+	{ "Micron_M500*",		NULL,	ATA_HORKAGE_BROKEN_QUEUED_TRIM, },
+	{ "Crucial_CT???M500SSD1",	NULL,	ATA_HORKAGE_BROKEN_QUEUED_TRIM, },
+
 	/* End Marker */
 	{ }
 };
Signature · 1 line
1.8.1.3
#3 lore raw

PATCH 1/1 libata: implement ATA_HORKAGE_NO_NCQ_TRIM and apply it to Micro M500 SSDs

In reply to Marc Carino

Hello,

I merged the two patches and made some trivial changes, cc'd stable and applied to libata/for-3.13-fixes.

Thanks.
------- 8< -------

Quoted 1 line

From f78dea064c5f7de07de4912a6e5136dbc443d614 Mon Sep 17 00:00:00 2001

From: Marc Carino <[email protected]>
Date: Mon, 16 Dec 2013 18:15:53 -0800

Certain drives cannot handle queued TRIM commands properly, even though support is indicated in the IDENTIFY DEVICE buffer. This patch allows for disabling the commands for the affected drives and apply it to the Micron/Crucial M500 SSDs which exhibit incorrect protocol behavior when issued queued TRIM commands, which could lead to silent data corruption.

tj: Merged two unnecessarily split patches and made minor edits
including shortening horkage name.

Signed-off-by Marc Carino <[email protected]>
Signed-off-by Tejun Heo <[email protected]>
Link http://lkml.kernel.org/g/[email protected] open here
Cc [email protected] # 3.12+
---
 drivers/ata/libata-core.c | 15 +++++++++++++--
 include/linux/libata.h    |  1 +
 2 files changed, 14 insertions(+), 2 deletions(-)
drivers/ata/libata-core.c +13 -2
index ff01584..1393a58 100644
@@ -2149,9 +2149,16 @@ static int ata_dev_config_ncq(struct ata_device *dev,
 				    "failed to get NCQ Send/Recv Log Emask 0x%x\n",
 				    err_mask);
 		} else {
+			u8 *cmds = dev->ncq_send_recv_cmds;
+
 			dev->flags |= ATA_DFLAG_NCQ_SEND_RECV;
-			memcpy(dev->ncq_send_recv_cmds, ap->sector_buf,
-				ATA_LOG_NCQ_SEND_RECV_SIZE);
+			memcpy(cmds, ap->sector_buf, ATA_LOG_NCQ_SEND_RECV_SIZE);
+
+			if (dev->horkage & ATA_HORKAGE_NO_NCQ_TRIM) {
+				ata_dev_dbg(dev, "disabling queued TRIM support\n");
+				cmds[ATA_LOG_NCQ_SEND_RECV_DSM_OFFSET] &=
+					~ATA_LOG_NCQ_SEND_RECV_DSM_TRIM;
+			}
 		}
 	}
 

@@ -4205,6 +4212,10 @@ static const struct ata_blacklist_entry ata_device_blacklist [] = {
 	{ "PIONEER DVD-RW  DVR-212D",	NULL,	ATA_HORKAGE_NOSETXFER },
 	{ "PIONEER DVD-RW  DVR-216D",	NULL,	ATA_HORKAGE_NOSETXFER },
 
+	/* devices that don't properly handle queued TRIM commands */
+	{ "Micron_M500*",		NULL,	ATA_HORKAGE_NO_NCQ_TRIM, },
+	{ "Crucial_CT???M500SSD1",	NULL,	ATA_HORKAGE_NO_NCQ_TRIM, },
+
 	/* End Marker */
 	{ }
 };
include/linux/libata.h +1 -0
index 0e23c26..9b50337 100644
@@ -418,6 +418,7 @@ enum {
 	ATA_HORKAGE_DUMP_ID	= (1 << 16),	/* dump IDENTIFY data */
 	ATA_HORKAGE_MAX_SEC_LBA48 = (1 << 17),	/* Set max sects to 65535 */
 	ATA_HORKAGE_ATAPI_DMADIR = (1 << 18),	/* device requires dmadir */
+	ATA_HORKAGE_NO_NCQ_TRIM	= (1 << 19),	/* don't use queued TRIM */
 
 	 /* DMA mask for user DMA control: User visible values; DO NOT
 	    renumber */
Signature · 1 line
1.8.4.2