[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

[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 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

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