[PATCH 1/1] libata: Fix ATA request sense

[PATCH 1/1] libata: Fix ATA request sense

[Damien Le Moal]

For an ATA device supporting the sense data reporting feature set,
a failed command will trigger the execution of ata_eh_request_sense if
the result task file of the failed command has the ATA_SENSE bit set
(sense data available bit). ata_eh_request_sense executes the
REQUEST SENSE DATA EXT command to retrieve the sense data of the failed
command. On success of REQUEST SENSE DATA EXT, the ATA_SENSE bit will
NOT be set (the command succeeded) but ata_eh_request_sense
nevertheless tests the availability of sense data by testing that bit
presence in the result tf of the REQUEST SENSE DATA EXT command.
This leads to a falsy assume that request sense data failed and to the
warning message:

atax.xx: request sense failed stat 50 emask 0

Upon success of REQUEST SENSE DATA EXT, set the ATA_SENSE bit in the
result task file command so that sense data can be returned by
ata_eh_request_sense.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 drivers/ata/libata-core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 9cd0a2d..366a380 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -1702,6 +1702,8 @@ unsigned ata_exec_internal_sg(struct ata_device *dev,
 
 		if (qc->err_mask & ~AC_ERR_OTHER)
 			qc->err_mask &= ~AC_ERR_OTHER;
+	} else if (qc->tf.command == ATA_CMD_REQ_SENSE_DATA) {
+		qc->result_tf.command |= ATA_SENSE;
 	}
 
 	/* finish up */
-- 
2.7.4

Re: [PATCH 1/1] libata: Fix ATA request sense

[Hannes Reinecke]

On 12/19/2016 02:17 AM, Damien Le Moal wrote:
> For an ATA device supporting the sense data reporting feature set,
> a failed command will trigger the execution of ata_eh_request_sense if
> the result task file of the failed command has the ATA_SENSE bit set
> (sense data available bit). ata_eh_request_sense executes the
> REQUEST SENSE DATA EXT command to retrieve the sense data of the failed
> command. On success of REQUEST SENSE DATA EXT, the ATA_SENSE bit will
> NOT be set (the command succeeded) but ata_eh_request_sense
> nevertheless tests the availability of sense data by testing that bit
> presence in the result tf of the REQUEST SENSE DATA EXT command.
> This leads to a falsy assume that request sense data failed and to the
> warning message:
> 
> atax.xx: request sense failed stat 50 emask 0
> 
> Upon success of REQUEST SENSE DATA EXT, set the ATA_SENSE bit in the
> result task file command so that sense data can be returned by
> ata_eh_request_sense.
> 
> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
> ---
>  drivers/ata/libata-core.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index 9cd0a2d..366a380 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -1702,6 +1702,8 @@ unsigned ata_exec_internal_sg(struct ata_device *dev,
>  
>  		if (qc->err_mask & ~AC_ERR_OTHER)
>  			qc->err_mask &= ~AC_ERR_OTHER;
> +	} else if (qc->tf.command == ATA_CMD_REQ_SENSE_DATA) {
> +		qc->result_tf.command |= ATA_SENSE;
>  	}
>  
>  	/* finish up */
> 
D'oh.

Of course.

Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH 1/1] libata: Fix ATA request sense

[Sergei Shtylyov]

Hello!

On 12/19/2016 4:17 AM, Damien Le Moal wrote:

> For an ATA device supporting the sense data reporting feature set,
> a failed command will trigger the execution of ata_eh_request_sense if
> the result task file of the failed command has the ATA_SENSE bit set
> (sense data available bit). ata_eh_request_sense executes the
> REQUEST SENSE DATA EXT command to retrieve the sense data of the failed
> command. On success of REQUEST SENSE DATA EXT, the ATA_SENSE bit will
> NOT be set (the command succeeded) but ata_eh_request_sense
> nevertheless tests the availability of sense data by testing that bit
> presence in the result tf of the REQUEST SENSE DATA EXT command.
> This leads to a falsy assume that request sense data failed and to the

    False assumption?

> warning message:
>
> atax.xx: request sense failed stat 50 emask 0
>
> Upon success of REQUEST SENSE DATA EXT, set the ATA_SENSE bit in the
> result task file command so that sense data can be returned by
> ata_eh_request_sense.
>
> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
[...]

MBR, Sergei

Re: [PATCH 1/1] libata: Fix ATA request sense

[Damien Le Moal]

On 12/19/16 21:19, Sergei Shtylyov wrote:
> Hello!
> 
> On 12/19/2016 4:17 AM, Damien Le Moal wrote:
> 
>> For an ATA device supporting the sense data reporting feature set,
>> a failed command will trigger the execution of ata_eh_request_sense if
>> the result task file of the failed command has the ATA_SENSE bit set
>> (sense data available bit). ata_eh_request_sense executes the
>> REQUEST SENSE DATA EXT command to retrieve the sense data of the failed
>> command. On success of REQUEST SENSE DATA EXT, the ATA_SENSE bit will
>> NOT be set (the command succeeded) but ata_eh_request_sense
>> nevertheless tests the availability of sense data by testing that bit
>> presence in the result tf of the REQUEST SENSE DATA EXT command.
>> This leads to a falsy assume that request sense data failed and to the
> 
>     False assumption?

"This leads to falsely assume that..."

My apologies for the typo.

