[1/4] libata: Allow NCQ TRIM to be enabled or disabled with a module parameter

We have started seeing SSD firmware updates introduce support for queued
TRIM. Sadly, in most cases this support is completely untested and can
lead to either errors or data corruption.

Add two libata force flags that can be used to either enable or disable
queued TRIM support.

Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
---
 Documentation/kernel-parameters.txt | 2 ++
 drivers/ata/libata-core.c           | 2 ++
 2 files changed, 4 insertions(+)

On 05/05/2015 03:54 AM, Martin K. Petersen wrote:
> We have started seeing SSD firmware updates introduce support for queued
> TRIM. Sadly, in most cases this support is completely untested and can
> lead to either errors or data corruption.
> 
> Add two libata force flags that can be used to either enable or disable
> queued TRIM support.
> 
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
> ---
>  Documentation/kernel-parameters.txt | 2 ++
>  drivers/ata/libata-core.c           | 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index 61ab1628a057..a2e4891a714f 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -1774,6 +1774,8 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>  
>  			* [no]ncq: Turn on or off NCQ.
>  
> +			* [no]ncqtrim: Turn off queued DSM TRIM.
> +
>  			* nohrst, nosrst, norst: suppress hard, soft
>                            and both resets.
>  
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index 4476fb590733..9cebd7872ac6 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -6472,6 +6472,8 @@ static int __init ata_parse_force_one(char **cur,
>  		{ "3.0Gbps",	.spd_limit	= 2 },
>  		{ "noncq",	.horkage_on	= ATA_HORKAGE_NONCQ },
>  		{ "ncq",	.horkage_off	= ATA_HORKAGE_NONCQ },
> +		{ "noncqtrim",	.horkage_on	= ATA_HORKAGE_NO_NCQ_TRIM },
> +		{ "ncqtrim",	.horkage_off	= ATA_HORKAGE_NO_NCQ_TRIM },
>  		{ "dump_id",	.horkage_on	= ATA_HORKAGE_DUMP_ID },
>  		{ "pio0",	.xfer_mask	= 1 << (ATA_SHIFT_PIO + 0) },
>  		{ "pio1",	.xfer_mask	= 1 << (ATA_SHIFT_PIO + 1) },
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes

On Mon, May 04, 2015 at 09:54:18PM -0400, Martin K. Petersen wrote:
> We have started seeing SSD firmware updates introduce support for queued
> TRIM. Sadly, in most cases this support is completely untested and can
> lead to either errors or data corruption.
> 
> Add two libata force flags that can be used to either enable or disable
> queued TRIM support.
> 
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

Applied 1-4 to libata/for-4.2.

Thanks.

On 05/05/2015 03:31 PM, Tejun Heo wrote:
> On Mon, May 04, 2015 at 09:54:18PM -0400, Martin K. Petersen wrote:
>> We have started seeing SSD firmware updates introduce support for queued
>> TRIM. Sadly, in most cases this support is completely untested and can
>> lead to either errors or data corruption.
>>
>> Add two libata force flags that can be used to either enable or disable
>> queued TRIM support.
>>
>> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
> 
> Applied 1-4 to libata/for-4.2.
> 
> Thanks.
> 
As a side note: queue TRIM requires 'SEND FPDMA QUEUED', which is an
_optional_ part of the NCQ feature set.
And unfortunately there are no identify bits telling us whether SEND
FPDMA QUEUE is actually implemented (or RECEIVE FPDMA QUEUED, for
that matter).
I've been advocating to implement this (it would come in very handy
for the SMR/ZAC stuff) but to no avail so far.
Needless to say, none of the drives I have implement SEND FPDMA
QUEUED...

Maybe it's an idea to poke the powers that be about additional
identify bits for this ...

Cheers,

Hannes

>>>>> "Hannes" == Hannes Reinecke <hare@suse.de> writes:

Hannes> As a side note: queue TRIM requires 'SEND FPDMA QUEUED', which
Hannes> is an _optional_ part of the NCQ feature set.  And unfortunately
Hannes> there are no identify bits telling us whether SEND FPDMA QUEUE
Hannes> is actually implemented (or RECEIVE FPDMA QUEUED, for that
Hannes> matter).

Yeah. At least in this case one would assume the command is supported
when queued TRIM is advertised.

But as you saw from my patch series, I have one particular drive that
advertises support for queued READ LOG and yet it fails when you attempt
to use it.

*sigh*