[PATCH 0/2] disable trim for SuperSSpeed S238 SSD

[PATCH 0/2] disable trim for SuperSSpeed S238 SSD

[Arne Fitzenreiter]

The SuperSSpeed S238 ssd lose data at trim, not only at queued trim
so this patchset introduce a new switch to disable trim and do this
for SuperSSpeed S238.

----
 drivers/ata/libata-core.c      |  3 +++
 drivers/ata/libata-scsi.c      | 14 ++++++++------
 drivers/ata/libata-transport.c |  2 ++
 include/linux/libata.h         |  2 ++
 4 files changed, 15 insertions(+), 6 deletions(-)
----

[PATCH 1/2] libata: add ATA_HORKAGE_NOTRIM

[Arne Fitzenreiter]

sone disks lose data at trim also with disabled ncq
so this add a switch to force disable trim.
---
 drivers/ata/libata-scsi.c      | 14 ++++++++------
 drivers/ata/libata-transport.c |  2 ++
 include/linux/libata.h         |  2 ++
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 3131adc..cc80e4d 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -2569,12 +2569,14 @@ static unsigned int ata_scsiop_read_cap(struct ata_scsi_args *args, u8 *rbuf)
 		rbuf[15] = lowest_aligned;
 
 		if (ata_id_has_trim(args->id)) {
-			rbuf[14] |= 0x80; /* LBPME */
-
-			if (ata_id_has_zero_after_trim(args->id) &&
-			    dev->horkage & ATA_HORKAGE_ZERO_AFTER_TRIM) {
-				ata_dev_info(dev, "Enabling discard_zeroes_data\n");
-				rbuf[14] |= 0x40; /* LBPRZ */
+			if (!(dev->horkage & ATA_HORKAGE_NOTRIM)) {
+				rbuf[14] |= 0x80; /* LBPME */
+
+				if (ata_id_has_zero_after_trim(args->id) &&
+				    dev->horkage & ATA_HORKAGE_ZERO_AFTER_TRIM) {
+					ata_dev_info(dev, "Enabling discard_zeroes_data\n");
+					rbuf[14] |= 0x40; /* LBPRZ */
+				}
 			}
 		}
 	}
