[PATCH] scsi_error fix

From: Andries.Brouwer@cwi.nl
Date: Thu Mar 06 2003 - 18:37:57 EST


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/



This archive was generated by hypermail 2b29 : Fri Mar 07 2003 - 22:00:34 EST