[PATCH] scsi_error fix

Andries.Brouwer@cwi.nl
Fri, 7 Mar 2003 00:37:57 +0100 (MET)


Below:
imm.c: spelling
scsi.h: remove old and now incorrect comment
scsi_scan.c: remove superfluous final return
scsi_error.c: apart from similar trivialities the only change:

If a command fails (e.g. because it belongs to a newer
SCSI version than the device), it is fed to
scsi_decide_disposition(). That routine must return
SUCCESS, unless the error handler should be invoked.

In the situation where host_byte is DID_OK, and message_byte
is COMMAND_COMPLETE, and status is CHECK_CONDITION, there is
no reason at all to invoke aborts and resets. The situation
is normal. I see here UNIT ATTENTION, Power on occurred
and ILLEGAL REQUEST, Invalid field in cdb.

The 2.5.64 code does not return SUCCESS, but it returns the
return code of scsi_check_sense(), and that may be FAILED
in case we do not have valid sense.

Further discussion is possible here, but I changed the return
into "return SUCCESS" and 2.5.64 boots fine.

Andries

[Further discussion and things I did not yet investigate:
What was changed to make this fail first in 2.5.63?
Experience shows that we get into a loop when something else
than SUCCESS is returned here. Probably that is a bug elsewhere.
Probably the commands that cause problems should never have been
sent in the first place.]

diff -u --recursive --new-file -X /linux/dontdiff a/drivers/scsi/imm.c b/drivers/scsi/imm.c
--- a/drivers/scsi/imm.c Sat Feb 15 20:41:59 2003
+++ b/drivers/scsi/imm.c Thu Mar 6 23:50:31 2003
@@ -174,6 +174,7 @@
parport_unregister_device(imm_hosts[i].dev);
continue;
}
+
/* now the glue ... */
switch (imm_hosts[i].mode) {
case IMM_NIBBLE:
@@ -218,7 +219,7 @@

/* This is to give the imm driver a way to modify the timings (and other
* parameters) by writing to the /proc/scsi/imm/0 file.
- * Very simple method really... (To simple, no error checking :( )
+ * Very simple method really... (Too simple, no error checking :( )
* Reason: Kernel hackers HATE having to unload and reload modules for
* testing...
* Also gives a method to use a script to obtain optimum timings (TODO)
@@ -948,7 +949,7 @@
unsigned char l = 0, h = 0;
int retv, x;

- /* First check for any errors that may of occurred
+ /* First check for any errors that may have occurred
* Here we check for internal errors
*/
if (tmp->failed)
diff -u --recursive --new-file -X /linux/dontdiff a/drivers/scsi/scsi.h b/drivers/scsi/scsi.h
--- a/drivers/scsi/scsi.h Wed Mar 5 10:47:26 2003
+++ b/drivers/scsi/scsi.h Thu Mar 6 23:50:31 2003
@@ -280,26 +280,7 @@
#define SCSI_SET_IOCTL_LOGGING(LEVEL) \
SCSI_SET_LOGGING(SCSI_LOG_IOCTL_SHIFT, SCSI_LOG_IOCTL_BITS, LEVEL);

-/*
- * the return of the status word will be in the following format :
- * The low byte is the status returned by the SCSI command,
- * with vendor specific bits masked.
- *
- * The next byte is the message which followed the SCSI status.
- * This allows a stos to be used, since the Intel is a little
- * endian machine.
- *
- * The final byte is a host return code, which is one of the following.
- *
- * IE
- * lsb msb
- * status msg host code
- *
- * Our errors returned by OUR driver, NOT SCSI message. Or'd with
- * SCSI message passed back to driver <IF any>.
- */
-
-
+/* host byte codes */
#define DID_OK 0x00 /* NO error */
#define DID_NO_CONNECT 0x01 /* Couldn't connect before timeout period */
#define DID_BUS_BUSY 0x02 /* BUS stayed busy through time out period */
diff -u --recursive --new-file -X /linux/dontdiff a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
--- a/drivers/scsi/scsi_error.c Mon Feb 24 23:02:52 2003
+++ b/drivers/scsi/scsi_error.c Thu Mar 6 23:50:31 2003
@@ -92,10 +92,8 @@
*
* Notes:
* This should be turned into an inline function. Each scsi command
- * has it's own timer, and as it is added to the queue, we set up the
- * timer. When the command completes, we cancel the timer. Pretty
- * simple, really, especially compared to the old way of handling this
- * crap.
+ * has its own timer, and as it is added to the queue, we set up the
+ * timer. When the command completes, we cancel the timer.
**/
void scsi_add_timer(struct scsi_cmnd *scmd, int timeout,
void (*complete)(struct scsi_cmnd *))
@@ -249,6 +247,7 @@
{
if (!SCSI_SENSE_VALID(scmd))
return FAILED;
+
if (scmd->sense_buffer[2] & 0xe0)
return SUCCESS;

@@ -1193,12 +1192,12 @@
* Notes:
* This is *only* called when we are examining the status after sending
* out the actual data command. any commands that are queued for error
- * recovery (i.e. test_unit_ready) do *not* come through here.
+ * recovery (e.g. test_unit_ready) do *not* come through here.
*
* When this routine returns failed, it means the error handler thread
- * is woken. in cases where the error code indicates an error that
+ * is woken. In cases where the error code indicates an error that
* doesn't require the error handler read (i.e. we don't need to
- * abort/reset), then this function should return SUCCESS.
+ * abort/reset), this function should return SUCCESS.
**/
int scsi_decide_disposition(struct scsi_cmnd *scmd)
{
@@ -1214,11 +1213,11 @@
__FUNCTION__));
return SUCCESS;
}
+
/*
* first check the host byte, to see if there is anything in there
* that would indicate what we need to do.
*/
-
switch (host_byte(scmd->result)) {
case DID_PASSTHROUGH:
/*
@@ -1296,11 +1295,11 @@
/*
* next, check the message byte.
*/
- if (msg_byte(scmd->result) != COMMAND_COMPLETE) {
+ if (msg_byte(scmd->result) != COMMAND_COMPLETE)
return FAILED;
- }
+
/*
- * now, check the status byte to see if this indicates anything special.
+ * check the status byte to see if this indicates anything special.
*/
switch (status_byte(scmd->result)) {
case QUEUE_FULL:
@@ -1321,10 +1320,11 @@
return SUCCESS;
case CHECK_CONDITION:
rtn = scsi_check_sense(scmd);
- if (rtn == NEEDS_RETRY) {
+ if (rtn == NEEDS_RETRY)
goto maybe_retry;
- }
- return rtn;
+ /* if rtn == FAILED, we have no sense information */
+ /* was: return rtn; */
+ return SUCCESS;
case CONDITION_GOOD:
case INTERMEDIATE_GOOD:
case INTERMEDIATE_C_GOOD:
diff -u --recursive --new-file -X /linux/dontdiff a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
--- a/drivers/scsi/scsi_scan.c Wed Mar 5 10:47:26 2003
+++ b/drivers/scsi/scsi_scan.c Fri Mar 7 00:02:09 2003
@@ -960,7 +960,6 @@
SCSI_LOG_SCAN_BUS(3, printk(KERN_INFO "scsi scan: host %d channel %d"
" id %d lun %d name/id: '%s'\n", sdev->host->host_no,
sdev->channel, sdev->id, sdev->lun, sdev->sdev_driverfs_dev.name));
- return;
}

/**
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/