diff --git a/drivers/ata/libata-transport.c b/drivers/ata/libata-transport.c
index d6c37bc..3a13c7d 100644
--- a/drivers/ata/libata-transport.c
+++ b/drivers/ata/libata-transport.c
@@ -569,6 +569,8 @@ show_ata_dev_trim(struct device *dev,
 
 	if (!ata_id_has_trim(ata_dev->id))
 		mode = "unsupported";
+	else if (ata_dev->horkage & ATA_HORKAGE_NOTRIM)
+			mode = "forced_unsupported";
 	else if (ata_dev->horkage & ATA_HORKAGE_NO_NCQ_TRIM)
 			mode = "forced_unqueued";
 	else if (ata_fpdma_dsm_supported(ata_dev))
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 36ce37b..5c8bac6 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -431,6 +431,8 @@ enum {
 	ATA_HORKAGE_WD_BROKEN_LPM = (1 << 21),	/* some WDs have broken LPM */
 	ATA_HORKAGE_ZERO_AFTER_TRIM = (1 << 22),/* guarantees zero after trim */
 	ATA_HORKAGE_NO_NCQ_LOG	= (1 << 23),	/* don't use NCQ for log read */
+	ATA_HORKAGE_NOTRIM	= (1 << 24),	/* don't use TRIM */
+
 
 	 /* DMA mask for user DMA control: User visible values; DO NOT
 	    renumber */
-- 
2.4.4

Re: [PATCH 1/2] libata: add ATA_HORKAGE_NOTRIM

[Sergei Shtylyov]

Hello.

On 7/14/2015 2:28 PM, Arne Fitzenreiter wrote:

> sone disks lose data at trim also with disabled ncq
> so this add a switch to force disable trim.

    You forgot to sign off on the patch, so it can't be applied.

[...]
> diff --git a/drivers/ata/libata-transport.c b/drivers/ata/libata-transport.c
> index d6c37bc..3a13c7d 100644
> --- a/drivers/ata/libata-transport.c
> +++ b/drivers/ata/libata-transport.c
> @@ -569,6 +569,8 @@ show_ata_dev_trim(struct device *dev,
>
>   	if (!ata_id_has_trim(ata_dev->id))
>   		mode = "unsupported";
> +	else if (ata_dev->horkage & ATA_HORKAGE_NOTRIM)
> +			mode = "forced_unsupported";

    Indented too much.

>   	else if (ata_dev->horkage & ATA_HORKAGE_NO_NCQ_TRIM)
>   			mode = "forced_unqueued";

    Hm, I see where this comes from...

[...]

MBR, Sergei

Re: [PATCH 1/2] libata: add ATA_HORKAGE_NOTRIM

[arne_f]

On 2015-07-14 13:43, Sergei Shtylyov wrote:
> Hello.
> 
> On 7/14/2015 2:28 PM, Arne Fitzenreiter wrote:
> 
>> sone disks lose data at trim also with disabled ncq
>> so this add a switch to force disable trim.
> 
>    You forgot to sign off on the patch, so it can't be applied.
Ok. Next time i will not forget the "-s"
> 
> [...]
>> diff --git a/drivers/ata/libata-transport.c 
>> b/drivers/ata/libata-transport.c
>> index d6c37bc..3a13c7d 100644
>> --- a/drivers/ata/libata-transport.c
>> +++ b/drivers/ata/libata-transport.c
>> @@ -569,6 +569,8 @@ show_ata_dev_trim(struct device *dev,
>> 
>>   	if (!ata_id_has_trim(ata_dev->id))
>>   		mode = "unsupported";
>> +	else if (ata_dev->horkage & ATA_HORKAGE_NOTRIM)
>> +			mode = "forced_unsupported";
> 
>    Indented too much.
Should i change this to "unsupported" or better to something else?

Greets,
Arne

Re: [PATCH 1/2] libata: add ATA_HORKAGE_NOTRIM

[Sergei Shtylyov]

On 7/14/2015 3:16 PM, arne_f wrote:

>>> sone disks lose data at trim also with disabled ncq
>>> so this add a switch to force disable trim.

>>    You forgot to sign off on the patch, so it can't be applied.

> Ok. Next time i will not forget the "-s"

>> [...]
>>> diff --git a/drivers/ata/libata-transport.c b/drivers/ata/libata-transport.c
>>> index d6c37bc..3a13c7d 100644
>>> --- a/drivers/ata/libata-transport.c
>>> +++ b/drivers/ata/libata-transport.c
>>> @@ -569,6 +569,8 @@ show_ata_dev_trim(struct device *dev,
>>>
>>>       if (!ata_id_has_trim(ata_dev->id))
>>>           mode = "unsupported";
>>> +    else if (ata_dev->horkage & ATA_HORKAGE_NOTRIM)
>>> +            mode = "forced_unsupported";
>>
>>    Indented too much.

> Should i change this to "unsupported" or better to something else?

   I just meant the line started too far to the right, that's all.

> Greets,
> Arne

MBR, Sergei

Re: [PATCH 1/2] libata: add ATA_HORKAGE_NOTRIM

[Tejun Heo]

On Tue, Jul 14, 2015 at 01:28:33PM +0200, Arne Fitzenreiter wrote:
> sone disks lose data at trim also with disabled ncq
> so this add a switch to force disable trim.

Some devices lose data on TRIM whether queued or not.  This patch adds
a horkage to disable TRIM.

Signed-off-by:....

Thanks.

-- 
tejun

[PATCH 2/2] libata: force disable trim for SuperSSpeed S238

[Arne Fitzenreiter]

this disc lose blocks if trim is used. most time the partition
area.
---
 drivers/ata/libata-core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index e83fc3d..d2d3b21 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -4238,6 +4238,9 @@ static const struct ata_blacklist_entry ata_device_blacklist [] = {
 	{ "Samsung SSD 8*",		NULL,	ATA_HORKAGE_NO_NCQ_TRIM |
 						ATA_HORKAGE_ZERO_AFTER_TRIM, },
 
+	/* devices that don't properly handle TRIM commands */
+	{ "SuperSSpeed S238*",		NULL,	ATA_HORKAGE_NOTRIM, },
+
 	/*
 	 * As defined, the DRAT (Deterministic Read After Trim) and RZAT
 	 * (Return Zero After Trim) flags in the ATA Command Set are
-- 
2.4.4

Re: [PATCH 2/2] libata: force disable trim for SuperSSpeed S238

[Tejun Heo]

On Tue, Jul 14, 2015 at 01:28:34PM +0200, Arne Fitzenreiter wrote:
> this disc lose blocks if trim is used. most time the partition
> area.

This device loses blocks, often the partition table area, on trim.
Disable TRIM.

Signed-off-by:...
Cc: stable@vger.kernel.org  (the previous patch too)

Also, how did you learn that the device loses data on trim?  Is there
a report?  If this is something you noticed yourself, can you please
detail what you observed and how reproducible the problem is?

Thanks.

-- 
tejun

[PATCH 0/2] disable trim for SuperSSpeed S238 SSD - v2

[Arne Fitzenreiter]

The SuperSSpeed S238 loose data at trim, not only at queued trim
so this patchset add a switch to disable trim amd do it for this
disc.

V1: - initial patchset
V2: - signed-off commits
    - fix indenting

----
 drivers/ata/libata-core.c      |  3 +++
 drivers/ata/libata-scsi.c      | 14 ++++++++------
 drivers/ata/libata-transport.c |  2 ++
 include/linux/libata.h         |  2 ++
 4 files changed, 15 insertions(+), 6 deletions(-)

[PATCH 1/2] libata: add ATA_HORKAGE_NOTRIM

[Arne Fitzenreiter]

sone disks lose data at trim also with disabled ncq
so this add a switch to force disable trim.

Signed-off-by: Arne Fitzenreiter <arne_f@ipfire.org>
---
 drivers/ata/libata-scsi.c      | 14 ++++++++------
 drivers/ata/libata-transport.c |  2 ++
 include/linux/libata.h         |  2 ++
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 3131adc..cc80e4d 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -2569,12 +2569,14 @@ static unsigned int ata_scsiop_read_cap(struct ata_scsi_args *args, u8 *rbuf)
 		rbuf[15] = lowest_aligned;
 
 		if (ata_id_has_trim(args->id)) {
-			rbuf[14] |= 0x80; /* LBPME */
-
-			if (ata_id_has_zero_after_trim(args->id) &&
-			    dev->horkage & ATA_HORKAGE_ZERO_AFTER_TRIM) {
-				ata_dev_info(dev, "Enabling discard_zeroes_data\n");
-				rbuf[14] |= 0x40; /* LBPRZ */
+			if (!(dev->horkage & ATA_HORKAGE_NOTRIM)) {
+				rbuf[14] |= 0x80; /* LBPME */
+
+				if (ata_id_has_zero_after_trim(args->id) &&
+				    dev->horkage & ATA_HORKAGE_ZERO_AFTER_TRIM) {
+					ata_dev_info(dev, "Enabling discard_zeroes_data\n");
+					rbuf[14] |= 0x40; /* LBPRZ */
+				}
 			}
 		}
 	}
diff --git a/drivers/ata/libata-transport.c b/drivers/ata/libata-transport.c
index d6c37bc..e2d9497 100644
--- a/drivers/ata/libata-transport.c
+++ b/drivers/ata/libata-transport.c
@@ -569,6 +569,8 @@ show_ata_dev_trim(struct device *dev,
 
 	if (!ata_id_has_trim(ata_dev->id))
 		mode = "unsupported";
+	else if (ata_dev->horkage & ATA_HORKAGE_NOTRIM)
+		mode = "forced_unsupported";
 	else if (ata_dev->horkage & ATA_HORKAGE_NO_NCQ_TRIM)
 			mode = "forced_unqueued";
 	else if (ata_fpdma_dsm_supported(ata_dev))
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 36ce37b..5c8bac6 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -431,6 +431,8 @@ enum {
 	ATA_HORKAGE_WD_BROKEN_LPM = (1 << 21),	/* some WDs have broken LPM */
 	ATA_HORKAGE_ZERO_AFTER_TRIM = (1 << 22),/* guarantees zero after trim */
 	ATA_HORKAGE_NO_NCQ_LOG	= (1 << 23),	/* don't use NCQ for log read */
+	ATA_HORKAGE_NOTRIM	= (1 << 24),	/* don't use TRIM */
+
 
 	 /* DMA mask for user DMA control: User visible values; DO NOT
 	    renumber */
-- 
2.4.4

[PATCH 0/2] disable trim for SuperSSpeed S238 SSD - v3

[Arne Fitzenreiter]

The SuperSSpeed S238 loose blocks at TRIM so this patchset add
a horkage to disable trim and set it for this disc.
A european distributor (pcengines.ch) has already started
to refund or exchange the discs by a version with disabled TRIM.
http://pcengines.ch/msata16a.htm

V1: - initial patchset
V2: - signed-off commits
    - fix indenting
V3: - better commit messages

--
 drivers/ata/libata-core.c      |  3 +++
 drivers/ata/libata-scsi.c      | 14 ++++++++------
 drivers/ata/libata-transport.c |  2 ++
 include/linux/libata.h         |  2 ++
 4 files changed, 15 insertions(+), 6 deletions(-)
--

[PATCH 1/2] libata: add ATA_HORKAGE_NOTRIM

[Arne Fitzenreiter]

Some devices lose data on TRIM whether queued or not.  This patch adds
a horkage to disable TRIM.

Signed-off-by: Arne Fitzenreiter <arne_f@ipfire.org>
---
 drivers/ata/libata-scsi.c      | 14 ++++++++------
 drivers/ata/libata-transport.c |  2 ++
 include/linux/libata.h         |  2 ++
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 3131adc..cc80e4d 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -2569,12 +2569,14 @@ static unsigned int ata_scsiop_read_cap(struct ata_scsi_args *args, u8 *rbuf)
 		rbuf[15] = lowest_aligned;
 
 		if (ata_id_has_trim(args->id)) {
-			rbuf[14] |= 0x80; /* LBPME */
-
-			if (ata_id_has_zero_after_trim(args->id) &&
-			    dev->horkage & ATA_HORKAGE_ZERO_AFTER_TRIM) {
-				ata_dev_info(dev, "Enabling discard_zeroes_data\n");
-				rbuf[14] |= 0x40; /* LBPRZ */
+			if (!(dev->horkage & ATA_HORKAGE_NOTRIM)) {
+				rbuf[14] |= 0x80; /* LBPME */
+
+				if (ata_id_has_zero_after_trim(args->id) &&
+				    dev->horkage & ATA_HORKAGE_ZERO_AFTER_TRIM) {
+					ata_dev_info(dev, "Enabling discard_zeroes_data\n");
+					rbuf[14] |= 0x40; /* LBPRZ */
+				}
 			}
 		}
 	}
diff --git a/drivers/ata/libata-transport.c b/drivers/ata/libata-transport.c
index d6c37bc..e2d9497 100644
--- a/drivers/ata/libata-transport.c
+++ b/drivers/ata/libata-transport.c
@@ -569,6 +569,8 @@ show_ata_dev_trim(struct device *dev,
 
 	if (!ata_id_has_trim(ata_dev->id))
 		mode = "unsupported";
+	else if (ata_dev->horkage & ATA_HORKAGE_NOTRIM)
+		mode = "forced_unsupported";
 	else if (ata_dev->horkage & ATA_HORKAGE_NO_NCQ_TRIM)
 			mode = "forced_unqueued";
 	else if (ata_fpdma_dsm_supported(ata_dev))
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 36ce37b..5c8bac6 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -431,6 +431,8 @@ enum {
 	ATA_HORKAGE_WD_BROKEN_LPM = (1 << 21),	/* some WDs have broken LPM */
 	ATA_HORKAGE_ZERO_AFTER_TRIM = (1 << 22),/* guarantees zero after trim */
 	ATA_HORKAGE_NO_NCQ_LOG	= (1 << 23),	/* don't use NCQ for log read */
+	ATA_HORKAGE_NOTRIM	= (1 << 24),	/* don't use TRIM */
+
 
 	 /* DMA mask for user DMA control: User visible values; DO NOT
 	    renumber */
-- 
2.4.4

Re: [PATCH 1/2] libata: add ATA_HORKAGE_NOTRIM

[Greg KH]

On Wed, Jul 15, 2015 at 01:54:36PM +0200, Arne Fitzenreiter wrote:
> Some devices lose data on TRIM whether queued or not.  This patch adds
> a horkage to disable TRIM.
> 
> Signed-off-by: Arne Fitzenreiter <arne_f@ipfire.org>
> ---
>  drivers/ata/libata-scsi.c      | 14 ++++++++------
>  drivers/ata/libata-transport.c |  2 ++
>  include/linux/libata.h         |  2 ++
>  3 files changed, 12 insertions(+), 6 deletions(-)

<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read Documentation/stable_kernel_rules.txt
for how to do this properly.

</formletter>

Re: [PATCH 1/2] libata: add ATA_HORKAGE_NOTRIM

[Tejun Heo]

Hello,

Applied to libata/for-4.2-fixes with a minor adjustment and stable tag
added.

Thanks.
----- 8< -----
>From 71d126fd28de2d4d9b7b2088dbccd7ca62fad6e0 Mon Sep 17 00:00:00 2001
From: Arne Fitzenreiter <arne_f@ipfire.org>
Date: Wed, 15 Jul 2015 13:54:36 +0200
Subject: [PATCH] libata: add ATA_HORKAGE_NOTRIM

Some devices lose data on TRIM whether queued or not.  This patch adds
a horkage to disable TRIM.

tj: Collapsed unnecessary if() nesting.

Signed-off-by: Arne Fitzenreiter <arne_f@ipfire.org>
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: stable@vger.kernel.org
---
 drivers/ata/libata-scsi.c      | 3 ++-
 drivers/ata/libata-transport.c | 2 ++
 include/linux/libata.h         | 2 ++
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 3131adc..641a61a 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -2568,7 +2568,8 @@ static unsigned int ata_scsiop_read_cap(struct ata_scsi_args *args, u8 *rbuf)
 		rbuf[14] = (lowest_aligned >> 8) & 0x3f;
 		rbuf[15] = lowest_aligned;
 
-		if (ata_id_has_trim(args->id)) {
+		if (ata_id_has_trim(args->id) &&
+		    !(dev->horkage & ATA_HORKAGE_NOTRIM)) {
 			rbuf[14] |= 0x80; /* LBPME */
 
 			if (ata_id_has_zero_after_trim(args->id) &&
diff --git a/drivers/ata/libata-transport.c b/drivers/ata/libata-transport.c
index d6c37bc..e2d9497 100644
--- a/drivers/ata/libata-transport.c
+++ b/drivers/ata/libata-transport.c
@@ -569,6 +569,8 @@ show_ata_dev_trim(struct device *dev,
 
 	if (!ata_id_has_trim(ata_dev->id))
 		mode = "unsupported";
+	else if (ata_dev->horkage & ATA_HORKAGE_NOTRIM)
+		mode = "forced_unsupported";
 	else if (ata_dev->horkage & ATA_HORKAGE_NO_NCQ_TRIM)
 			mode = "forced_unqueued";
 	else if (ata_fpdma_dsm_supported(ata_dev))
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 36ce37b..5c8bac6 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -431,6 +431,8 @@ enum {
 	ATA_HORKAGE_WD_BROKEN_LPM = (1 << 21),	/* some WDs have broken LPM */
 	ATA_HORKAGE_ZERO_AFTER_TRIM = (1 << 22),/* guarantees zero after trim */
 	ATA_HORKAGE_NO_NCQ_LOG	= (1 << 23),	/* don't use NCQ for log read */
+	ATA_HORKAGE_NOTRIM	= (1 << 24),	/* don't use TRIM */
+
 
 	 /* DMA mask for user DMA control: User visible values; DO NOT
 	    renumber */
-- 
2.4.3

Re: [PATCH 1/2] libata: add ATA_HORKAGE_NOTRIM

[Sergei Shtylyov]

Hello.

On 07/15/2015 02:54 PM, Arne Fitzenreiter wrote:

> Some devices lose data on TRIM whether queued or not.  This patch adds
> a horkage to disable TRIM.

> Signed-off-by: Arne Fitzenreiter <arne_f@ipfire.org>

    Sorry for not complaining about the below items earlier, perhaps Tejun 
could fix while appying?

[...]
> diff --git a/include/linux/libata.h b/include/linux/libata.h
> index 36ce37b..5c8bac6 100644
> --- a/include/linux/libata.h
> +++ b/include/linux/libata.h
> @@ -431,6 +431,8 @@ enum {
>   	ATA_HORKAGE_WD_BROKEN_LPM = (1 << 21),	/* some WDs have broken LPM */
>   	ATA_HORKAGE_ZERO_AFTER_TRIM = (1 << 22),/* guarantees zero after trim */
>   	ATA_HORKAGE_NO_NCQ_LOG	= (1 << 23),	/* don't use NCQ for log read */
> +	ATA_HORKAGE_NOTRIM	= (1 << 24),	/* don't use TRIM */

    BTW, why not ATA_HORKAGE_NO_TRIM like the others?

> +

    Empty line not needed here.

MBR, Sergei

Re: [PATCH 1/2] libata: add ATA_HORKAGE_NOTRIM

[Tejun Heo]

Hello,

On Wed, Jul 15, 2015 at 08:23:33PM +0300, Sergei Shtylyov wrote:
> >@@ -431,6 +431,8 @@ enum {
> >  	ATA_HORKAGE_WD_BROKEN_LPM = (1 << 21),	/* some WDs have broken LPM */
> >  	ATA_HORKAGE_ZERO_AFTER_TRIM = (1 << 22),/* guarantees zero after trim */
> >  	ATA_HORKAGE_NO_NCQ_LOG	= (1 << 23),	/* don't use NCQ for log read */
> >+	ATA_HORKAGE_NOTRIM	= (1 << 24),	/* don't use TRIM */
> 
>    BTW, why not ATA_HORKAGE_NO_TRIM like the others?

We actually have a bunch of NOXXX's.  We prolly wanna clean them up
together later but things like this ultimately don't really matter.

> >+
> 
>    Empty line not needed here.

I removed it while merging a later patch.

Thanks.

-- 
tejun