Errors on USB-storage device are ignored.

PATCH Rogier Wolff 3 participants 4 messages Aug 16, 2007 – Aug 20, 2007

Subsystems (3)

SCSI USB USB mass storage driver

lore

Share & tools

lore

Rogier Wolff

author

2007-08-16 14:58 UTC #0 lore raw

Hi,

I have an usb-storage enclosure that houses a normal desktop harddrive. I have been wondering why disks in that enclosure seemed to be having less errors than when connected to a normal IDE connector.

The reason is: USB-storage is ignoring a hint that something is wrong. Probably my enclosure is also not completely following specs, but Linux is ignoring the hint as well....

On hitting a bad block, the disk reports error. The USB converter then reports "auto-sense-required", and this is carried out. However at this point, my USB enclosure returns all-zeroes. This is considered non-fatal by the kernel.

I'm guessing not many people are testing these things with bad drives. So, I don't know wether or not other USB converters handle this situation more gracefully.

As a patch, I've decided to set the sense key to "vendor specific" (9), and then no "additional sense" (0:0), if, and only if the device didn't return any valid sense info.

The rest of the kernel then correctly interprets the situation as an IO error.

Roger Wolff.

drivers/usb/storage/transport.c +8 -0

@@ -629,6 +629,14 @@
 
 		/* let's clean up right away */
 		memcpy(srb->sense_buffer, us->sensebuf, US_SENSE_SIZE);
+		if (((srb->sense_buffer[2]&0xf) == 0) &&
+		    (srb->sense_buffer[12] == 0) &&
+		    (srb->sense_buffer[13] == 0)) {
+			/* Hmmmmm. The device requested sense, but then
+			   declined to give us more info.... -- REW */ 
+			srb->sense_buffer[2] |= 0x09; /* Vendor specific */
+		}
+
 		srb->resid = old_resid;
 		srb->request_buffer = old_request_buffer;
 		srb->request_bufflen = old_request_bufflen;

Signature · 16 lines

** [email protected] ** http://www.BitWizard.nl/ ** +31-15-2600998 **
**    Delftechpark 26 2628 XH  Delft, The Netherlands. KVK: 27239233    **
*-- BitWizard writes Linux device drivers for any device you may have! --*
Q: It doesn't work. A: Look buddy, doesn't work is an ambiguous statement. 
Does it sit on the couch all day? Is it unemployed? Please be specific! 
Define 'it' and what it isn't doing. --------- Adapted from lxrbot FAQ

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Matthew Dharm

2007-08-20 06:09 UTC #1 lore raw

Off the cuff, this really looks like something for the SCSI layer. I'll bet there are SCSI devices that do something similar...

And, I'm generally reluctant to modify the data to/from a device unless usb-storage *really* has to in order to make it work....

Matt

Quoted 57 lines · Rogier Wolff

Hi,
I have an usb-storage enclosure that houses a normal desktop harddrive. I have been wondering why disks in that enclosure seemed to be having less errors than when connected to a normal IDE connector.
The reason is: USB-storage is ignoring a hint that something is wrong. Probably my enclosure is also not completely following specs, but Linux is ignoring the hint as well....
On hitting a bad block, the disk reports error. The USB converter then reports "auto-sense-required", and this is carried out. However at this point, my USB enclosure returns all-zeroes. This is considered non-fatal by the kernel.
I'm guessing not many people are testing these things with bad drives. So, I don't know wether or not other USB converters handle this situation more gracefully.
As a patch, I've decided to set the sense key to "vendor specific" (9), and then no "additional sense" (0:0), if, and only if the device didn't return any valid sense info.
The rest of the kernel then correctly interprets the situation as an IO error.
Roger Wolff.
drivers/usb/storage/transport.c +8 -0
@@ -629,6 +629,14 @@
 
 		/* let's clean up right away */
 		memcpy(srb->sense_buffer, us->sensebuf, US_SENSE_SIZE);
+		if (((srb->sense_buffer[2]&0xf) == 0) &&
+		    (srb->sense_buffer[12] == 0) &&
+		    (srb->sense_buffer[13] == 0)) {
+			/* Hmmmmm. The device requested sense, but then
+			   declined to give us more info.... -- REW */ 
+			srb->sense_buffer[2] |= 0x09; /* Vendor specific */
+		}
+
 		srb->resid = old_resid;
 		srb->request_buffer = old_request_buffer;
 		srb->request_bufflen = old_request_bufflen;
Signature · 7 lines
-- 
** [email protected] ** http://www.BitWizard.nl/ ** +31-15-2600998 **
**    Delftechpark 26 2628 XH  Delft, The Netherlands. KVK: 27239233    **
*-- BitWizard writes Linux device drivers for any device you may have! --*
Q: It doesn't work. A: Look buddy, doesn't work is an ambiguous statement. 
Does it sit on the couch all day? Is it unemployed? Please be specific! 
Define 'it' and what it isn't doing. --------- Adapted from lxrbot FAQ

Signature · 8 lines

Matthew Dharm                              Home: [email protected] 
Maintainer, Linux USB Mass Storage Driver

G:  Money isn't everything, A.J.
AJ: Who convinced you of that?
G:  The Chief, at my last salary review.
					-- Mike and Greg