> 
>> warning message:
>>
>> atax.xx: request sense failed stat 50 emask 0
>>
>> Upon success of REQUEST SENSE DATA EXT, set the ATA_SENSE bit in the
>> result task file command so that sense data can be returned by
>> ata_eh_request_sense.
>>
>> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
> [...]
> 
> MBR, Sergei
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ide" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 
Damien Le Moal, Ph.D.
Sr. Manager, System Software Research Group,
Western Digital Corporation
Damien.LeMoal@wdc.com
(+81) 0466-98-3593 (ext. 513593)
1 kirihara-cho, Fujisawa,
Kanagawa, 252-0888 Japan
www.wdc.com, www.hgst.com

Re: [PATCH 1/1] libata: Fix ATA request sense

[Sergei Shtylyov]

On 12/20/2016 5:25 AM, Damien Le Moal wrote:

>>> For an ATA device supporting the sense data reporting feature set,
>>> a failed command will trigger the execution of ata_eh_request_sense if
>>> the result task file of the failed command has the ATA_SENSE bit set
>>> (sense data available bit). ata_eh_request_sense executes the
>>> REQUEST SENSE DATA EXT command to retrieve the sense data of the failed
>>> command. On success of REQUEST SENSE DATA EXT, the ATA_SENSE bit will
>>> NOT be set (the command succeeded) but ata_eh_request_sense
>>> nevertheless tests the availability of sense data by testing that bit
>>> presence in the result tf of the REQUEST SENSE DATA EXT command.
>>> This leads to a falsy assume that request sense data failed and to the
>>
>>     False assumption?
>
> "This leads to falsely assume that..."

    "Leads us to falsely assume" then, maybe?

> My apologies for the typo.

MBR, Sergei

Re: [PATCH 1/1] libata: Fix ATA request sense

[Tejun Heo]

On Mon, Dec 19, 2016 at 10:17:40AM +0900, Damien Le Moal wrote:
> For an ATA device supporting the sense data reporting feature set,
> a failed command will trigger the execution of ata_eh_request_sense if
> the result task file of the failed command has the ATA_SENSE bit set
> (sense data available bit). ata_eh_request_sense executes the
> REQUEST SENSE DATA EXT command to retrieve the sense data of the failed
> command. On success of REQUEST SENSE DATA EXT, the ATA_SENSE bit will
> NOT be set (the command succeeded) but ata_eh_request_sense
> nevertheless tests the availability of sense data by testing that bit
> presence in the result tf of the REQUEST SENSE DATA EXT command.
> This leads to a falsy assume that request sense data failed and to the
> warning message:
> 
> atax.xx: request sense failed stat 50 emask 0
> 
> Upon success of REQUEST SENSE DATA EXT, set the ATA_SENSE bit in the
> result task file command so that sense data can be returned by
> ata_eh_request_sense.
> 
> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>

Applied to libata/for-4.10-fixes with patch description updated as
suggested by Sergei.

Thanks.

-- 
tejun

Re: [PATCH 1/1] libata: Fix ATA request sense

[Damien Le Moal]

Tejun,

On 1/7/17 05:44, Tejun Heo wrote:
> On Mon, Dec 19, 2016 at 10:17:40AM +0900, Damien Le Moal wrote:
>> For an ATA device supporting the sense data reporting feature set,
>> a failed command will trigger the execution of ata_eh_request_sense if
>> the result task file of the failed command has the ATA_SENSE bit set
>> (sense data available bit). ata_eh_request_sense executes the
>> REQUEST SENSE DATA EXT command to retrieve the sense data of the failed
>> command. On success of REQUEST SENSE DATA EXT, the ATA_SENSE bit will
>> NOT be set (the command succeeded) but ata_eh_request_sense
>> nevertheless tests the availability of sense data by testing that bit
>> presence in the result tf of the REQUEST SENSE DATA EXT command.
>> This leads to a falsy assume that request sense data failed and to the
>> warning message:
>>
>> atax.xx: request sense failed stat 50 emask 0
>>
>> Upon success of REQUEST SENSE DATA EXT, set the ATA_SENSE bit in the
>> result task file command so that sense data can be returned by
>> ata_eh_request_sense.
>>
>> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
> 
> Applied to libata/for-4.10-fixes with patch description updated as
> suggested by Sergei.

Thank you.

Should we send this patch to stable too ?
This bug does break applications on kernels 4.7 to 4.9 (e.g. libzbc test
suite and many tools we have using SGIO to directly send ATA commands to
SATA disks). I suspect sg-utils and hdparm may be affected in some way too.

Best regards.

-- 
Damien Le Moal, Ph.D.
Sr. Manager, System Software Research Group,
Western Digital Corporation
Damien.LeMoal@wdc.com
(+81) 0466-98-3593 (ext. 513593)
1 kirihara-cho, Fujisawa,
Kanagawa, 252-0888 Japan
www.wdc.com, www.hgst.com

Re: [PATCH 1/1] libata: Fix ATA request sense

[Tejun Heo]

On Tue, Jan 10, 2017 at 09:54:44AM +0900, Damien Le Moal wrote:
> Should we send this patch to stable too ?
> This bug does break applications on kernels 4.7 to 4.9 (e.g. libzbc test
> suite and many tools we have using SGIO to directly send ATA commands to
> SATA disks). I suspect sg-utils and hdparm may be affected in some way too.

Already marked for stable.

Thanks.

-- 
tejun