Hi,
commit 84a9a8cd9d0aa93c17e5815ab8a9cc4c0a765c63
("[libata] Set proper SK when CK_COND is set") breaks
"SMART Status command" ATA command issued by
smartctl -d ata -a /dev/sda at least on FUJITSU MHV2060AH
on ICH6 IDE Controller. The kernel returns -EIO on
HDIO_DRIVE_TASK (0x31e) or HDIO_DRIVE_CMD (0x31f) ioctl,
probably due to RECOVERED_ERROR translated to -EIO.
The regression still exists in v3.9-rc3-218-g0a7e453.
Ancient smartctl like 5.33 give up after such error:
smartctl version 5.33 [i686-pc-linux-gnu] Copyright (C) 2002-4 Bruce Allen
Home page is http://smartmontools.sourceforge.net/
=== START OF INFORMATION SECTION ===
Device Model: FUJITSU MHV2060AH
Serial Number: NT29T5B2D23C
Firmware Version: 00830096
User Capacity: 60,011,642,880 bytes
Device is: Not in smartctl database [for details use: -P showall]
ATA Version is: 6
ATA Standard is: ATA/ATAPI-6 T13 1410D revision 3a
Local Time is: Sat Mar 23 01:55:27 2013 CET
SMART support is: Available - device has SMART capability.
SMART support is: Enabled
SMART Disabled. Use option -s with argument 'on' to enable it.
# strace -x smartctl-5.33 -d ata -a /dev/sda 2>&1 | tail -n 10
write(1, "SMART support is: Available - de"..., 59SMART support is: Available - device has SMART capability.
) = 59
write(1, "SMART support is: Enabled\n", 26SMART support is: Enabled
) = 26
write(1, "\n", 1
) = 1
ioctl(3, 0x31f, 0x7f98a240) = -1 EIO (Input/output error)
write(1, "SMART Disabled. Use option -s wi"..., 63SMART Disabled. Use option -s with argument 'on' to enable it.
) = 63
exit_group(0) = ?
Newer smartctl versions (at least 6.1) just reports an error in such
case and continue:
Error SMART Status command failed: Input/output error
# strace -x smartctl-6.1 -d ata -a /dev/sda 2>&1
...
ioctl(3, 0x31e, 0x7fac798c) = -1 EIO (Input/output error)
write(1, "Error SMART Status command faile"..., 54Error SMART Status command failed: Input/output error
...
On older kernels ioctl() returns 0 instead of -EIO.
Thanks,
Krzysiek
In reply to [1]: I have the same issue. Git bisect took 50+ rebuilds xD
Smartd does not work anymore since 84a9a8cd9 ([libata] Set proper SK
when CK_COND is set.).
I hope I'm not stepping on anyone's toe's by chosing the same title.
I'm not subscribed to this list.
Just wanted to add a 'me2'
[1] http://www.spinics.net/lists/linux-ide/msg45268.html
On Mon, Mar 25, 2013 at 06:26:50PM +0100, Ronald wrote:
> In reply to [1]: I have the same issue. Git bisect took 50+ rebuilds xD
>
> Smartd does not work anymore since 84a9a8cd9 ([libata] Set proper SK
> when CK_COND is set.).
>
> I hope I'm not stepping on anyone's toe's by chosing the same title.
> I'm not subscribed to this list.
>
> Just wanted to add a 'me2'
>
> [1] http://www.spinics.net/lists/linux-ide/msg45268.html
It seems that the SAM_STAT_CHECK_CONDITION is not cleared
causing -EIO, because that patch modified sensebuf and
the check for clearing SAM_STAT_CHECK_CONDITION is no longer valid.
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 318b413..ff44787 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -532,8 +532,8 @@ int ata_cmd_ioctl(struct scsi_device *scsidev, void __user *arg)
struct scsi_sense_hdr sshdr;
scsi_normalize_sense(sensebuf, SCSI_SENSE_BUFFERSIZE,
&sshdr);
- if (sshdr.sense_key == 0 &&
- sshdr.asc == 0 && sshdr.ascq == 0)
+ if (sshdr.sense_key == RECOVERED_ERROR &&
+ sshdr.asc == 0 && sshdr.ascq == 0x1d)
cmd_result &= ~SAM_STAT_CHECK_CONDITION;
}
@@ -618,8 +618,8 @@ int ata_task_ioctl(struct scsi_device *scsidev, void __user *arg)
struct scsi_sense_hdr sshdr;
scsi_normalize_sense(sensebuf, SCSI_SENSE_BUFFERSIZE,
&sshdr);
- if (sshdr.sense_key == 0 &&
- sshdr.asc == 0 && sshdr.ascq == 0)
+ if (sshdr.sense_key == RECOVERED_ERROR &&
+ sshdr.asc == 0 && sshdr.ascq == 0x1d)
cmd_result &= ~SAM_STAT_CHECK_CONDITION;
}
Krzysiek
Ronald [and other],
Sorry for the bug I introduced. I just send a mail with a patch
"[PATCH] [libata] Fix HDIO_DRIVE_CMD ioctl sense data check" that
addresses the problem.
Gwendal.
On Mon, Mar 25, 2013 at 10:26 AM, Ronald <[email protected]> wrote:
> In reply to [1]: I have the same issue. Git bisect took 50+ rebuilds xD
>
> Smartd does not work anymore since 84a9a8cd9 ([libata] Set proper SK
> when CK_COND is set.).
>
> I hope I'm not stepping on anyone's toe's by chosing the same title.
> I'm not subscribed to this list.
>
> Just wanted to add a 'me2'
>
> [1] http://www.spinics.net/lists/linux-ide/msg45268.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ide" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
commit 84a9a8cd9d0aa93c17e5815ab8a9cc4c0a765c63 changed the sense key
used for returning task registers, but HDIO_DRIVE_CMD ioctl was
not changed accordingly.
Tested: check that SMART ENABLE sent using HDIO_DRIVE_CMD returns 0
instead of EIO.
Signed-off-by: Gwendal Grignou <[email protected]>
---
drivers/ata/libata-scsi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 318b413..e05bf4c 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -532,8 +532,8 @@ int ata_cmd_ioctl(struct scsi_device *scsidev, void __user *arg)
struct scsi_sense_hdr sshdr;
scsi_normalize_sense(sensebuf, SCSI_SENSE_BUFFERSIZE,
&sshdr);
- if (sshdr.sense_key == 0 &&
- sshdr.asc == 0 && sshdr.ascq == 0)
+ if (sshdr.sense_key == RECOVERED_ERROR &&
+ sshdr.asc == 0 && sshdr.ascq == 0x1D)
cmd_result &= ~SAM_STAT_CHECK_CONDITION;
}
--
1.8.1.3
On Fri, Mar 29, 2013 at 08:26:41AM -0700, Gwendal Grignou wrote:
> commit 84a9a8cd9d0aa93c17e5815ab8a9cc4c0a765c63 changed the sense key
> used for returning task registers, but HDIO_DRIVE_CMD ioctl was
> not changed accordingly.
>
> Tested: check that SMART ENABLE sent using HDIO_DRIVE_CMD returns 0
> instead of EIO.
>
> Signed-off-by: Gwendal Grignou <[email protected]>
> ---
> drivers/ata/libata-scsi.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 318b413..e05bf4c 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -532,8 +532,8 @@ int ata_cmd_ioctl(struct scsi_device *scsidev, void __user *arg)
> struct scsi_sense_hdr sshdr;
> scsi_normalize_sense(sensebuf, SCSI_SENSE_BUFFERSIZE,
> &sshdr);
> - if (sshdr.sense_key == 0 &&
> - sshdr.asc == 0 && sshdr.ascq == 0)
> + if (sshdr.sense_key == RECOVERED_ERROR &&
> + sshdr.asc == 0 && sshdr.ascq == 0x1D)
> cmd_result &= ~SAM_STAT_CHECK_CONDITION;
> }
>
> --
> 1.8.1.3
I did not test your patch, but I think that you missed the second
test in ata_task_ioctl() and the kernel will still return -EIO
in case of HDIO_DRIVE_TASK (0x31e) ioctl.
See http://marc.info/?l=linux-kernel&m=136438868606981&w=2.
The version I've sent fixes the problem for me.
Krzysiek
Yours work.
On Fri, Mar 29, 2013 at 9:10 AM, Krzysztof Mazur <[email protected]> wrote:
> On Fri, Mar 29, 2013 at 08:26:41AM -0700, Gwendal Grignou wrote:
>> commit 84a9a8cd9d0aa93c17e5815ab8a9cc4c0a765c63 changed the sense key
>> used for returning task registers, but HDIO_DRIVE_CMD ioctl was
>> not changed accordingly.
>>
>> Tested: check that SMART ENABLE sent using HDIO_DRIVE_CMD returns 0
>> instead of EIO.
>>
>> Signed-off-by: Gwendal Grignou <[email protected]>
>> ---
>> drivers/ata/libata-scsi.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
>> index 318b413..e05bf4c 100644
>> --- a/drivers/ata/libata-scsi.c
>> +++ b/drivers/ata/libata-scsi.c
>> @@ -532,8 +532,8 @@ int ata_cmd_ioctl(struct scsi_device *scsidev, void __user *arg)
>> struct scsi_sense_hdr sshdr;
>> scsi_normalize_sense(sensebuf, SCSI_SENSE_BUFFERSIZE,
>> &sshdr);
>> - if (sshdr.sense_key == 0 &&
>> - sshdr.asc == 0 && sshdr.ascq == 0)
>> + if (sshdr.sense_key == RECOVERED_ERROR &&
>> + sshdr.asc == 0 && sshdr.ascq == 0x1D)
>> cmd_result &= ~SAM_STAT_CHECK_CONDITION;
>> }
>>
>> --
>> 1.8.1.3
>
> I did not test your patch, but I think that you missed the second
> test in ata_task_ioctl() and the kernel will still return -EIO
> in case of HDIO_DRIVE_TASK (0x31e) ioctl.
> See http://marc.info/?l=linux-kernel&m=136438868606981&w=2.
> The version I've sent fixes the problem for me.
>
> Krzysiek
commit 84a9a8cd9d0aa93c17e5815ab8a9cc4c0a765c63 changed the sense key
used for returning task registers, but HDIO_DRIVE_CMD ioctl was
not changed accordingly.
Tested: check that SMART ENABLE sent using HDIO_DRIVE_CMD returns 0
instead of EIO.
Change-Id: I84dccd3febb0467a83a39e55ecfdaaa9686332cd
Signed-off-by: Gwendal Grignou <[email protected]>
---
drivers/ata/libata-scsi.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 318b413..5eae74b 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -532,8 +532,8 @@ int ata_cmd_ioctl(struct scsi_device *scsidev, void __user *arg)
struct scsi_sense_hdr sshdr;
scsi_normalize_sense(sensebuf, SCSI_SENSE_BUFFERSIZE,
&sshdr);
- if (sshdr.sense_key == 0 &&
- sshdr.asc == 0 && sshdr.ascq == 0)
+ if (sshdr.sense_key == RECOVERED_ERROR &&
+ sshdr.asc == 0 && sshdr.ascq == 0x1D)
cmd_result &= ~SAM_STAT_CHECK_CONDITION;
}
@@ -618,8 +618,8 @@ int ata_task_ioctl(struct scsi_device *scsidev, void __user *arg)
struct scsi_sense_hdr sshdr;
scsi_normalize_sense(sensebuf, SCSI_SENSE_BUFFERSIZE,
&sshdr);
- if (sshdr.sense_key == 0 &&
- sshdr.asc == 0 && sshdr.ascq == 0)
+ if (sshdr.sense_key == RECOVERED_ERROR &&
+ sshdr.asc == 0 && sshdr.ascq == 0x1D)
cmd_result &= ~SAM_STAT_CHECK_CONDITION;
}
--
1.8.1.3
commit 84a9a8cd9d0aa93c17e5815ab8a9cc4c0a765c63 changed the sense key
used for returning task registers, but HDIO_DRIVE_CMD ioctl was
not changed accordingly.
Tested: check that SMART ENABLE sent using HDIO_DRIVE_CMD returns 0
instead of EIO.
Signed-off-by: Gwendal Grignou <[email protected]>
---
drivers/ata/libata-scsi.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 318b413..5eae74b 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -532,8 +532,8 @@ int ata_cmd_ioctl(struct scsi_device *scsidev, void __user *arg)
struct scsi_sense_hdr sshdr;
scsi_normalize_sense(sensebuf, SCSI_SENSE_BUFFERSIZE,
&sshdr);
- if (sshdr.sense_key == 0 &&
- sshdr.asc == 0 && sshdr.ascq == 0)
+ if (sshdr.sense_key == RECOVERED_ERROR &&
+ sshdr.asc == 0 && sshdr.ascq == 0x1D)
cmd_result &= ~SAM_STAT_CHECK_CONDITION;
}
@@ -618,8 +618,8 @@ int ata_task_ioctl(struct scsi_device *scsidev, void __user *arg)
struct scsi_sense_hdr sshdr;
scsi_normalize_sense(sensebuf, SCSI_SENSE_BUFFERSIZE,
&sshdr);
- if (sshdr.sense_key == 0 &&
- sshdr.asc == 0 && sshdr.ascq == 0)
+ if (sshdr.sense_key == RECOVERED_ERROR &&
+ sshdr.asc == 0 && sshdr.ascq == 0x1D)
cmd_result &= ~SAM_STAT_CHECK_CONDITION;
}
--
1.8.1.3
On Fri, Mar 29, 2013 at 10:12:46AM -0700, Gwendal Grignou wrote:
> commit 84a9a8cd9d0aa93c17e5815ab8a9cc4c0a765c63 changed the sense key
> used for returning task registers, but HDIO_DRIVE_CMD ioctl was
> not changed accordingly.
>
> Tested: check that SMART ENABLE sent using HDIO_DRIVE_CMD returns 0
> instead of EIO.
>
> Signed-off-by: Gwendal Grignou <[email protected]>
Works for me. If you like you can add:
Reported-and-tested-by: Krzysztof Mazur <[email protected]>
Krzysiek
Hello.
On 03/29/2013 08:12 PM, Gwendal Grignou wrote:
> commit 84a9a8cd9d0aa93c17e5815ab8a9cc4c0a765c63
Please, also specify that commit's summary line in parens
(or however you like).
> changed the sense key
> used for returning task registers, but HDIO_DRIVE_CMD ioctl was
> not changed accordingly.
>
> Tested: check that SMART ENABLE sent using HDIO_DRIVE_CMD returns 0
> instead of EIO.
>
> Signed-off-by: Gwendal Grignou <[email protected]>
>
MBR, Sergei
commit 84a9a8 "Set proper SK when CK_COND is set" changed the sense key
used for returning task registers, but HDIO_DRIVE_CMD ioctl was
not changed accordingly.
Tested: check that SMART ENABLE sent using HDIO_DRIVE_CMD returns 0
instead of EIO.
Signed-off-by: Gwendal Grignou <[email protected]>
---
drivers/ata/libata-scsi.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 318b413..5eae74b 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -532,8 +532,8 @@ int ata_cmd_ioctl(struct scsi_device *scsidev, void __user *arg)
struct scsi_sense_hdr sshdr;
scsi_normalize_sense(sensebuf, SCSI_SENSE_BUFFERSIZE,
&sshdr);
- if (sshdr.sense_key == 0 &&
- sshdr.asc == 0 && sshdr.ascq == 0)
+ if (sshdr.sense_key == RECOVERED_ERROR &&
+ sshdr.asc == 0 && sshdr.ascq == 0x1D)
cmd_result &= ~SAM_STAT_CHECK_CONDITION;
}
@@ -618,8 +618,8 @@ int ata_task_ioctl(struct scsi_device *scsidev, void __user *arg)
struct scsi_sense_hdr sshdr;
scsi_normalize_sense(sensebuf, SCSI_SENSE_BUFFERSIZE,
&sshdr);
- if (sshdr.sense_key == 0 &&
- sshdr.asc == 0 && sshdr.ascq == 0)
+ if (sshdr.sense_key == RECOVERED_ERROR &&
+ sshdr.asc == 0 && sshdr.ascq == 0x1D)
cmd_result &= ~SAM_STAT_CHECK_CONDITION;
}
--
1.8.1.3
On 03/27/2013 08:51 AM, Krzysztof Mazur wrote:
> On Mon, Mar 25, 2013 at 06:26:50PM +0100, Ronald wrote:
>> In reply to [1]: I have the same issue. Git bisect took 50+ rebuilds xD
>>
>> Smartd does not work anymore since 84a9a8cd9 ([libata] Set proper SK
>> when CK_COND is set.).
>>
>> I hope I'm not stepping on anyone's toe's by chosing the same title.
>> I'm not subscribed to this list.
>>
>> Just wanted to add a 'me2'
>>
>> [1] http://www.spinics.net/lists/linux-ide/msg45268.html
>
> It seems that the SAM_STAT_CHECK_CONDITION is not cleared
> causing -EIO, because that patch modified sensebuf and
> the check for clearing SAM_STAT_CHECK_CONDITION is no longer valid.
>
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 318b413..ff44787 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -532,8 +532,8 @@ int ata_cmd_ioctl(struct scsi_device *scsidev, void __user *arg)
> struct scsi_sense_hdr sshdr;
> scsi_normalize_sense(sensebuf, SCSI_SENSE_BUFFERSIZE,
> &sshdr);
> - if (sshdr.sense_key == 0 &&
> - sshdr.asc == 0 && sshdr.ascq == 0)
> + if (sshdr.sense_key == RECOVERED_ERROR &&
> + sshdr.asc == 0 && sshdr.ascq == 0x1d)
> cmd_result &= ~SAM_STAT_CHECK_CONDITION;
> }
>
> @@ -618,8 +618,8 @@ int ata_task_ioctl(struct scsi_device *scsidev, void __user *arg)
> struct scsi_sense_hdr sshdr;
> scsi_normalize_sense(sensebuf, SCSI_SENSE_BUFFERSIZE,
> &sshdr);
> - if (sshdr.sense_key == 0 &&
> - sshdr.asc == 0 && sshdr.ascq == 0)
> + if (sshdr.sense_key == RECOVERED_ERROR &&
> + sshdr.asc == 0 && sshdr.ascq == 0x1d)
> cmd_result &= ~SAM_STAT_CHECK_CONDITION;
applied