User Friendly, 11/3/1998

_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Rogier Wolff

author

2007-08-20 08:37 UTC #2 lore raw

Quoted 5 lines · Matthew Dharm

Off the cuff, this really looks like something for the SCSI layer. I'll bet there are SCSI devices that do something similar...
And, I'm generally reluctant to modify the data to/from a device unless usb-storage *really* has to in order to make it work....

I fully agree, but as the scsi layer has been in existance almost 10 years longer than USB-storage, and that nobody seems to have encountered a device requesting auto-sense, and then reporting no error, I would say this is just a classical: "shitty USB implementation".

The reverse situation is also probelematic. If a device happens to occasionally request an autosense, but perhaps the problem solves itself before the sense request comes through, then it might report "no error". If that is actually true, for some SCSI harddisks, then changing this at the scsi layer will make those harddrives report an IO error every now and then, which is spurious.

That is why I chose to implement it at the lowest possible level, as close as possible to the known non-conformant device.

Yes, I've generalized to: "Maybe there are other USB-IDE converters that may make this mistake", but won't go as far as: "Maybe there are other SCSI devices that make this mistake".

Roger.

Quoted 71 lines

Matt
Quoted 57 lines · Rogier Wolff
Hi,
I have an usb-storage enclosure that houses a normal desktop harddrive. I have been wondering why disks in that enclosure seemed to be having less errors than when connected to a normal IDE connector.
The reason is: USB-storage is ignoring a hint that something is wrong. Probably my enclosure is also not completely following specs, but Linux is ignoring the hint as well....
On hitting a bad block, the disk reports error. The USB converter then reports "auto-sense-required", and this is carried out. However at this point, my USB enclosure returns all-zeroes. This is considered non-fatal by the kernel.
I'm guessing not many people are testing these things with bad drives. So, I don't know wether or not other USB converters handle this situation more gracefully.
As a patch, I've decided to set the sense key to "vendor specific" (9), and then no "additional sense" (0:0), if, and only if the device didn't return any valid sense info.
The rest of the kernel then correctly interprets the situation as an IO error.
Roger Wolff.
drivers/usb/storage/transport.c +8 -0
@@ -629,6 +629,14 @@
 
 		/* let's clean up right away */
 		memcpy(srb->sense_buffer, us->sensebuf, US_SENSE_SIZE);
+		if (((srb->sense_buffer[2]&0xf) == 0) &&
+		    (srb->sense_buffer[12] == 0) &&
+		    (srb->sense_buffer[13] == 0)) {
+			/* Hmmmmm. The device requested sense, but then
+			   declined to give us more info.... -- REW */ 
+			srb->sense_buffer[2] |= 0x09; /* Vendor specific */
+		}
+
 		srb->resid = old_resid;
 		srb->request_buffer = old_request_buffer;
 		srb->request_bufflen = old_request_bufflen;
Signature · 7 lines
-- 
** [email protected] ** http://www.BitWizard.nl/ ** +31-15-2600998 **
**    Delftechpark 26 2628 XH  Delft, The Netherlands. KVK: 27239233    **
*-- BitWizard writes Linux device drivers for any device you may have! --*
Q: It doesn't work. A: Look buddy, doesn't work is an ambiguous statement. 
Does it sit on the couch all day? Is it unemployed? Please be specific! 
Define 'it' and what it isn't doing. --------- Adapted from lxrbot FAQ
Signature · 8 lines
Matthew Dharm                              Home: [email protected] 
Maintainer, Linux USB Mass Storage Driver

G:  Money isn't everything, A.J.
AJ: Who convinced you of that?
G:  The Chief, at my last salary review.
					-- Mike and Greg
User Friendly, 11/3/1998

Signature · 6 lines

** [email protected] ** http://www.BitWizard.nl/ ** +31-15-2600998 **
**    Delftechpark 26 2628 XH  Delft, The Netherlands. KVK: 27239233    **
*-- BitWizard writes Linux device drivers for any device you may have! --*
Q: It doesn't work. A: Look buddy, doesn't work is an ambiguous statement. 
Does it sit on the couch all day? Is it unemployed? Please be specific! 
Define 'it' and what it isn't doing. --------- Adapted from lxrbot FAQ

James Bottomley

2007-08-20 13:03 UTC #3 lore raw

Quoted 11 lines · Rogier Wolff

Quoted 5 lines · Matthew Dharm
Off the cuff, this really looks like something for the SCSI layer. I'll bet there are SCSI devices that do something similar...
And, I'm generally reluctant to modify the data to/from a device unless usb-storage *really* has to in order to make it work....
I fully agree, but as the scsi layer has been in existance almost 10 years longer than USB-storage, and that nobody seems to have encountered a device requesting auto-sense, and then reporting no error, I would say this is just a classical: "shitty USB implementation".

I'm afraid it is. Reporting NO SENSE is actually a legitimate thing for a SCSI device to do. Usually it just means that the problem it was reporting went away in the interim. So, making it mean there's a serious drive problem would put us in volation of the spec. If this can be fixed, it will have to be in USB ... probably tied as specificially as possible to this particular device ... USB storage that correctly follows the RBC spec is allowed to report NO SENSE for the same reason as standard SCSI.

James

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel