Re: [PATCH] usb-storage: scsiglue: Changing the command result
Hi Greg,
There was problem with my email client.
Hope now it is fine.
---
[PATCH 1/3] usb: storage: Proper cmd result assignment
This change replaces DID_ABORT with DID_TIMEOUT as a command result
whenever US_FLIDX_TIMED_OUT bit is set.
This change is made to bring USB storage inline with a recent change:
commit18a4d0a22ed6c54b67af7718c305cd010f09ddf8
[SCSI] Handle disk devices which can not process medium access commands
We have experienced several devices which fail in a fashion we do not
currently handle gracefully in SCSI. After a failure these devices will
respond to the SCSI primary command set (INQUIRY, TEST UNIT READY, etc.)
but any command accessing the storage medium will time out.
As the USB storage was setting command result as aborted rather than
timed out, SCSI layer was not recognizing the above mentioned failure
pattern.
Signed-off-by: Vishal Annapurve
---
drivers/usb/storage/cypress_atacb.c | 1 +
drivers/usb/storage/isd200.c| 2 +-
drivers/usb/storage/transport.c | 8
drivers/usb/storage/usb.c | 10 ++
4 files changed, 12 insertions(+), 9 deletions(-)
---
diff --git a/drivers/usb/storage/cypress_atacb.c
b/drivers/usb/storage/cypress_atacb.c
index 8514a2d..3477ca19 100644
--- a/drivers/usb/storage/cypress_atacb.c
+++ b/drivers/usb/storage/cypress_atacb.c
@@ -168,6 +168,7 @@ static void cypress_atacb_passthrough(struct scsi_cmnd
*srb, struct us_data *us)
*/
if ((srb->result != (DID_ERROR << 16) &&
srb->result != (DID_ABORT << 16)) &&
+ srb->result != (DID_TIME_OUT << 16) &&
save_cmnd[2] & 0x20) {
struct scsi_eh_save ses;
unsigned char regs[8];
diff --git a/drivers/usb/storage/isd200.c b/drivers/usb/storage/isd200.c
index 599d8bf..ffd5d58 100644
--- a/drivers/usb/storage/isd200.c
+++ b/drivers/usb/storage/isd200.c
@@ -703,7 +703,7 @@ static void isd200_invoke_transport( struct us_data *us,
/* abort processing: the bulk-only transport requires a reset
* following an abort */
Handle_Abort:
- srb->result = DID_ABORT << 16;
+ srb->result = DID_TIME_OUT << 16;
/* permit the reset transfer to take place */
clear_bit(US_FLIDX_ABORTING, >dflags);
diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
index 22c7d43..6a90161 100644
--- a/drivers/usb/storage/transport.c
+++ b/drivers/usb/storage/transport.c
@@ -607,8 +607,8 @@ void usb_stor_invoke_transport(struct scsi_cmnd *srb,
struct us_data *us)
* short-circuit all other processing
*/
if (test_bit(US_FLIDX_TIMED_OUT, >dflags)) {
- usb_stor_dbg(us, "-- command was aborted\n");
- srb->result = DID_ABORT << 16;
+ usb_stor_dbg("-- command was aborted because of timeout\n");
+ srb->result = DID_TIME_OUT << 16;
goto Handle_Errors;
}
@@ -717,8 +717,8 @@ Retry_Sense:
scsi_eh_restore_cmnd(srb, );
if (test_bit(US_FLIDX_TIMED_OUT, >dflags)) {
- usb_stor_dbg(us, "-- auto-sense aborted\n");
- srb->result = DID_ABORT << 16;
+ usb_stor_dbg("-- auto-sense aborted due to timeout\n");
+ srb->result = DID_TIME_OUT << 16;
/* If SANE_SENSE caused this problem, disable it */
if (sense_size != US_SENSE_SIZE) {
diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index 5c4fe07..04a68eb 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -325,7 +325,7 @@ static int usb_stor_control_thread(void * __us)
/* has the command timed out *already* ? */
if (test_bit(US_FLIDX_TIMED_OUT, >dflags)) {
- us->srb->result = DID_ABORT << 16;
+ us->srb->result = DID_TIME_OUT << 16;
goto SkipForAbort;
}
@@ -379,7 +379,8 @@ static int usb_stor_control_thread(void * __us)
scsi_lock(host);
/* indicate that the command is done */
- if (us->srb->result != DID_ABORT << 16) {
+ if ((us->srb->result != DID_ABORT << 16) &&
+ (us->srb->result != DID_TIME_OUT << 16)) {
usb_stor_dbg(us, "scsi cmd done, result=0x%x\n",
us->srb->result);
us->srb->scsi_done(us->srb);
@@ -390,8 +391,9 @@ SkipForAbort:
/* If an abort request was received we need to signal that
* the
Re: [PATCH] usb-storage: scsiglue: Changing the command result
Hi Greg,
There was problem with my email client.
Hope now it is fine.
---
[PATCH 1/3] usb: storage: Proper cmd result assignment
This change replaces DID_ABORT with DID_TIMEOUT as a command result
whenever US_FLIDX_TIMED_OUT bit is set.
This change is made to bring USB storage inline with a recent change:
commit18a4d0a22ed6c54b67af7718c305cd010f09ddf8
[SCSI] Handle disk devices which can not process medium access commands
We have experienced several devices which fail in a fashion we do not
currently handle gracefully in SCSI. After a failure these devices will
respond to the SCSI primary command set (INQUIRY, TEST UNIT READY, etc.)
but any command accessing the storage medium will time out.
As the USB storage was setting command result as aborted rather than
timed out, SCSI layer was not recognizing the above mentioned failure
pattern.
Signed-off-by: Vishal Annapurve vannapu...@nvidia.com
---
drivers/usb/storage/cypress_atacb.c | 1 +
drivers/usb/storage/isd200.c| 2 +-
drivers/usb/storage/transport.c | 8
drivers/usb/storage/usb.c | 10 ++
4 files changed, 12 insertions(+), 9 deletions(-)
---
diff --git a/drivers/usb/storage/cypress_atacb.c
b/drivers/usb/storage/cypress_atacb.c
index 8514a2d..3477ca19 100644
--- a/drivers/usb/storage/cypress_atacb.c
+++ b/drivers/usb/storage/cypress_atacb.c
@@ -168,6 +168,7 @@ static void cypress_atacb_passthrough(struct scsi_cmnd
*srb, struct us_data *us)
*/
if ((srb-result != (DID_ERROR 16)
srb-result != (DID_ABORT 16))
+ srb-result != (DID_TIME_OUT 16)
save_cmnd[2] 0x20) {
struct scsi_eh_save ses;
unsigned char regs[8];
diff --git a/drivers/usb/storage/isd200.c b/drivers/usb/storage/isd200.c
index 599d8bf..ffd5d58 100644
--- a/drivers/usb/storage/isd200.c
+++ b/drivers/usb/storage/isd200.c
@@ -703,7 +703,7 @@ static void isd200_invoke_transport( struct us_data *us,
/* abort processing: the bulk-only transport requires a reset
* following an abort */
Handle_Abort:
- srb-result = DID_ABORT 16;
+ srb-result = DID_TIME_OUT 16;
/* permit the reset transfer to take place */
clear_bit(US_FLIDX_ABORTING, us-dflags);
diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
index 22c7d43..6a90161 100644
--- a/drivers/usb/storage/transport.c
+++ b/drivers/usb/storage/transport.c
@@ -607,8 +607,8 @@ void usb_stor_invoke_transport(struct scsi_cmnd *srb,
struct us_data *us)
* short-circuit all other processing
*/
if (test_bit(US_FLIDX_TIMED_OUT, us-dflags)) {
- usb_stor_dbg(us, -- command was aborted\n);
- srb-result = DID_ABORT 16;
+ usb_stor_dbg(-- command was aborted because of timeout\n);
+ srb-result = DID_TIME_OUT 16;
goto Handle_Errors;
}
@@ -717,8 +717,8 @@ Retry_Sense:
scsi_eh_restore_cmnd(srb, ses);
if (test_bit(US_FLIDX_TIMED_OUT, us-dflags)) {
- usb_stor_dbg(us, -- auto-sense aborted\n);
- srb-result = DID_ABORT 16;
+ usb_stor_dbg(-- auto-sense aborted due to timeout\n);
+ srb-result = DID_TIME_OUT 16;
/* If SANE_SENSE caused this problem, disable it */
if (sense_size != US_SENSE_SIZE) {
diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index 5c4fe07..04a68eb 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -325,7 +325,7 @@ static int usb_stor_control_thread(void * __us)
/* has the command timed out *already* ? */
if (test_bit(US_FLIDX_TIMED_OUT, us-dflags)) {
- us-srb-result = DID_ABORT 16;
+ us-srb-result = DID_TIME_OUT 16;
goto SkipForAbort;
}
@@ -379,7 +379,8 @@ static int usb_stor_control_thread(void * __us)
scsi_lock(host);
/* indicate that the command is done */
- if (us-srb-result != DID_ABORT 16) {
+ if ((us-srb-result != DID_ABORT 16)
+ (us-srb-result != DID_TIME_OUT 16)) {
usb_stor_dbg(us, scsi cmd done, result=0x%x\n,
us-srb-result);
us-srb-scsi_done(us-srb);
@@ -390,8 +391,9 @@ SkipForAbort:
/* If an abort request was received we need to signal that
* the abort has finished. The proper test for this is
-* the TIMED_OUT flag, not srb-result == DID_ABORT, because
-* the timeout might have occurred after the command had
+* the TIMED_OUT flag, not srb-result == DID_ABORT or
+* srb-result
Re: [PATCH] usb-storage: scsiglue: Changing the command result
Hi Greg,
Does this look fine? I will send over other patches
once you confirm.
Patch set 1:
-
[PATCH 1/3] usb: storage: Proper cmd result assignment
This change replaces DID_ABORT with DID_TIMEOUT as a command result
whenever US_FLIDX_TIMED_OUT bit is set.
This change is made to bring USB storage inline with a recent change:
commit18a4d0a22ed6c54b67af7718c305cd010f09ddf8
[SCSI] Handle disk devices which can not process medium access commands
We have experienced several devices which fail in a fashion we do not
currently handle gracefully in SCSI. After a failure these devices will
respond to the SCSI primary command set (INQUIRY, TEST UNIT READY, etc.)
but any command accessing the storage medium will time out.
As the USB storage was setting command result as aborted rather than
timed out, SCSI layer was not recognizing the above mentioned failure
pattern.
Signed-off-by: Vishal Annapurve
---
drivers/usb/storage/cypress_atacb.c | 1 +
drivers/usb/storage/isd200.c| 2 +-
drivers/usb/storage/transport.c | 8
drivers/usb/storage/usb.c | 10 ++
4 files changed, 12 insertions(+), 9 deletions(-)
---
diff --git a/drivers/usb/storage/cypress_atacb.c
b/drivers/usb/storage/cypress_atacb.c
index 8514a2d..3477ca19 100644
--- a/drivers/usb/storage/cypress_atacb.c
+++ b/drivers/usb/storage/cypress_atacb.c
@@ -168,6 +168,7 @@ static void cypress_atacb_passthrough(struct
scsi_cmnd *srb, struct us_data *us)
*/
if ((srb->result != (DID_ERROR << 16) &&
srb->result != (DID_ABORT << 16)) &&
+srb->result != (DID_TIME_OUT << 16) &&
save_cmnd[2] & 0x20) {
struct scsi_eh_save ses;
unsigned char regs[8];
diff --git a/drivers/usb/storage/isd200.c b/drivers/usb/storage/isd200.c
index 599d8bf..ffd5d58 100644
--- a/drivers/usb/storage/isd200.c
+++ b/drivers/usb/storage/isd200.c
@@ -703,7 +703,7 @@ static void isd200_invoke_transport( struct us_data *us,
/* abort processing: the bulk-only transport requires a reset
* following an abort */
Handle_Abort:
-srb->result = DID_ABORT << 16;
+srb->result = DID_TIME_OUT << 16;
/* permit the reset transfer to take place */
clear_bit(US_FLIDX_ABORTING, >dflags);
diff --git a/drivers/usb/storage/transport.c
b/drivers/usb/storage/transport.c
index 22c7d43..6a90161 100644
--- a/drivers/usb/storage/transport.c
+++ b/drivers/usb/storage/transport.c
@@ -607,8 +607,8 @@ void usb_stor_invoke_transport(struct scsi_cmnd
*srb, struct us_data *us)
* short-circuit all other processing
*/
if (test_bit(US_FLIDX_TIMED_OUT, >dflags)) {
-usb_stor_dbg(us, "-- command was aborted\n");
-srb->result = DID_ABORT << 16;
+usb_stor_dbg("-- command was aborted because of timeout\n");
+srb->result = DID_TIME_OUT << 16;
goto Handle_Errors;
}
@@ -717,8 +717,8 @@ Retry_Sense:
scsi_eh_restore_cmnd(srb, );
if (test_bit(US_FLIDX_TIMED_OUT, >dflags)) {
-usb_stor_dbg(us, "-- auto-sense aborted\n");
-srb->result = DID_ABORT << 16;
+usb_stor_dbg("-- auto-sense aborted due to timeout\n");
+srb->result = DID_TIME_OUT << 16;
/* If SANE_SENSE caused this problem, disable it */
if (sense_size != US_SENSE_SIZE) {
diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index 5c4fe07..04a68eb 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -325,7 +325,7 @@ static int usb_stor_control_thread(void * __us)
/* has the command timed out *already* ? */
if (test_bit(US_FLIDX_TIMED_OUT, >dflags)) {
-us->srb->result = DID_ABORT << 16;
+us->srb->result = DID_TIME_OUT << 16;
goto SkipForAbort;
}
@@ -379,7 +379,8 @@ static int usb_stor_control_thread(void * __us)
scsi_lock(host);
/* indicate that the command is done */
-if (us->srb->result != DID_ABORT << 16) {
+if ((us->srb->result != DID_ABORT << 16) &&
+(us->srb->result != DID_TIME_OUT << 16)) {
usb_stor_dbg(us, "scsi cmd done, result=0x%x\n",
us->srb->result);
us->srb->scsi_done(us->srb);
@@ -390,8 +391,9 @@ SkipForAbort:
/* If an abort request was received we need to signal that
* the abort has finished. The proper test for this is
- * the TIMED_OUT flag, not srb->result == DID_ABORT, because
- * the timeout might have occurred after the command had
+ * the TIMED_OUT flag, not srb->result == DID_ABORT or
+ * srb->result == DID_TIMEOUT , because the timeout mig
Re: [PATCH] usb-storage: scsiglue: Changing the command result
Hi Greg, Attached are the patches. Regards, Vishal On Monday 09 December 2013 07:29 AM, Greg KH wrote: On Sat, Nov 16, 2013 at 12:23:50PM +0530, Vishal Annapurve wrote: Hi, Here are the updated patches: Can you please resend these in a format which I can apply them in without having to hand-edit all three of them? thanks, greg k-h 01-PATCH-usb-storage-Proper-cmd-result-assignment.patch.gz Description: GNU Zip compressed data 02-keucr-Proper-cmd-result-assignment.patch.gz Description: GNU Zip compressed data 03-usb-storage-uas-Proper-cmd-result-assignment.patch.gz Description: GNU Zip compressed data
Re: [PATCH] usb-storage: scsiglue: Changing the command result
Hi Greg, Attached are the patches. Regards, Vishal On Monday 09 December 2013 07:29 AM, Greg KH wrote: On Sat, Nov 16, 2013 at 12:23:50PM +0530, Vishal Annapurve wrote: Hi, Here are the updated patches: Can you please resend these in a format which I can apply them in without having to hand-edit all three of them? thanks, greg k-h 01-PATCH-usb-storage-Proper-cmd-result-assignment.patch.gz Description: GNU Zip compressed data 02-keucr-Proper-cmd-result-assignment.patch.gz Description: GNU Zip compressed data 03-usb-storage-uas-Proper-cmd-result-assignment.patch.gz Description: GNU Zip compressed data
Re: [PATCH] usb-storage: scsiglue: Changing the command result
Hi Greg,
Does this look fine? I will send over other patches
once you confirm.
Patch set 1:
-
[PATCH 1/3] usb: storage: Proper cmd result assignment
This change replaces DID_ABORT with DID_TIMEOUT as a command result
whenever US_FLIDX_TIMED_OUT bit is set.
This change is made to bring USB storage inline with a recent change:
commit18a4d0a22ed6c54b67af7718c305cd010f09ddf8
[SCSI] Handle disk devices which can not process medium access commands
We have experienced several devices which fail in a fashion we do not
currently handle gracefully in SCSI. After a failure these devices will
respond to the SCSI primary command set (INQUIRY, TEST UNIT READY, etc.)
but any command accessing the storage medium will time out.
As the USB storage was setting command result as aborted rather than
timed out, SCSI layer was not recognizing the above mentioned failure
pattern.
Signed-off-by: Vishal Annapurve vannapu...@nvidia.com
---
drivers/usb/storage/cypress_atacb.c | 1 +
drivers/usb/storage/isd200.c| 2 +-
drivers/usb/storage/transport.c | 8
drivers/usb/storage/usb.c | 10 ++
4 files changed, 12 insertions(+), 9 deletions(-)
---
diff --git a/drivers/usb/storage/cypress_atacb.c
b/drivers/usb/storage/cypress_atacb.c
index 8514a2d..3477ca19 100644
--- a/drivers/usb/storage/cypress_atacb.c
+++ b/drivers/usb/storage/cypress_atacb.c
@@ -168,6 +168,7 @@ static void cypress_atacb_passthrough(struct
scsi_cmnd *srb, struct us_data *us)
*/
if ((srb-result != (DID_ERROR 16)
srb-result != (DID_ABORT 16))
+srb-result != (DID_TIME_OUT 16)
save_cmnd[2] 0x20) {
struct scsi_eh_save ses;
unsigned char regs[8];
diff --git a/drivers/usb/storage/isd200.c b/drivers/usb/storage/isd200.c
index 599d8bf..ffd5d58 100644
--- a/drivers/usb/storage/isd200.c
+++ b/drivers/usb/storage/isd200.c
@@ -703,7 +703,7 @@ static void isd200_invoke_transport( struct us_data *us,
/* abort processing: the bulk-only transport requires a reset
* following an abort */
Handle_Abort:
-srb-result = DID_ABORT 16;
+srb-result = DID_TIME_OUT 16;
/* permit the reset transfer to take place */
clear_bit(US_FLIDX_ABORTING, us-dflags);
diff --git a/drivers/usb/storage/transport.c
b/drivers/usb/storage/transport.c
index 22c7d43..6a90161 100644
--- a/drivers/usb/storage/transport.c
+++ b/drivers/usb/storage/transport.c
@@ -607,8 +607,8 @@ void usb_stor_invoke_transport(struct scsi_cmnd
*srb, struct us_data *us)
* short-circuit all other processing
*/
if (test_bit(US_FLIDX_TIMED_OUT, us-dflags)) {
-usb_stor_dbg(us, -- command was aborted\n);
-srb-result = DID_ABORT 16;
+usb_stor_dbg(-- command was aborted because of timeout\n);
+srb-result = DID_TIME_OUT 16;
goto Handle_Errors;
}
@@ -717,8 +717,8 @@ Retry_Sense:
scsi_eh_restore_cmnd(srb, ses);
if (test_bit(US_FLIDX_TIMED_OUT, us-dflags)) {
-usb_stor_dbg(us, -- auto-sense aborted\n);
-srb-result = DID_ABORT 16;
+usb_stor_dbg(-- auto-sense aborted due to timeout\n);
+srb-result = DID_TIME_OUT 16;
/* If SANE_SENSE caused this problem, disable it */
if (sense_size != US_SENSE_SIZE) {
diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index 5c4fe07..04a68eb 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -325,7 +325,7 @@ static int usb_stor_control_thread(void * __us)
/* has the command timed out *already* ? */
if (test_bit(US_FLIDX_TIMED_OUT, us-dflags)) {
-us-srb-result = DID_ABORT 16;
+us-srb-result = DID_TIME_OUT 16;
goto SkipForAbort;
}
@@ -379,7 +379,8 @@ static int usb_stor_control_thread(void * __us)
scsi_lock(host);
/* indicate that the command is done */
-if (us-srb-result != DID_ABORT 16) {
+if ((us-srb-result != DID_ABORT 16)
+(us-srb-result != DID_TIME_OUT 16)) {
usb_stor_dbg(us, scsi cmd done, result=0x%x\n,
us-srb-result);
us-srb-scsi_done(us-srb);
@@ -390,8 +391,9 @@ SkipForAbort:
/* If an abort request was received we need to signal that
* the abort has finished. The proper test for this is
- * the TIMED_OUT flag, not srb-result == DID_ABORT, because
- * the timeout might have occurred after the command had
+ * the TIMED_OUT flag, not srb-result == DID_ABORT or
+ * srb-result == DID_TIMEOUT , because the timeout might
+ * have occurred after the command had
* already completed with a different result code. */
if (test_bit(US_FLIDX_TIMED_OUT, us-dflags)) {
complete((us-notify));
---
Regards,
Vishal
On Monday 09 December 2013 04:53 PM, Greg KH wrote:
On Mon, Dec 09, 2013
RE: [PATCH] usb-storage: scsiglue: Changing the command result
Hi,
[PATCH 3/3] usb: storage: uas: Proper cmd result assignment
This change replaces DID_ABORT with DID_TIMEOUT as a command result
whenever US_FLIDX_TIMED_OUT bit is set.
This change is made to bring USB storage inline with a recent change:
commit18a4d0a22ed6c54b67af7718c305cd010f09ddf8
[SCSI] Handle disk devices which can not process medium access commands
We have experienced several devices which fail in a fashion we do not
currently handle gracefully in SCSI. After a failure these devices will
respond to the SCSI primary command set (INQUIRY, TEST UNIT READY, etc.)
but any command accessing the storage medium will time out.
As the USB storage was setting command result as aborted rather than
timed out, SCSI layer was not recognizing the above mentioned failure
pattern.
Signed-off-by: Vishal Annapurve
---
drivers/usb/storage/uas.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index d966b59..3a4a44e 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -271,7 +271,10 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const
char *caller)
usb_free_urb(cmdinfo->data_out_urb);
if (cmdinfo->state & COMMAND_ABORTED) {
scmd_printk(KERN_INFO, cmnd, "abort completed\n");
- cmnd->result = DID_ABORT << 16;
+ /* Better to check for US_FLIDX_TIMED_OUT
+* bit settings.
+*/
+ cmnd->result = DID_TIME_OUT << 16;
}
cmnd->scsi_done(cmnd);
return 0;
--
1.8.4.2
Regards,
Vishal
-Original Message-
From: Vishal Annapurve
Sent: Saturday, November 16, 2013 12:25 PM
To: 'Alan Stern'
Cc: 'Ming Lei'; 'Linux Kernel Mailing List'; 'linux-usb'
Subject: RE: [PATCH] usb-storage: scsiglue: Changing the command result
Hi,
[PATCH 2/3] keucr: Proper cmd result assignment
This change replaces DID_ABORT with DID_TIMEOUT as a command result whenever
US_FLIDX_TIMED_OUT bit is set.
This change is made to bring USB storage inline with a recent change:
commit18a4d0a22ed6c54b67af7718c305cd010f09ddf8
[SCSI] Handle disk devices which can not process medium access commands We have
experienced several devices which fail in a fashion we do not currently handle
gracefully in SCSI. After a failure these devices will respond to the SCSI
primary command set (INQUIRY, TEST UNIT READY, etc.) but any command accessing
the storage medium will time out.
As the USB storage was setting command result as aborted rather than timed out,
SCSI layer was not recognizing the above mentioned failure pattern.
Signed-off-by: Vishal Annapurve
---
drivers/staging/keucr/transport.c | 6 +++---
drivers/staging/keucr/usb.c | 5 +++--
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/keucr/transport.c
b/drivers/staging/keucr/transport.c
index aeb2186..6f61c20 100644
--- a/drivers/staging/keucr/transport.c
+++ b/drivers/staging/keucr/transport.c
@@ -353,7 +353,7 @@ void usb_stor_invoke_transport(struct scsi_cmnd *srb,
struct us_data *us)
we need to short-circuit all other processing */
if (test_bit(US_FLIDX_TIMED_OUT, >dflags)) {
/* pr_info("-- command was aborted\n"); */
- srb->result = DID_ABORT << 16;
+ srb->result = DID_TIME_OUT << 16;
goto Handle_Errors;
}
@@ -412,7 +412,7 @@ void usb_stor_invoke_transport(struct scsi_cmnd *srb,
struct us_data *us)
if (test_bit(US_FLIDX_TIMED_OUT, >dflags)) {
/* pr_info("-- auto-sense aborted\n"); */
- srb->result = DID_ABORT << 16;
+ srb->result = DID_TIME_OUT << 16;
goto Handle_Errors;
}
if (temp_result != USB_STOR_TRANSPORT_GOOD) { @@ -488,7 +488,7
@@ void ENE_stor_invoke_transport(struct scsi_cmnd *srb, struct us_data *us)
we need to short-circuit all other processing */
if (test_bit(US_FLIDX_TIMED_OUT, >dflags)) {
/* pr_info("-- command was aborted\n"); */
- srb->result = DID_ABORT << 16;
+ srb->result = DID_TIME_OUT << 16;
goto Handle_Errors;
}
diff --git a/drivers/staging/keucr/usb.c b/drivers/staging/keucr/usb.c index
a84ee63..26ec64c 100644
--- a/drivers/staging/keucr/usb.c
+++ b/drivers/staging/keucr/usb.c
@@ -181,7 +181,7 @@ static int usb_stor_control_thread(void *__us)
/* has the command timed out *already* ? */
if (test_bit(US_FLIDX_TIMED_OUT, >dflags)) {
- us->srb->result = DID_ABORT << 16;
+ us->srb->result = DID_TIME_OUT << 16;
RE: [PATCH] usb-storage: scsiglue: Changing the command result
Hi,
[PATCH 2/3] keucr: Proper cmd result assignment
This change replaces DID_ABORT with DID_TIMEOUT as a command result
whenever US_FLIDX_TIMED_OUT bit is set.
This change is made to bring USB storage inline with a recent change:
commit18a4d0a22ed6c54b67af7718c305cd010f09ddf8
[SCSI] Handle disk devices which can not process medium access commands
We have experienced several devices which fail in a fashion we do not
currently handle gracefully in SCSI. After a failure these devices will
respond to the SCSI primary command set (INQUIRY, TEST UNIT READY, etc.)
but any command accessing the storage medium will time out.
As the USB storage was setting command result as aborted rather than
timed out, SCSI layer was not recognizing the above mentioned failure
pattern.
Signed-off-by: Vishal Annapurve
---
drivers/staging/keucr/transport.c | 6 +++---
drivers/staging/keucr/usb.c | 5 +++--
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/keucr/transport.c
b/drivers/staging/keucr/transport.c
index aeb2186..6f61c20 100644
--- a/drivers/staging/keucr/transport.c
+++ b/drivers/staging/keucr/transport.c
@@ -353,7 +353,7 @@ void usb_stor_invoke_transport(struct scsi_cmnd *srb,
struct us_data *us)
we need to short-circuit all other processing */
if (test_bit(US_FLIDX_TIMED_OUT, >dflags)) {
/* pr_info("-- command was aborted\n"); */
- srb->result = DID_ABORT << 16;
+ srb->result = DID_TIME_OUT << 16;
goto Handle_Errors;
}
@@ -412,7 +412,7 @@ void usb_stor_invoke_transport(struct scsi_cmnd *srb,
struct us_data *us)
if (test_bit(US_FLIDX_TIMED_OUT, >dflags)) {
/* pr_info("-- auto-sense aborted\n"); */
- srb->result = DID_ABORT << 16;
+ srb->result = DID_TIME_OUT << 16;
goto Handle_Errors;
}
if (temp_result != USB_STOR_TRANSPORT_GOOD) {
@@ -488,7 +488,7 @@ void ENE_stor_invoke_transport(struct scsi_cmnd *srb,
struct us_data *us)
we need to short-circuit all other processing */
if (test_bit(US_FLIDX_TIMED_OUT, >dflags)) {
/* pr_info("-- command was aborted\n"); */
- srb->result = DID_ABORT << 16;
+ srb->result = DID_TIME_OUT << 16;
goto Handle_Errors;
}
diff --git a/drivers/staging/keucr/usb.c b/drivers/staging/keucr/usb.c
index a84ee63..26ec64c 100644
--- a/drivers/staging/keucr/usb.c
+++ b/drivers/staging/keucr/usb.c
@@ -181,7 +181,7 @@ static int usb_stor_control_thread(void *__us)
/* has the command timed out *already* ? */
if (test_bit(US_FLIDX_TIMED_OUT, >dflags)) {
- us->srb->result = DID_ABORT << 16;
+ us->srb->result = DID_TIME_OUT << 16;
goto SkipForAbort;
}
@@ -209,7 +209,8 @@ static int usb_stor_control_thread(void *__us)
scsi_lock(host);
/* indicate that the command is done */
- if (us->srb->result != DID_ABORT << 16) {
+ if ((us->srb->result != DID_ABORT << 16) &&
+ (us->srb->result != DID_TIME_OUT << 16)) {
us->srb->scsi_done(us->srb);
} else {
SkipForAbort:
--
1.8.4.2
Regards,
Vishal
-Original Message-
From: Vishal Annapurve
Sent: Saturday, November 16, 2013 12:24 PM
To: 'Alan Stern'
Cc: Ming Lei; Linux Kernel Mailing List; linux-usb
Subject: RE: [PATCH] usb-storage: scsiglue: Changing the command result
Hi,
Here are the updated patches:
[PATCH 1/3] usb: storage: Proper cmd result assignment
This change replaces DID_ABORT with DID_TIMEOUT as a command result whenever
US_FLIDX_TIMED_OUT bit is set.
This change is made to bring USB storage inline with a recent change:
commit18a4d0a22ed6c54b67af7718c305cd010f09ddf8
[SCSI] Handle disk devices which can not process medium access commands We have
experienced several devices which fail in a fashion we do not currently handle
gracefully in SCSI. After a failure these devices will respond to the SCSI
primary command set (INQUIRY, TEST UNIT READY, etc.) but any command accessing
the storage medium will time out.
As the USB storage was setting command result as aborted rather than timed out,
SCSI layer was not recognizing the above mentioned failure pattern.
Signed-off-by: Vishal Annapurve
---
drivers/usb/storage/cypress_atacb.c | 1 +
drivers/usb/storage/isd200.c| 2 +-
drivers/usb/storage/transport.c | 8
drivers/usb/storage/usb.c | 10 ++
4 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/
RE: [PATCH] usb-storage: scsiglue: Changing the command result
Hi,
Here are the updated patches:
[PATCH 1/3] usb: storage: Proper cmd result assignment
This change replaces DID_ABORT with DID_TIMEOUT as a command result
whenever US_FLIDX_TIMED_OUT bit is set.
This change is made to bring USB storage inline with a recent change:
commit18a4d0a22ed6c54b67af7718c305cd010f09ddf8
[SCSI] Handle disk devices which can not process medium access commands
We have experienced several devices which fail in a fashion we do not
currently handle gracefully in SCSI. After a failure these devices will
respond to the SCSI primary command set (INQUIRY, TEST UNIT READY, etc.)
but any command accessing the storage medium will time out.
As the USB storage was setting command result as aborted rather than
timed out, SCSI layer was not recognizing the above mentioned failure
pattern.
Signed-off-by: Vishal Annapurve
---
drivers/usb/storage/cypress_atacb.c | 1 +
drivers/usb/storage/isd200.c| 2 +-
drivers/usb/storage/transport.c | 8
drivers/usb/storage/usb.c | 10 ++
4 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/storage/cypress_atacb.c
b/drivers/usb/storage/cypress_atacb.c
index 8514a2d..3477ca19 100644
--- a/drivers/usb/storage/cypress_atacb.c
+++ b/drivers/usb/storage/cypress_atacb.c
@@ -168,6 +168,7 @@ static void cypress_atacb_passthrough(struct scsi_cmnd
*srb, struct us_data *us)
*/
if ((srb->result != (DID_ERROR << 16) &&
srb->result != (DID_ABORT << 16)) &&
+ srb->result != (DID_TIME_OUT << 16) &&
save_cmnd[2] & 0x20) {
struct scsi_eh_save ses;
unsigned char regs[8];
diff --git a/drivers/usb/storage/isd200.c b/drivers/usb/storage/isd200.c
index 599d8bf..ffd5d58 100644
--- a/drivers/usb/storage/isd200.c
+++ b/drivers/usb/storage/isd200.c
@@ -703,7 +703,7 @@ static void isd200_invoke_transport( struct us_data *us,
/* abort processing: the bulk-only transport requires a reset
* following an abort */
Handle_Abort:
- srb->result = DID_ABORT << 16;
+ srb->result = DID_TIME_OUT << 16;
/* permit the reset transfer to take place */
clear_bit(US_FLIDX_ABORTING, >dflags);
diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
index 22c7d43..6a90161 100644
--- a/drivers/usb/storage/transport.c
+++ b/drivers/usb/storage/transport.c
@@ -607,8 +607,8 @@ void usb_stor_invoke_transport(struct scsi_cmnd *srb,
struct us_data *us)
* short-circuit all other processing
*/
if (test_bit(US_FLIDX_TIMED_OUT, >dflags)) {
- usb_stor_dbg(us, "-- command was aborted\n");
- srb->result = DID_ABORT << 16;
+ usb_stor_dbg("-- command was aborted because of timeout\n");
+ srb->result = DID_TIME_OUT << 16;
goto Handle_Errors;
}
@@ -717,8 +717,8 @@ Retry_Sense:
scsi_eh_restore_cmnd(srb, );
if (test_bit(US_FLIDX_TIMED_OUT, >dflags)) {
- usb_stor_dbg(us, "-- auto-sense aborted\n");
- srb->result = DID_ABORT << 16;
+ usb_stor_dbg("-- auto-sense aborted due to timeout\n");
+ srb->result = DID_TIME_OUT << 16;
/* If SANE_SENSE caused this problem, disable it */
if (sense_size != US_SENSE_SIZE) {
diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index 5c4fe07..04a68eb 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -325,7 +325,7 @@ static int usb_stor_control_thread(void * __us)
/* has the command timed out *already* ? */
if (test_bit(US_FLIDX_TIMED_OUT, >dflags)) {
- us->srb->result = DID_ABORT << 16;
+ us->srb->result = DID_TIME_OUT << 16;
goto SkipForAbort;
}
@@ -379,7 +379,8 @@ static int usb_stor_control_thread(void * __us)
scsi_lock(host);
/* indicate that the command is done */
- if (us->srb->result != DID_ABORT << 16) {
+ if ((us->srb->result != DID_ABORT << 16) &&
+ (us->srb->result != DID_TIME_OUT << 16)) {
usb_stor_dbg(us, "scsi cmd done, result=0x%x\n",
us->srb->result);
us->srb->scsi_done(us->srb);
@@ -390,8 +391,9 @@ SkipForAbort:
/* If an abort request was received we need to signal that
* the abort has fi
RE: [PATCH] usb-storage: scsiglue: Changing the command result
Hi,
Here are the updated patches:
[PATCH 1/3] usb: storage: Proper cmd result assignment
This change replaces DID_ABORT with DID_TIMEOUT as a command result
whenever US_FLIDX_TIMED_OUT bit is set.
This change is made to bring USB storage inline with a recent change:
commit18a4d0a22ed6c54b67af7718c305cd010f09ddf8
[SCSI] Handle disk devices which can not process medium access commands
We have experienced several devices which fail in a fashion we do not
currently handle gracefully in SCSI. After a failure these devices will
respond to the SCSI primary command set (INQUIRY, TEST UNIT READY, etc.)
but any command accessing the storage medium will time out.
As the USB storage was setting command result as aborted rather than
timed out, SCSI layer was not recognizing the above mentioned failure
pattern.
Signed-off-by: Vishal Annapurve vannapu...@nvidia.com
---
drivers/usb/storage/cypress_atacb.c | 1 +
drivers/usb/storage/isd200.c| 2 +-
drivers/usb/storage/transport.c | 8
drivers/usb/storage/usb.c | 10 ++
4 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/storage/cypress_atacb.c
b/drivers/usb/storage/cypress_atacb.c
index 8514a2d..3477ca19 100644
--- a/drivers/usb/storage/cypress_atacb.c
+++ b/drivers/usb/storage/cypress_atacb.c
@@ -168,6 +168,7 @@ static void cypress_atacb_passthrough(struct scsi_cmnd
*srb, struct us_data *us)
*/
if ((srb-result != (DID_ERROR 16)
srb-result != (DID_ABORT 16))
+ srb-result != (DID_TIME_OUT 16)
save_cmnd[2] 0x20) {
struct scsi_eh_save ses;
unsigned char regs[8];
diff --git a/drivers/usb/storage/isd200.c b/drivers/usb/storage/isd200.c
index 599d8bf..ffd5d58 100644
--- a/drivers/usb/storage/isd200.c
+++ b/drivers/usb/storage/isd200.c
@@ -703,7 +703,7 @@ static void isd200_invoke_transport( struct us_data *us,
/* abort processing: the bulk-only transport requires a reset
* following an abort */
Handle_Abort:
- srb-result = DID_ABORT 16;
+ srb-result = DID_TIME_OUT 16;
/* permit the reset transfer to take place */
clear_bit(US_FLIDX_ABORTING, us-dflags);
diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
index 22c7d43..6a90161 100644
--- a/drivers/usb/storage/transport.c
+++ b/drivers/usb/storage/transport.c
@@ -607,8 +607,8 @@ void usb_stor_invoke_transport(struct scsi_cmnd *srb,
struct us_data *us)
* short-circuit all other processing
*/
if (test_bit(US_FLIDX_TIMED_OUT, us-dflags)) {
- usb_stor_dbg(us, -- command was aborted\n);
- srb-result = DID_ABORT 16;
+ usb_stor_dbg(-- command was aborted because of timeout\n);
+ srb-result = DID_TIME_OUT 16;
goto Handle_Errors;
}
@@ -717,8 +717,8 @@ Retry_Sense:
scsi_eh_restore_cmnd(srb, ses);
if (test_bit(US_FLIDX_TIMED_OUT, us-dflags)) {
- usb_stor_dbg(us, -- auto-sense aborted\n);
- srb-result = DID_ABORT 16;
+ usb_stor_dbg(-- auto-sense aborted due to timeout\n);
+ srb-result = DID_TIME_OUT 16;
/* If SANE_SENSE caused this problem, disable it */
if (sense_size != US_SENSE_SIZE) {
diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index 5c4fe07..04a68eb 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -325,7 +325,7 @@ static int usb_stor_control_thread(void * __us)
/* has the command timed out *already* ? */
if (test_bit(US_FLIDX_TIMED_OUT, us-dflags)) {
- us-srb-result = DID_ABORT 16;
+ us-srb-result = DID_TIME_OUT 16;
goto SkipForAbort;
}
@@ -379,7 +379,8 @@ static int usb_stor_control_thread(void * __us)
scsi_lock(host);
/* indicate that the command is done */
- if (us-srb-result != DID_ABORT 16) {
+ if ((us-srb-result != DID_ABORT 16)
+ (us-srb-result != DID_TIME_OUT 16)) {
usb_stor_dbg(us, scsi cmd done, result=0x%x\n,
us-srb-result);
us-srb-scsi_done(us-srb);
@@ -390,8 +391,9 @@ SkipForAbort:
/* If an abort request was received we need to signal that
* the abort has finished. The proper test for this is
-* the TIMED_OUT flag, not srb-result == DID_ABORT, because
-* the timeout might have occurred after the command had
+* the TIMED_OUT flag, not srb-result == DID_ABORT or
+* srb-result == DID_TIMEOUT , because
RE: [PATCH] usb-storage: scsiglue: Changing the command result
Hi,
[PATCH 2/3] keucr: Proper cmd result assignment
This change replaces DID_ABORT with DID_TIMEOUT as a command result
whenever US_FLIDX_TIMED_OUT bit is set.
This change is made to bring USB storage inline with a recent change:
commit18a4d0a22ed6c54b67af7718c305cd010f09ddf8
[SCSI] Handle disk devices which can not process medium access commands
We have experienced several devices which fail in a fashion we do not
currently handle gracefully in SCSI. After a failure these devices will
respond to the SCSI primary command set (INQUIRY, TEST UNIT READY, etc.)
but any command accessing the storage medium will time out.
As the USB storage was setting command result as aborted rather than
timed out, SCSI layer was not recognizing the above mentioned failure
pattern.
Signed-off-by: Vishal Annapurve vannapu...@nvidia.com
---
drivers/staging/keucr/transport.c | 6 +++---
drivers/staging/keucr/usb.c | 5 +++--
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/keucr/transport.c
b/drivers/staging/keucr/transport.c
index aeb2186..6f61c20 100644
--- a/drivers/staging/keucr/transport.c
+++ b/drivers/staging/keucr/transport.c
@@ -353,7 +353,7 @@ void usb_stor_invoke_transport(struct scsi_cmnd *srb,
struct us_data *us)
we need to short-circuit all other processing */
if (test_bit(US_FLIDX_TIMED_OUT, us-dflags)) {
/* pr_info(-- command was aborted\n); */
- srb-result = DID_ABORT 16;
+ srb-result = DID_TIME_OUT 16;
goto Handle_Errors;
}
@@ -412,7 +412,7 @@ void usb_stor_invoke_transport(struct scsi_cmnd *srb,
struct us_data *us)
if (test_bit(US_FLIDX_TIMED_OUT, us-dflags)) {
/* pr_info(-- auto-sense aborted\n); */
- srb-result = DID_ABORT 16;
+ srb-result = DID_TIME_OUT 16;
goto Handle_Errors;
}
if (temp_result != USB_STOR_TRANSPORT_GOOD) {
@@ -488,7 +488,7 @@ void ENE_stor_invoke_transport(struct scsi_cmnd *srb,
struct us_data *us)
we need to short-circuit all other processing */
if (test_bit(US_FLIDX_TIMED_OUT, us-dflags)) {
/* pr_info(-- command was aborted\n); */
- srb-result = DID_ABORT 16;
+ srb-result = DID_TIME_OUT 16;
goto Handle_Errors;
}
diff --git a/drivers/staging/keucr/usb.c b/drivers/staging/keucr/usb.c
index a84ee63..26ec64c 100644
--- a/drivers/staging/keucr/usb.c
+++ b/drivers/staging/keucr/usb.c
@@ -181,7 +181,7 @@ static int usb_stor_control_thread(void *__us)
/* has the command timed out *already* ? */
if (test_bit(US_FLIDX_TIMED_OUT, us-dflags)) {
- us-srb-result = DID_ABORT 16;
+ us-srb-result = DID_TIME_OUT 16;
goto SkipForAbort;
}
@@ -209,7 +209,8 @@ static int usb_stor_control_thread(void *__us)
scsi_lock(host);
/* indicate that the command is done */
- if (us-srb-result != DID_ABORT 16) {
+ if ((us-srb-result != DID_ABORT 16)
+ (us-srb-result != DID_TIME_OUT 16)) {
us-srb-scsi_done(us-srb);
} else {
SkipForAbort:
--
1.8.4.2
Regards,
Vishal
-Original Message-
From: Vishal Annapurve
Sent: Saturday, November 16, 2013 12:24 PM
To: 'Alan Stern'
Cc: Ming Lei; Linux Kernel Mailing List; linux-usb
Subject: RE: [PATCH] usb-storage: scsiglue: Changing the command result
Hi,
Here are the updated patches:
[PATCH 1/3] usb: storage: Proper cmd result assignment
This change replaces DID_ABORT with DID_TIMEOUT as a command result whenever
US_FLIDX_TIMED_OUT bit is set.
This change is made to bring USB storage inline with a recent change:
commit18a4d0a22ed6c54b67af7718c305cd010f09ddf8
[SCSI] Handle disk devices which can not process medium access commands We have
experienced several devices which fail in a fashion we do not currently handle
gracefully in SCSI. After a failure these devices will respond to the SCSI
primary command set (INQUIRY, TEST UNIT READY, etc.) but any command accessing
the storage medium will time out.
As the USB storage was setting command result as aborted rather than timed out,
SCSI layer was not recognizing the above mentioned failure pattern.
Signed-off-by: Vishal Annapurve vannapu...@nvidia.com
---
drivers/usb/storage/cypress_atacb.c | 1 +
drivers/usb/storage/isd200.c| 2 +-
drivers/usb/storage/transport.c | 8
drivers/usb/storage/usb.c | 10 ++
4 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/storage/cypress_atacb.c
b/drivers/usb/storage/cypress_atacb.c
index 8514a2d..3477ca19 100644
--- a/drivers/usb/storage/cypress_atacb.c
+++ b/drivers/usb/storage
RE: [PATCH] usb-storage: scsiglue: Changing the command result
Hi,
[PATCH 3/3] usb: storage: uas: Proper cmd result assignment
This change replaces DID_ABORT with DID_TIMEOUT as a command result
whenever US_FLIDX_TIMED_OUT bit is set.
This change is made to bring USB storage inline with a recent change:
commit18a4d0a22ed6c54b67af7718c305cd010f09ddf8
[SCSI] Handle disk devices which can not process medium access commands
We have experienced several devices which fail in a fashion we do not
currently handle gracefully in SCSI. After a failure these devices will
respond to the SCSI primary command set (INQUIRY, TEST UNIT READY, etc.)
but any command accessing the storage medium will time out.
As the USB storage was setting command result as aborted rather than
timed out, SCSI layer was not recognizing the above mentioned failure
pattern.
Signed-off-by: Vishal Annapurve vannapu...@nvidia.com
---
drivers/usb/storage/uas.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index d966b59..3a4a44e 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -271,7 +271,10 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const
char *caller)
usb_free_urb(cmdinfo-data_out_urb);
if (cmdinfo-state COMMAND_ABORTED) {
scmd_printk(KERN_INFO, cmnd, abort completed\n);
- cmnd-result = DID_ABORT 16;
+ /* Better to check for US_FLIDX_TIMED_OUT
+* bit settings.
+*/
+ cmnd-result = DID_TIME_OUT 16;
}
cmnd-scsi_done(cmnd);
return 0;
--
1.8.4.2
Regards,
Vishal
-Original Message-
From: Vishal Annapurve
Sent: Saturday, November 16, 2013 12:25 PM
To: 'Alan Stern'
Cc: 'Ming Lei'; 'Linux Kernel Mailing List'; 'linux-usb'
Subject: RE: [PATCH] usb-storage: scsiglue: Changing the command result
Hi,
[PATCH 2/3] keucr: Proper cmd result assignment
This change replaces DID_ABORT with DID_TIMEOUT as a command result whenever
US_FLIDX_TIMED_OUT bit is set.
This change is made to bring USB storage inline with a recent change:
commit18a4d0a22ed6c54b67af7718c305cd010f09ddf8
[SCSI] Handle disk devices which can not process medium access commands We have
experienced several devices which fail in a fashion we do not currently handle
gracefully in SCSI. After a failure these devices will respond to the SCSI
primary command set (INQUIRY, TEST UNIT READY, etc.) but any command accessing
the storage medium will time out.
As the USB storage was setting command result as aborted rather than timed out,
SCSI layer was not recognizing the above mentioned failure pattern.
Signed-off-by: Vishal Annapurve vannapu...@nvidia.com
---
drivers/staging/keucr/transport.c | 6 +++---
drivers/staging/keucr/usb.c | 5 +++--
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/keucr/transport.c
b/drivers/staging/keucr/transport.c
index aeb2186..6f61c20 100644
--- a/drivers/staging/keucr/transport.c
+++ b/drivers/staging/keucr/transport.c
@@ -353,7 +353,7 @@ void usb_stor_invoke_transport(struct scsi_cmnd *srb,
struct us_data *us)
we need to short-circuit all other processing */
if (test_bit(US_FLIDX_TIMED_OUT, us-dflags)) {
/* pr_info(-- command was aborted\n); */
- srb-result = DID_ABORT 16;
+ srb-result = DID_TIME_OUT 16;
goto Handle_Errors;
}
@@ -412,7 +412,7 @@ void usb_stor_invoke_transport(struct scsi_cmnd *srb,
struct us_data *us)
if (test_bit(US_FLIDX_TIMED_OUT, us-dflags)) {
/* pr_info(-- auto-sense aborted\n); */
- srb-result = DID_ABORT 16;
+ srb-result = DID_TIME_OUT 16;
goto Handle_Errors;
}
if (temp_result != USB_STOR_TRANSPORT_GOOD) { @@ -488,7 +488,7
@@ void ENE_stor_invoke_transport(struct scsi_cmnd *srb, struct us_data *us)
we need to short-circuit all other processing */
if (test_bit(US_FLIDX_TIMED_OUT, us-dflags)) {
/* pr_info(-- command was aborted\n); */
- srb-result = DID_ABORT 16;
+ srb-result = DID_TIME_OUT 16;
goto Handle_Errors;
}
diff --git a/drivers/staging/keucr/usb.c b/drivers/staging/keucr/usb.c index
a84ee63..26ec64c 100644
--- a/drivers/staging/keucr/usb.c
+++ b/drivers/staging/keucr/usb.c
@@ -181,7 +181,7 @@ static int usb_stor_control_thread(void *__us)
/* has the command timed out *already* ? */
if (test_bit(US_FLIDX_TIMED_OUT, us-dflags)) {
- us-srb-result = DID_ABORT 16;
+ us-srb-result = DID_TIME_OUT 16;
goto SkipForAbort;
}
@@ -209,7 +209,8 @@ static int usb_stor_control_thread(void *__us)
scsi_lock(host
RE: [PATCH] usb-storage: scsiglue: Changing the command result
Hi Alan,
Here is the new patch:
From: Vishal Annapurve
Date: Sat, 26 Oct 2013 21:10:11 +0530
Subject: [PATCH] usb: storage: Proper cmd result assignment
This change replaces DID_ABORT with DID_TIMEOUT as a command result
whenever US_FLIDX_TIMED_OUT bit is set.
This change is made to bring USB storage inline with a recent change:
commit18a4d0a22ed6c54b67af7718c305cd010f09ddf8
[SCSI] Handle disk devices which can not process medium access commands
We have experienced several devices which fail in a fashion we do not
currently handle gracefully in SCSI. After a failure these devices will
respond to the SCSI primary command set (INQUIRY, TEST UNIT READY, etc.)
but any command accessing the storage medium will time out.
As the USB storage was setting command result as aborted rather than
timed out, SCSI layer was not recognizing the above mentioned failure
pattern.
Change-Id: Ic58e2247fed11649f4dbea56382354ba2fe0be1b
---
drivers/staging/keucr/transport.c | 6 +++---
drivers/staging/keucr/usb.c | 5 +++--
drivers/usb/storage/cypress_atacb.c | 1 +
drivers/usb/storage/isd200.c| 2 +-
drivers/usb/storage/transport.c | 8
drivers/usb/storage/usb.c | 9 +
6 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/keucr/transport.c
b/drivers/staging/keucr/transport.c
index 1a8837d..ac0e3c2 100644
--- a/drivers/staging/keucr/transport.c
+++ b/drivers/staging/keucr/transport.c
@@ -312,7 +312,7 @@ void usb_stor_invoke_transport(struct scsi_cmnd *srb,
struct us_data *us)
we need to short-circuit all other processing */
if (test_bit(US_FLIDX_TIMED_OUT, >dflags)) {
/* pr_info("-- command was aborted\n"); */
- srb->result = DID_ABORT << 16;
+ srb->result = DID_TIME_OUT << 16;
goto Handle_Errors;
}
@@ -371,7 +371,7 @@ void usb_stor_invoke_transport(struct scsi_cmnd *srb,
struct us_data *us)
if (test_bit(US_FLIDX_TIMED_OUT, >dflags)) {
/* pr_info("-- auto-sense aborted\n"); */
- srb->result = DID_ABORT << 16;
+ srb->result = DID_TIME_OUT << 16;
goto Handle_Errors;
}
if (temp_result != USB_STOR_TRANSPORT_GOOD) {
@@ -447,7 +447,7 @@ void ENE_stor_invoke_transport(struct scsi_cmnd *srb,
struct us_data *us)
we need to short-circuit all other processing */
if (test_bit(US_FLIDX_TIMED_OUT, >dflags)) {
/* pr_info("-- command was aborted\n"); */
- srb->result = DID_ABORT << 16;
+ srb->result = DID_TIME_OUT << 16;
goto Handle_Errors;
}
diff --git a/drivers/staging/keucr/usb.c b/drivers/staging/keucr/usb.c
index 66aad3a..79405be 100644
--- a/drivers/staging/keucr/usb.c
+++ b/drivers/staging/keucr/usb.c
@@ -200,7 +200,7 @@ static int usb_stor_control_thread(void * __us)
/* has the command timed out *already* ? */
if (test_bit(US_FLIDX_TIMED_OUT, >dflags))
{
- us->srb->result = DID_ABORT << 16;
+ us->srb->result = DID_TIME_OUT << 16;
goto SkipForAbort;
}
@@ -234,7 +234,8 @@ static int usb_stor_control_thread(void * __us)
scsi_lock(host);
/* indicate that the command is done */
- if (us->srb->result != DID_ABORT << 16)
+ if ((us->srb->result != DID_ABORT << 16) &&
+ (us->srb->result != DID_TIME_OUT << 16))
{
us->srb->scsi_done(us->srb);
}
diff --git a/drivers/usb/storage/cypress_atacb.c
b/drivers/usb/storage/cypress_atacb.c
index c844718..af24894 100644
--- a/drivers/usb/storage/cypress_atacb.c
+++ b/drivers/usb/storage/cypress_atacb.c
@@ -168,6 +168,7 @@ static void cypress_atacb_passthrough(struct scsi_cmnd
*srb, struct us_data *us)
*/
if ((srb->result != (DID_ERROR << 16) &&
srb->result != (DID_ABORT << 16)) &&
+ srb->result != (DID_TIME_OUT << 16) &&
save_cmnd[2] & 0x20) {
struct scsi_eh_save ses;
unsigned char regs[8];
diff --git a/drivers/usb/storage/isd200.c b/drivers/usb/storage/isd200.c
index ffc4193..28b688b 100644
--- a/drivers/usb/storage/isd200.c
+++ b/drivers/usb/storage/isd200.c
@@ -705,7 +705,7 @@ static void isd200_invoke_transport( struct us_data *us,
/* abort processing: the bulk-only transport requires a reset
* following an abort */
Handle_Abort:
-
RE: [PATCH] usb-storage: scsiglue: Changing the command result
Hi Alan,
Here is the new patch:
From: Vishal Annapurve vannapu...@nvidia.com
Date: Sat, 26 Oct 2013 21:10:11 +0530
Subject: [PATCH] usb: storage: Proper cmd result assignment
This change replaces DID_ABORT with DID_TIMEOUT as a command result
whenever US_FLIDX_TIMED_OUT bit is set.
This change is made to bring USB storage inline with a recent change:
commit18a4d0a22ed6c54b67af7718c305cd010f09ddf8
[SCSI] Handle disk devices which can not process medium access commands
We have experienced several devices which fail in a fashion we do not
currently handle gracefully in SCSI. After a failure these devices will
respond to the SCSI primary command set (INQUIRY, TEST UNIT READY, etc.)
but any command accessing the storage medium will time out.
As the USB storage was setting command result as aborted rather than
timed out, SCSI layer was not recognizing the above mentioned failure
pattern.
Change-Id: Ic58e2247fed11649f4dbea56382354ba2fe0be1b
---
drivers/staging/keucr/transport.c | 6 +++---
drivers/staging/keucr/usb.c | 5 +++--
drivers/usb/storage/cypress_atacb.c | 1 +
drivers/usb/storage/isd200.c| 2 +-
drivers/usb/storage/transport.c | 8
drivers/usb/storage/usb.c | 9 +
6 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/keucr/transport.c
b/drivers/staging/keucr/transport.c
index 1a8837d..ac0e3c2 100644
--- a/drivers/staging/keucr/transport.c
+++ b/drivers/staging/keucr/transport.c
@@ -312,7 +312,7 @@ void usb_stor_invoke_transport(struct scsi_cmnd *srb,
struct us_data *us)
we need to short-circuit all other processing */
if (test_bit(US_FLIDX_TIMED_OUT, us-dflags)) {
/* pr_info(-- command was aborted\n); */
- srb-result = DID_ABORT 16;
+ srb-result = DID_TIME_OUT 16;
goto Handle_Errors;
}
@@ -371,7 +371,7 @@ void usb_stor_invoke_transport(struct scsi_cmnd *srb,
struct us_data *us)
if (test_bit(US_FLIDX_TIMED_OUT, us-dflags)) {
/* pr_info(-- auto-sense aborted\n); */
- srb-result = DID_ABORT 16;
+ srb-result = DID_TIME_OUT 16;
goto Handle_Errors;
}
if (temp_result != USB_STOR_TRANSPORT_GOOD) {
@@ -447,7 +447,7 @@ void ENE_stor_invoke_transport(struct scsi_cmnd *srb,
struct us_data *us)
we need to short-circuit all other processing */
if (test_bit(US_FLIDX_TIMED_OUT, us-dflags)) {
/* pr_info(-- command was aborted\n); */
- srb-result = DID_ABORT 16;
+ srb-result = DID_TIME_OUT 16;
goto Handle_Errors;
}
diff --git a/drivers/staging/keucr/usb.c b/drivers/staging/keucr/usb.c
index 66aad3a..79405be 100644
--- a/drivers/staging/keucr/usb.c
+++ b/drivers/staging/keucr/usb.c
@@ -200,7 +200,7 @@ static int usb_stor_control_thread(void * __us)
/* has the command timed out *already* ? */
if (test_bit(US_FLIDX_TIMED_OUT, us-dflags))
{
- us-srb-result = DID_ABORT 16;
+ us-srb-result = DID_TIME_OUT 16;
goto SkipForAbort;
}
@@ -234,7 +234,8 @@ static int usb_stor_control_thread(void * __us)
scsi_lock(host);
/* indicate that the command is done */
- if (us-srb-result != DID_ABORT 16)
+ if ((us-srb-result != DID_ABORT 16)
+ (us-srb-result != DID_TIME_OUT 16))
{
us-srb-scsi_done(us-srb);
}
diff --git a/drivers/usb/storage/cypress_atacb.c
b/drivers/usb/storage/cypress_atacb.c
index c844718..af24894 100644
--- a/drivers/usb/storage/cypress_atacb.c
+++ b/drivers/usb/storage/cypress_atacb.c
@@ -168,6 +168,7 @@ static void cypress_atacb_passthrough(struct scsi_cmnd
*srb, struct us_data *us)
*/
if ((srb-result != (DID_ERROR 16)
srb-result != (DID_ABORT 16))
+ srb-result != (DID_TIME_OUT 16)
save_cmnd[2] 0x20) {
struct scsi_eh_save ses;
unsigned char regs[8];
diff --git a/drivers/usb/storage/isd200.c b/drivers/usb/storage/isd200.c
index ffc4193..28b688b 100644
--- a/drivers/usb/storage/isd200.c
+++ b/drivers/usb/storage/isd200.c
@@ -705,7 +705,7 @@ static void isd200_invoke_transport( struct us_data *us,
/* abort processing: the bulk-only transport requires a reset
* following an abort */
Handle_Abort:
- srb-result = DID_ABORT 16;
+ srb-result = DID_TIME_OUT 16;
/* permit the reset transfer to take place */
clear_bit(US_FLIDX_ABORTING, us-dflags);
diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
index b2697dc
RE: [PATCH] usb-storage: scsiglue: Changing the command result
Hi, Attaching the new patch which will replace all the occurrences of DID_ABORT with DID_TIMOUT in USB Storage whenever it sees US_FLIDX_TIMED_OUT bit is set. Regards, Vishal -Original Message- From: Alan Stern [mailto:st...@rowland.harvard.edu] Sent: Friday, October 18, 2013 4:10 PM To: Vishal Annapurve Cc: Ming Lei; Linux Kernel Mailing List; linux-usb Subject: RE: [PATCH] usb-storage: scsiglue: Changing the command result On Fri, 18 Oct 2013, Vishal Annapurve wrote: > Hi Alan, > > What I wanted to say was If the bit US_FLIDX_TIMED_OUT can have more > meanings than timed out then maybe it would be best to override the > results after usb-storage is done with the command maybe in scsi layer > itself who aborted it in the first place. US_FLIDX_TIMED_OUT has one very specific meaning: command_abort() was called. Since command_abort() is the .eh_abort_handler routine, US_FLIDX_TIMED_OUT means that the SCSI layer decided to abort the command. Does the SCSI layer ever abort a command for any reason other than a timeout? If not, you may conclude that US_FLIDX_TIMED_OUT indicates a timeout. But if it does, you should not make this conclusion. > My concern was that overriding the result in usb storage or scsi > layers will have more side effects than doing it in scsiglue.c. > And by scsi-usb storage bridge what I meant was specifically the code > in scsiglue. > > Question about your last mail, do you want to change all the > occurrences of DID_ABORT from usb-storage to DID_TIMEOUT? Put it this way: There's no good reason for changing some of them but not all of them. And if you're going to change them at all, it makes no sense to first set the result to DID_ABORT and then change it to DID_TIMEOUT. You should simply set it to DID_TIMEOUT in the first place. Alan Stern usb_storage_change_abort_to_timeout.patch Description: usb_storage_change_abort_to_timeout.patch
RE: [PATCH] usb-storage: scsiglue: Changing the command result
Hi, Attaching the new patch which will replace all the occurrences of DID_ABORT with DID_TIMOUT in USB Storage whenever it sees US_FLIDX_TIMED_OUT bit is set. Regards, Vishal -Original Message- From: Alan Stern [mailto:st...@rowland.harvard.edu] Sent: Friday, October 18, 2013 4:10 PM To: Vishal Annapurve Cc: Ming Lei; Linux Kernel Mailing List; linux-usb Subject: RE: [PATCH] usb-storage: scsiglue: Changing the command result On Fri, 18 Oct 2013, Vishal Annapurve wrote: Hi Alan, What I wanted to say was If the bit US_FLIDX_TIMED_OUT can have more meanings than timed out then maybe it would be best to override the results after usb-storage is done with the command maybe in scsi layer itself who aborted it in the first place. US_FLIDX_TIMED_OUT has one very specific meaning: command_abort() was called. Since command_abort() is the .eh_abort_handler routine, US_FLIDX_TIMED_OUT means that the SCSI layer decided to abort the command. Does the SCSI layer ever abort a command for any reason other than a timeout? If not, you may conclude that US_FLIDX_TIMED_OUT indicates a timeout. But if it does, you should not make this conclusion. My concern was that overriding the result in usb storage or scsi layers will have more side effects than doing it in scsiglue.c. And by scsi-usb storage bridge what I meant was specifically the code in scsiglue. Question about your last mail, do you want to change all the occurrences of DID_ABORT from usb-storage to DID_TIMEOUT? Put it this way: There's no good reason for changing some of them but not all of them. And if you're going to change them at all, it makes no sense to first set the result to DID_ABORT and then change it to DID_TIMEOUT. You should simply set it to DID_TIMEOUT in the first place. Alan Stern usb_storage_change_abort_to_timeout.patch Description: usb_storage_change_abort_to_timeout.patch
RE: [PATCH] usb-storage: scsiglue: Changing the command result
Hi Alan, What I wanted to say was If the bit US_FLIDX_TIMED_OUT can have more meanings than timed out then maybe it would be best to override the results after usb-storage is done with the command maybe in scsi layer itself who aborted it in the first place. My concern was that overriding the result in usb storage or scsi layers will have more side effects than doing it in scsiglue.c. And by scsi-usb storage bridge what I meant was specifically the code in scsiglue. Question about your last mail, do you want to change all the occurrences of DID_ABORT from usb-storage to DID_TIMEOUT? Regards, Vishal -Original Message- From: Alan Stern [mailto:st...@rowland.harvard.edu] Sent: Tuesday, October 15, 2013 10:22 PM To: Vishal Annapurve Cc: Ming Lei; Linux Kernel Mailing List; linux-usb Subject: RE: [PATCH] usb-storage: scsiglue: Changing the command result On Tue, 15 Oct 2013, Vishal Annapurve wrote: > Hi Alan, > > USB storage maybe just has to say that the abort occurred. By setting > the US_FLIDX_TIMED_OUT bit USB storage is getting signaled that the > reason was time out and the command is being aborted. No. By setting the US_FLIDX_TIMED_OUT bit, usb-storage indicates that the command was aborted. This doesn't indicate anything about the reason for the abort. (Maybe this bit's name wasn't chosen very well.) > Now, it's arguable whether to change the implication of > US_FLIDX_TIMED_OUT bit for scsi - USB storage bridge or for entire usb > storage I don't understand this. What's the difference between "scsi - USB storage bridge" and "entire usb storage"? Aren't they the same thing? > Or maybe scsi has > decided to abort so it should override the result. Of course the SCSI midlayer has decided to abort. That's the only way this bit can get set. But usb-storage doesn't know why SCSI decided to abort. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] usb-storage: scsiglue: Changing the command result
Hi Alan, What I wanted to say was If the bit US_FLIDX_TIMED_OUT can have more meanings than timed out then maybe it would be best to override the results after usb-storage is done with the command maybe in scsi layer itself who aborted it in the first place. My concern was that overriding the result in usb storage or scsi layers will have more side effects than doing it in scsiglue.c. And by scsi-usb storage bridge what I meant was specifically the code in scsiglue. Question about your last mail, do you want to change all the occurrences of DID_ABORT from usb-storage to DID_TIMEOUT? Regards, Vishal -Original Message- From: Alan Stern [mailto:st...@rowland.harvard.edu] Sent: Tuesday, October 15, 2013 10:22 PM To: Vishal Annapurve Cc: Ming Lei; Linux Kernel Mailing List; linux-usb Subject: RE: [PATCH] usb-storage: scsiglue: Changing the command result On Tue, 15 Oct 2013, Vishal Annapurve wrote: Hi Alan, USB storage maybe just has to say that the abort occurred. By setting the US_FLIDX_TIMED_OUT bit USB storage is getting signaled that the reason was time out and the command is being aborted. No. By setting the US_FLIDX_TIMED_OUT bit, usb-storage indicates that the command was aborted. This doesn't indicate anything about the reason for the abort. (Maybe this bit's name wasn't chosen very well.) Now, it's arguable whether to change the implication of US_FLIDX_TIMED_OUT bit for scsi - USB storage bridge or for entire usb storage I don't understand this. What's the difference between scsi - USB storage bridge and entire usb storage? Aren't they the same thing? Or maybe scsi has decided to abort so it should override the result. Of course the SCSI midlayer has decided to abort. That's the only way this bit can get set. But usb-storage doesn't know why SCSI decided to abort. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] usb-storage: scsiglue: Changing the command result
Hi Alan, USB storage maybe just has to say that the abort occurred. By setting the US_FLIDX_TIMED_OUT bit USB storage is getting signaled that the reason was time out and the command is being aborted. Now, it's arguable whether to change the implication of US_FLIDX_TIMED_OUT bit for scsi - USB storage bridge or for entire usb storage Or maybe scsi has decided to abort so it should override the result. Regards, Vishal -Original Message- From: Alan Stern [mailto:st...@rowland.harvard.edu] Sent: Tuesday, October 15, 2013 7:03 PM To: Vishal Annapurve Cc: Ming Lei; Linux Kernel Mailing List; linux-usb Subject: RE: [PATCH] usb-storage: scsiglue: Changing the command result On Tue, 15 Oct 2013, Vishal Annapurve wrote: > Hi Alan, > > This issue seems more related to the devices using SCSI protocol and > the changes otherwise will be at more places giving the same end result. > > I think as the comment says over the command_abort function, > intentional result change should only happen in case of timeout. usb-storage doesn't know _why_ a command was aborted; it knows only that the abort occurred. If you look carefully at the code, you'll see that the result is set to DID_ABORT only when the US_FLIDX_TIMED_OUT bit is set, and this bit gets set only when a SCSI abort occurs. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] usb-storage: scsiglue: Changing the command result
Hi Alan, This issue seems more related to the devices using SCSI protocol and the changes otherwise will be at more places giving the same end result. I think as the comment says over the command_abort function, intentional result change should only happen in case of timeout. Regards, Vishal -Original Message- From: Alan Stern [mailto:st...@rowland.harvard.edu] Sent: Tuesday, October 15, 2013 5:55 PM To: Ming Lei Cc: Vishal Annapurve; Linux Kernel Mailing List; linux-usb Subject: Re: [PATCH] usb-storage: scsiglue: Changing the command result On Tue, 15 Oct 2013, Ming Lei wrote: > On Thu, Sep 26, 2013 at 7:35 PM, Vishal Annapurve > wrote: > > Hi, > > > > There was a recent commit in mainline for the scsi devices which do > > not respond properly to medium access command: > > > > commit18a4d0a22ed6c54b67af7718c305cd010f09ddf8 > > > > [SCSI] Handle disk devices which can not process medium access > > commands We have experienced several devices which fail in a fashion > > we do not currently handle gracefully in SCSI. After a failure these > > devices will respond to the SCSI primary command set (INQUIRY, TEST > > UNIT READY, etc.) but any command accessing the storage medium will time > > out. > > > > I came across a USB drive which showed similar problem and what I > > see is usb storage is still not able to cope with such devices properly. > > > > The control flow downwards is like: > > scsi_times_out --> Setting cmd->result as DID_TIME_OUT > > scsi_error_handler scsi_unjam_host scsi_eh_abort_cmds > > command_abort(sets US_FLIDX_TIMED_OUT for us->dflags > > calls stop_transport, > > and waits for)usb_stor_control_thread > > (which is waiting for > > > > transport call to return inside > > > > usb_stor_invoke_transport) > > > > both usb_stor_control_thread and > > > > usb_stor_invoke_transport > > check for > > us->dflags timed_out bit and > > set the result > > as DID_ABORT > > and signal > > completion for command_abort > > to complete > > .. > > sd_eh_action > > checks for cmd->result and finds out that it's DID_ABORT rather than > > DID_TIME_OUT. > > > > This patch updates the command result to be TIME_OUT explicitly > > before returning from command_abort in scsiglue.c. > > > > I would like to know if this patch can work out for such USB Storage > > devices? What would be the better way to do the same? > > Looks your diagnose is correct, and patch should be doable, the only > side effect is that previous returned DID_ABORT in srb->result is > replaced with DID_TIME_OUT now. > > Another way is to implement .eh_timed_out callback to return different > error for the two failure, but looks not a big deal. Instead of overriding the result code, a better way to do this would simply be to change the places where srb->result is currently set to DID_ABORT << 16. Just make it DID_TIME_OUT << 16 instead. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] usb-storage: scsiglue: Changing the command result
Hi Alan, This issue seems more related to the devices using SCSI protocol and the changes otherwise will be at more places giving the same end result. I think as the comment says over the command_abort function, intentional result change should only happen in case of timeout. Regards, Vishal -Original Message- From: Alan Stern [mailto:st...@rowland.harvard.edu] Sent: Tuesday, October 15, 2013 5:55 PM To: Ming Lei Cc: Vishal Annapurve; Linux Kernel Mailing List; linux-usb Subject: Re: [PATCH] usb-storage: scsiglue: Changing the command result On Tue, 15 Oct 2013, Ming Lei wrote: On Thu, Sep 26, 2013 at 7:35 PM, Vishal Annapurve vannapu...@nvidia.com wrote: Hi, There was a recent commit in mainline for the scsi devices which do not respond properly to medium access command: commit18a4d0a22ed6c54b67af7718c305cd010f09ddf8 [SCSI] Handle disk devices which can not process medium access commands We have experienced several devices which fail in a fashion we do not currently handle gracefully in SCSI. After a failure these devices will respond to the SCSI primary command set (INQUIRY, TEST UNIT READY, etc.) but any command accessing the storage medium will time out. I came across a USB drive which showed similar problem and what I see is usb storage is still not able to cope with such devices properly. The control flow downwards is like: scsi_times_out -- Setting cmd-result as DID_TIME_OUT scsi_error_handler scsi_unjam_host scsi_eh_abort_cmds command_abort(sets US_FLIDX_TIMED_OUT for us-dflags calls stop_transport, and waits for)usb_stor_control_thread (which is waiting for transport call to return inside usb_stor_invoke_transport) both usb_stor_control_thread and usb_stor_invoke_transport check for us-dflags timed_out bit and set the result as DID_ABORT and signal completion for command_abort to complete .. sd_eh_action checks for cmd-result and finds out that it's DID_ABORT rather than DID_TIME_OUT. This patch updates the command result to be TIME_OUT explicitly before returning from command_abort in scsiglue.c. I would like to know if this patch can work out for such USB Storage devices? What would be the better way to do the same? Looks your diagnose is correct, and patch should be doable, the only side effect is that previous returned DID_ABORT in srb-result is replaced with DID_TIME_OUT now. Another way is to implement .eh_timed_out callback to return different error for the two failure, but looks not a big deal. Instead of overriding the result code, a better way to do this would simply be to change the places where srb-result is currently set to DID_ABORT 16. Just make it DID_TIME_OUT 16 instead. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] usb-storage: scsiglue: Changing the command result
Hi Alan, USB storage maybe just has to say that the abort occurred. By setting the US_FLIDX_TIMED_OUT bit USB storage is getting signaled that the reason was time out and the command is being aborted. Now, it's arguable whether to change the implication of US_FLIDX_TIMED_OUT bit for scsi - USB storage bridge or for entire usb storage Or maybe scsi has decided to abort so it should override the result. Regards, Vishal -Original Message- From: Alan Stern [mailto:st...@rowland.harvard.edu] Sent: Tuesday, October 15, 2013 7:03 PM To: Vishal Annapurve Cc: Ming Lei; Linux Kernel Mailing List; linux-usb Subject: RE: [PATCH] usb-storage: scsiglue: Changing the command result On Tue, 15 Oct 2013, Vishal Annapurve wrote: Hi Alan, This issue seems more related to the devices using SCSI protocol and the changes otherwise will be at more places giving the same end result. I think as the comment says over the command_abort function, intentional result change should only happen in case of timeout. usb-storage doesn't know _why_ a command was aborted; it knows only that the abort occurred. If you look carefully at the code, you'll see that the result is set to DID_ABORT only when the US_FLIDX_TIMED_OUT bit is set, and this bit gets set only when a SCSI abort occurs. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] usb-storage: scsiglue: Changing the command result
Hi, There was a recent commit in mainline for the scsi devices which do not respond properly to medium access command: commit18a4d0a22ed6c54b67af7718c305cd010f09ddf8 [SCSI] Handle disk devices which can not process medium access commands We have experienced several devices which fail in a fashion we do not currently handle gracefully in SCSI. After a failure these devices will respond to the SCSI primary command set (INQUIRY, TEST UNIT READY, etc.) but any command accessing the storage medium will time out. I came across a USB drive which showed similar problem and what I see is usb storage is still not able to cope with such devices properly. The control flow downwards is like: scsi_times_out --> Setting cmd->result as DID_TIME_OUT scsi_error_handler scsi_unjam_host scsi_eh_abort_cmds command_abort(sets US_FLIDX_TIMED_OUT for us->dflags calls stop_transport, and waits for)usb_stor_control_thread (which is waiting for transport call to return inside usb_stor_invoke_transport) both usb_stor_control_thread and usb_stor_invoke_transport check for us->dflags timed_out bit and set the result as DID_ABORT and signal completion for command_abort to complete .. sd_eh_action checks for cmd->result and finds out that it's DID_ABORT rather than DID_TIME_OUT. This patch updates the command result to be TIME_OUT explicitly before returning from command_abort in scsiglue.c. I would like to know if this patch can work out for such USB Storage devices? What would be the better way to do the same? Regards, Vishal Annapurve From: Vishal Annapurve Subject: [PATCH] usb-storage: Changing the command result This change updates the returned result to scsi layer to use DID_TIMEOUT flag as a result status rather than DID_ABORT for commands aborted due to timeout. Signed-off-by: Vishal Annapurve --- drivers/usb/storage/scsiglue.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c index a74e9d8..e7b532e 100644 --- a/drivers/usb/storage/scsiglue.c +++ b/drivers/usb/storage/scsiglue.c @@ -353,6 +353,15 @@ static int command_abort(struct scsi_cmnd *srb) /* Wait for the aborted command to finish */ wait_for_completion(>notify); + /* Given that this bit would be only set in case of timedout + * command, we can return with DID_TIME_OUT, as scsi layer needs + * to use this result rather than DID_ABORT. + * This is a WR to avoid removing DID_ABORT altogether + * and before the final control goes to scsi layer change the + * result to be timedout. + */ + us->srb->result = DID_TIME_OUT << 16; + return SUCCESS; } -- 1.8.4
[PATCH] mmc: Reducing cache operations in the host driver
Hi,
I am attaching a patch which attempts to reduce the cache operations
while doing
MMC transactions. I have tested it only on arm and the tests performed
with benchmarks
like iozone/bonnie showed that the data integrity is maintained while
I/O bandwidth is increased.
I have tested it with K3.1 and I believe it can apply to 3.12 also.
My understanding from the important API's dealing with DMA memory is as
follows:
1) dma_map_sg/ dma_sync_sg_for_device -> make sure that cache is flushed
after CPU is done
updating the memory allocated for DMA and is called before giving
control of DMA memory to
the device.
2) dma_unmap_sg/dma_sync_sg_for_cpu -> Make sure that cache is
invalidated before
reading from the DMA area which was used by the device to write the data.
About the patch:
Changes in sdhci_adma_table_pre make sure that we only flush if we have
updated DMA
area after the call to dma_map_sg.
Changes in sdhci_adma_table_post take care of following:
1) Remove invalidation of cache for memory locations which are going to
be updated by CPU,
as they are not being read.
2) Perform the unmap of sg before CPU accesses DMA area as the changes
we did for unaligned
cases might get lost due to invalidation afterwards. I was not able to
induce unaligned buffer
accesses using normal filesystem/raw device operations. Maybe that's why
this issue was not
discovered so far.
3) Only drawback is sg->dma_address gets used after the call to
dma_unmap_sg.
I would like to understand if this patch can cause any regressions for
any of the architectures
or with the MMC functionality.
Thanks & Regards,
Vishal Annapurve
Author: Vishal Annapurve
MMC: Remove unnecessary cache operations
1) This change removes unnecessary cache operations happening
after and before DMA setup in MMC host driver.
Signed-off-by: Vishal Annapurve
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 6e43c84..d5cd0ae 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -434,6 +434,7 @@ static int sdhci_adma_table_pre(struct sdhci_host *host,
dma_addr_t addr;
dma_addr_t align_addr;
int len, offset;
+ int align_buf_modified_len = 0;
struct scatterlist *sg;
int i;
@@ -498,6 +499,7 @@ static int sdhci_adma_table_pre(struct sdhci_host *host,
align += 4;
align_addr += 4;
+ align_buf_modified_len += 4;
desc += 8;
@@ -538,9 +540,10 @@ static int sdhci_adma_table_pre(struct sdhci_host *host,
/*
* Resync align buffer as we might have changed it.
*/
- if (data->flags & MMC_DATA_WRITE) {
+ if ((data->flags & MMC_DATA_WRITE) &&
+ align_buf_modified_len) {
dma_sync_single_for_device(mmc_dev(host->mmc),
- host->align_addr, 128 * 4, direction);
+ host->align_addr, align_buf_modified_len, direction);
}
host->adma_addr = dma_map_single(mmc_dev(host->mmc),
@@ -583,9 +586,10 @@ static void sdhci_adma_table_post(struct sdhci_host *host,
dma_unmap_single(mmc_dev(host->mmc), host->align_addr,
128 * 4, direction);
+ dma_unmap_sg(mmc_dev(host->mmc), data->sg,
+ data->sg_len, direction);
+
if (data->flags & MMC_DATA_READ) {
- dma_sync_sg_for_cpu(mmc_dev(host->mmc), data->sg,
- data->sg_len, direction);
align = host->align_buffer;
@@ -602,9 +606,6 @@ static void sdhci_adma_table_post(struct sdhci_host *host,
}
}
}
-
- dma_unmap_sg(mmc_dev(host->mmc), data->sg,
- data->sg_len, direction);
}
static u8 sdhci_calc_timeout(struct sdhci_host *host, struct mmc_command *cmd)
[PATCH] mmc: Reducing cache operations in the host driver
Hi,
I am attaching a patch which attempts to reduce the cache operations
while doing
MMC transactions. I have tested it only on arm and the tests performed
with benchmarks
like iozone/bonnie showed that the data integrity is maintained while
I/O bandwidth is increased.
I have tested it with K3.1 and I believe it can apply to 3.12 also.
My understanding from the important API's dealing with DMA memory is as
follows:
1) dma_map_sg/ dma_sync_sg_for_device - make sure that cache is flushed
after CPU is done
updating the memory allocated for DMA and is called before giving
control of DMA memory to
the device.
2) dma_unmap_sg/dma_sync_sg_for_cpu - Make sure that cache is
invalidated before
reading from the DMA area which was used by the device to write the data.
About the patch:
Changes in sdhci_adma_table_pre make sure that we only flush if we have
updated DMA
area after the call to dma_map_sg.
Changes in sdhci_adma_table_post take care of following:
1) Remove invalidation of cache for memory locations which are going to
be updated by CPU,
as they are not being read.
2) Perform the unmap of sg before CPU accesses DMA area as the changes
we did for unaligned
cases might get lost due to invalidation afterwards. I was not able to
induce unaligned buffer
accesses using normal filesystem/raw device operations. Maybe that's why
this issue was not
discovered so far.
3) Only drawback is sg-dma_address gets used after the call to
dma_unmap_sg.
I would like to understand if this patch can cause any regressions for
any of the architectures
or with the MMC functionality.
Thanks Regards,
Vishal Annapurve
Author: Vishal Annapurve vannapu...@nvidia.com
MMC: Remove unnecessary cache operations
1) This change removes unnecessary cache operations happening
after and before DMA setup in MMC host driver.
Signed-off-by: Vishal Annapurve vannapu...@nvidia.com
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 6e43c84..d5cd0ae 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -434,6 +434,7 @@ static int sdhci_adma_table_pre(struct sdhci_host *host,
dma_addr_t addr;
dma_addr_t align_addr;
int len, offset;
+ int align_buf_modified_len = 0;
struct scatterlist *sg;
int i;
@@ -498,6 +499,7 @@ static int sdhci_adma_table_pre(struct sdhci_host *host,
align += 4;
align_addr += 4;
+ align_buf_modified_len += 4;
desc += 8;
@@ -538,9 +540,10 @@ static int sdhci_adma_table_pre(struct sdhci_host *host,
/*
* Resync align buffer as we might have changed it.
*/
- if (data-flags MMC_DATA_WRITE) {
+ if ((data-flags MMC_DATA_WRITE)
+ align_buf_modified_len) {
dma_sync_single_for_device(mmc_dev(host-mmc),
- host-align_addr, 128 * 4, direction);
+ host-align_addr, align_buf_modified_len, direction);
}
host-adma_addr = dma_map_single(mmc_dev(host-mmc),
@@ -583,9 +586,10 @@ static void sdhci_adma_table_post(struct sdhci_host *host,
dma_unmap_single(mmc_dev(host-mmc), host-align_addr,
128 * 4, direction);
+ dma_unmap_sg(mmc_dev(host-mmc), data-sg,
+ data-sg_len, direction);
+
if (data-flags MMC_DATA_READ) {
- dma_sync_sg_for_cpu(mmc_dev(host-mmc), data-sg,
- data-sg_len, direction);
align = host-align_buffer;
@@ -602,9 +606,6 @@ static void sdhci_adma_table_post(struct sdhci_host *host,
}
}
}
-
- dma_unmap_sg(mmc_dev(host-mmc), data-sg,
- data-sg_len, direction);
}
static u8 sdhci_calc_timeout(struct sdhci_host *host, struct mmc_command *cmd)
[PATCH] usb-storage: scsiglue: Changing the command result
Hi, There was a recent commit in mainline for the scsi devices which do not respond properly to medium access command: commit18a4d0a22ed6c54b67af7718c305cd010f09ddf8 [SCSI] Handle disk devices which can not process medium access commands We have experienced several devices which fail in a fashion we do not currently handle gracefully in SCSI. After a failure these devices will respond to the SCSI primary command set (INQUIRY, TEST UNIT READY, etc.) but any command accessing the storage medium will time out. I came across a USB drive which showed similar problem and what I see is usb storage is still not able to cope with such devices properly. The control flow downwards is like: scsi_times_out -- Setting cmd-result as DID_TIME_OUT scsi_error_handler scsi_unjam_host scsi_eh_abort_cmds command_abort(sets US_FLIDX_TIMED_OUT for us-dflags calls stop_transport, and waits for)usb_stor_control_thread (which is waiting for transport call to return inside usb_stor_invoke_transport) both usb_stor_control_thread and usb_stor_invoke_transport check for us-dflags timed_out bit and set the result as DID_ABORT and signal completion for command_abort to complete .. sd_eh_action checks for cmd-result and finds out that it's DID_ABORT rather than DID_TIME_OUT. This patch updates the command result to be TIME_OUT explicitly before returning from command_abort in scsiglue.c. I would like to know if this patch can work out for such USB Storage devices? What would be the better way to do the same? Regards, Vishal Annapurve From: Vishal Annapurve vannapu...@nvidia.com Subject: [PATCH] usb-storage: Changing the command result This change updates the returned result to scsi layer to use DID_TIMEOUT flag as a result status rather than DID_ABORT for commands aborted due to timeout. Signed-off-by: Vishal Annapurve vannapu...@nvidia.com --- drivers/usb/storage/scsiglue.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c index a74e9d8..e7b532e 100644 --- a/drivers/usb/storage/scsiglue.c +++ b/drivers/usb/storage/scsiglue.c @@ -353,6 +353,15 @@ static int command_abort(struct scsi_cmnd *srb) /* Wait for the aborted command to finish */ wait_for_completion(us-notify); + /* Given that this bit would be only set in case of timedout + * command, we can return with DID_TIME_OUT, as scsi layer needs + * to use this result rather than DID_ABORT. + * This is a WR to avoid removing DID_ABORT altogether + * and before the final control goes to scsi layer change the + * result to be timedout. + */ + us-srb-result = DID_TIME_OUT 16; + return SUCCESS; } -- 1.8.4
