[SCSI] sd/scsi_lib simplify sd_rw_intr and scsi_io_completion
authorLuben Tuikov <ltuikov@yahoo.com>
Fri, 23 Jun 2006 16:39:09 +0000 (09:39 -0700)
committerJames Bottomley <jejb@mulgrave.il.steeleye.com>
Mon, 26 Jun 2006 15:00:20 +0000 (10:00 -0500)
This patch simplifies "good_bytes" computation in sd_rw_intr().
sd: "good_bytes" computation is always done in terms of the resolution
of the device's medium, since after that it is the number of good bytes
we pass around and other layers/contexts (as opposed ot sd) can translate
that to their own resolution (block layer:512).  It also makes
scsi_io_completion() processing more straightforward, eliminating the
3rd argument to the function.

It also fixes a couple of bugs like not checking return value,
using "break" instead of "return;", etc.

I've been running with this patch for some time now on a
test (do-it-all) system.

Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
Signed-off-by: James Bottomley <James.Bottomley@SteelEye.com>
drivers/scsi/scsi_lib.c
drivers/scsi/sd.c
drivers/scsi/sr.c
include/scsi/scsi_cmnd.h

index 3d04a9f386acc51744aad2c098b7ee3b9abeb518..4c4add53d69b27a377e3909cc059d290441bc297 100644 (file)
@@ -855,8 +855,7 @@ static void scsi_release_buffers(struct scsi_cmnd *cmd)
  *             b) We can just use scsi_requeue_command() here.  This would
  *                be used if we just wanted to retry, for example.
  */
-void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes,
-                       unsigned int block_bytes)
+void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
 {
        int result = cmd->result;
        int this_count = cmd->bufflen;
@@ -921,87 +920,72 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes,
         * Next deal with any sectors which we were able to correctly
         * handle.
         */
-       if (good_bytes >= 0) {
-               SCSI_LOG_HLCOMPLETE(1, printk("%ld sectors total, %d bytes done.\n",
+       if (good_bytes > 0) {
+               SCSI_LOG_HLCOMPLETE(1, printk("%ld sectors total, "
+                                             "%d bytes done.\n",
                                              req->nr_sectors, good_bytes));
                SCSI_LOG_HLCOMPLETE(1, printk("use_sg is %d\n", cmd->use_sg));
 
                if (clear_errors)
                        req->errors = 0;
-               /*
-                * If multiple sectors are requested in one buffer, then
-                * they will have been finished off by the first command.
-                * If not, then we have a multi-buffer command.
-                *
-                * If block_bytes != 0, it means we had a medium error
-                * of some sort, and that we want to mark some number of
-                * sectors as not uptodate.  Thus we want to inhibit
-                * requeueing right here - we will requeue down below
-                * when we handle the bad sectors.
-                */
 
-               /*
-                * If the command completed without error, then either
-                * finish off the rest of the command, or start a new one.
+               /* A number of bytes were successfully read.  If there
+                * is leftovers and there is some kind of error
+                * (result != 0), retry the rest.
                 */
-               if (scsi_end_request(cmd, 1, good_bytes, result == 0) == NULL)
+               if (scsi_end_request(cmd, 1, good_bytes, !!result) == NULL)
                        return;
        }
-       /*
-        * Now, if we were good little boys and girls, Santa left us a request
-        * sense buffer.  We can extract information from this, so we
-        * can choose a block to remap, etc.
+
+       /* good_bytes = 0, or (inclusive) there were leftovers and
+        * result = 0, so scsi_end_request couldn't retry.
         */
        if (sense_valid && !sense_deferred) {
                switch (sshdr.sense_key) {
                case UNIT_ATTENTION:
                        if (cmd->device->removable) {
-                               /* detected disc change.  set a bit 
+                               /* Detected disc change.  Set a bit
                                 * and quietly refuse further access.
                                 */
                                cmd->device->changed = 1;
-                               scsi_end_request(cmd, 0,
-                                               this_count, 1);
+                               scsi_end_request(cmd, 0, this_count, 1);
                                return;
                        } else {
-                               /*
-                               * Must have been a power glitch, or a
-                               * bus reset.  Could not have been a
-                               * media change, so we just retry the
-                               * request and see what happens.  
-                               */
+                               /* Must have been a power glitch, or a
+                                * bus reset.  Could not have been a
+                                * media change, so we just retry the
+                                * request and see what happens.
+                                */
                                scsi_requeue_command(q, cmd);
                                return;
                        }
                        break;
                case ILLEGAL_REQUEST:
-                       /*
-                       * If we had an ILLEGAL REQUEST returned, then we may
-                       * have performed an unsupported command.  The only
-                       * thing this should be would be a ten byte read where
-                       * only a six byte read was supported.  Also, on a
-                       * system where READ CAPACITY failed, we may have read
-                       * past the end of the disk.
-                       */
+                       /* If we had an ILLEGAL REQUEST returned, then
+                        * we may have performed an unsupported
+                        * command.  The only thing this should be
+                        * would be a ten byte read where only a six
+                        * byte read was supported.  Also, on a system
+                        * where READ CAPACITY failed, we may have
+                        * read past the end of the disk.
+                        */
                        if ((cmd->device->use_10_for_rw &&
                            sshdr.asc == 0x20 && sshdr.ascq == 0x00) &&
                            (cmd->cmnd[0] == READ_10 ||
                             cmd->cmnd[0] == WRITE_10)) {
                                cmd->device->use_10_for_rw = 0;
-                               /*
-                                * This will cause a retry with a 6-byte
-                                * command.
+                               /* This will cause a retry with a
+                                * 6-byte command.
                                 */
                                scsi_requeue_command(q, cmd);
-                               result = 0;
+                               return;
                        } else {
                                scsi_end_request(cmd, 0, this_count, 1);
                                return;
                        }
                        break;
                case NOT_READY:
-                       /*
-                        * If the device is in the process of becoming
+                       /* If the device is in the process of becoming
                         * ready, or has a temporary blockage, retry.
                         */
                        if (sshdr.asc == 0x04) {
@@ -1021,7 +1005,7 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes,
                        }
                        if (!(req->flags & REQ_QUIET)) {
                                scmd_printk(KERN_INFO, cmd,
-                                          "Device not ready: ");
+                                           "Device not ready: ");
                                scsi_print_sense_hdr("", &sshdr);
                        }
                        scsi_end_request(cmd, 0, this_count, 1);
@@ -1029,21 +1013,21 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes,
                case VOLUME_OVERFLOW:
                        if (!(req->flags & REQ_QUIET)) {
                                scmd_printk(KERN_INFO, cmd,
-                                          "Volume overflow, CDB: ");
+                                           "Volume overflow, CDB: ");
                                __scsi_print_command(cmd->data_cmnd);
                                scsi_print_sense("", cmd);
                        }
-                       scsi_end_request(cmd, 0, block_bytes, 1);
+                       /* See SSC3rXX or current. */
+                       scsi_end_request(cmd, 0, this_count, 1);
                        return;
                default:
                        break;
                }
-       }                       /* driver byte != 0 */
+       }
        if (host_byte(result) == DID_RESET) {
-               /*
-                * Third party bus reset or reset for error
-                * recovery reasons.  Just retry the request
-                * and see what happens.  
+               /* Third party bus reset or reset for error recovery
+                * reasons.  Just retry the request and see what
+                * happens.
                 */
                scsi_requeue_command(q, cmd);
                return;
@@ -1051,21 +1035,13 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes,
        if (result) {
                if (!(req->flags & REQ_QUIET)) {
                        scmd_printk(KERN_INFO, cmd,
-                                  "SCSI error: return code = 0x%x\n", result);
-
+                                   "SCSI error: return code = 0x%08x\n",
+                                   result);
                        if (driver_byte(result) & DRIVER_SENSE)
                                scsi_print_sense("", cmd);
                }
-               /*
-                * Mark a single buffer as not uptodate.  Queue the remainder.
-                * We sometimes get this cruft in the event that a medium error
-                * isn't properly reported.
-                */
-               block_bytes = req->hard_cur_sectors << 9;
-               if (!block_bytes)
-                       block_bytes = req->data_len;
-               scsi_end_request(cmd, 0, block_bytes, 1);
        }
+       scsi_end_request(cmd, 0, this_count, !result);
 }
 EXPORT_SYMBOL(scsi_io_completion);
 
@@ -1169,7 +1145,7 @@ static void scsi_blk_pc_done(struct scsi_cmnd *cmd)
         * successfully. Since this is a REQ_BLOCK_PC command the
         * caller should check the request's errors value
         */
-       scsi_io_completion(cmd, cmd->bufflen, 0);
+       scsi_io_completion(cmd, cmd->bufflen);
 }
 
 static void scsi_setup_blk_pc_cmnd(struct scsi_cmnd *cmd)
index 354199011246633ce9cf8fcc172231648492d6ef..f899ff0cf0056de8e927be2e99cdc98e747c3914 100644 (file)
@@ -891,11 +891,10 @@ static struct block_device_operations sd_fops = {
 static void sd_rw_intr(struct scsi_cmnd * SCpnt)
 {
        int result = SCpnt->result;
-       int this_count = SCpnt->request_bufflen;
-       int good_bytes = (result == 0 ? this_count : 0);
-       sector_t block_sectors = 1;
-       u64 first_err_block;
-       sector_t error_sector;
+       unsigned int xfer_size = SCpnt->request_bufflen;
+       unsigned int good_bytes = result ? 0 : xfer_size;
+       u64 start_lba = SCpnt->request->sector;
+       u64 bad_lba;
        struct scsi_sense_hdr sshdr;
        int sense_valid = 0;
        int sense_deferred = 0;
@@ -906,7 +905,6 @@ static void sd_rw_intr(struct scsi_cmnd * SCpnt)
                if (sense_valid)
                        sense_deferred = scsi_sense_is_deferred(&sshdr);
        }
-
 #ifdef CONFIG_SCSI_LOGGING
        SCSI_LOG_HLCOMPLETE(1, printk("sd_rw_intr: %s: res=0x%x\n", 
                                SCpnt->request->rq_disk->disk_name, result));
@@ -916,89 +914,72 @@ static void sd_rw_intr(struct scsi_cmnd * SCpnt)
                                sshdr.sense_key, sshdr.asc, sshdr.ascq));
        }
 #endif
-       /*
-          Handle MEDIUM ERRORs that indicate partial success.  Since this is a
-          relatively rare error condition, no care is taken to avoid
-          unnecessary additional work such as memcpy's that could be avoided.
-        */
-       if (driver_byte(result) != 0 &&
-                sense_valid && !sense_deferred) {
-               switch (sshdr.sense_key) {
-               case MEDIUM_ERROR:
-                       if (!blk_fs_request(SCpnt->request))
-                               break;
-                       info_valid = scsi_get_sense_info_fld(
-                               SCpnt->sense_buffer, SCSI_SENSE_BUFFERSIZE,
-                               &first_err_block);
-                       /*
-                        * May want to warn and skip if following cast results
-                        * in actual truncation (if sector_t < 64 bits)
-                        */
-                       error_sector = (sector_t)first_err_block;
-                       if (SCpnt->request->bio != NULL)
-                               block_sectors = bio_sectors(SCpnt->request->bio);
-                       switch (SCpnt->device->sector_size) {
-                       case 1024:
-                               error_sector <<= 1;
-                               if (block_sectors < 2)
-                                       block_sectors = 2;
-                               break;
-                       case 2048:
-                               error_sector <<= 2;
-                               if (block_sectors < 4)
-                                       block_sectors = 4;
-                               break;
-                       case 4096:
-                               error_sector <<=3;
-                               if (block_sectors < 8)
-                                       block_sectors = 8;
-                               break;
-                       case 256:
-                               error_sector >>= 1;
-                               break;
-                       default:
-                               break;
-                       }
+       if (driver_byte(result) != DRIVER_SENSE &&
+           (!sense_valid || sense_deferred))
+               goto out;
 
-                       error_sector &= ~(block_sectors - 1);
-                       good_bytes = (error_sector - SCpnt->request->sector) << 9;
-                       if (good_bytes < 0 || good_bytes >= this_count)
-                               good_bytes = 0;
+       switch (sshdr.sense_key) {
+       case HARDWARE_ERROR:
+       case MEDIUM_ERROR:
+               if (!blk_fs_request(SCpnt->request))
+                       goto out;
+               info_valid = scsi_get_sense_info_fld(SCpnt->sense_buffer,
+                                                    SCSI_SENSE_BUFFERSIZE,
+                                                    &bad_lba);
+               if (!info_valid)
+                       goto out;
+               if (xfer_size <= SCpnt->device->sector_size)
+                       goto out;
+               switch (SCpnt->device->sector_size) {
+               case 256:
+                       start_lba <<= 1;
                        break;
-
-               case RECOVERED_ERROR: /* an error occurred, but it recovered */
-               case NO_SENSE: /* LLDD got sense data */
-                       /*
-                        * Inform the user, but make sure that it's not treated
-                        * as a hard error.
-                        */
-                       scsi_print_sense("sd", SCpnt);
-                       SCpnt->result = 0;
-                       memset(SCpnt->sense_buffer, 0, SCSI_SENSE_BUFFERSIZE);
-                       good_bytes = this_count;
+               case 512:
                        break;
-
-               case ILLEGAL_REQUEST:
-                       if (SCpnt->device->use_10_for_rw &&
-                           (SCpnt->cmnd[0] == READ_10 ||
-                            SCpnt->cmnd[0] == WRITE_10))
-                               SCpnt->device->use_10_for_rw = 0;
-                       if (SCpnt->device->use_10_for_ms &&
-                           (SCpnt->cmnd[0] == MODE_SENSE_10 ||
-                            SCpnt->cmnd[0] == MODE_SELECT_10))
-                               SCpnt->device->use_10_for_ms = 0;
+               case 1024:
+                       start_lba >>= 1;
+                       break;
+               case 2048:
+                       start_lba >>= 2;
+                       break;
+               case 4096:
+                       start_lba >>= 3;
                        break;
-
                default:
+                       /* Print something here with limiting frequency. */
+                       goto out;
                        break;
                }
+               /* This computation should always be done in terms of
+                * the resolution of the device's medium.
+                */
+               good_bytes = (bad_lba - start_lba)*SCpnt->device->sector_size;
+               break;
+       case RECOVERED_ERROR:
+       case NO_SENSE:
+               /* Inform the user, but make sure that it's not treated
+                * as a hard error.
+                */
+               scsi_print_sense("sd", SCpnt);
+               SCpnt->result = 0;
+               memset(SCpnt->sense_buffer, 0, SCSI_SENSE_BUFFERSIZE);
+               good_bytes = xfer_size;
+               break;
+       case ILLEGAL_REQUEST:
+               if (SCpnt->device->use_10_for_rw &&
+                   (SCpnt->cmnd[0] == READ_10 ||
+                    SCpnt->cmnd[0] == WRITE_10))
+                       SCpnt->device->use_10_for_rw = 0;
+               if (SCpnt->device->use_10_for_ms &&
+                   (SCpnt->cmnd[0] == MODE_SENSE_10 ||
+                    SCpnt->cmnd[0] == MODE_SELECT_10))
+                       SCpnt->device->use_10_for_ms = 0;
+               break;
+       default:
+               break;
        }
-       /*
-        * This calls the generic completion function, now that we know
-        * how many actual sectors finished, and how many sectors we need
-        * to say have failed.
-        */
-       scsi_io_completion(SCpnt, good_bytes, block_sectors << 9);
+ out:
+       scsi_io_completion(SCpnt, good_bytes);
 }
 
 static int media_not_present(struct scsi_disk *sdkp,
index ebf6579ed6985bb9e781b35bbc5e4d6daf85af33..fd94408577e5ae3aec19b5e7fa3657d14e641e22 100644 (file)
@@ -292,7 +292,7 @@ static void rw_intr(struct scsi_cmnd * SCpnt)
         * how many actual sectors finished, and how many sectors we need
         * to say have failed.
         */
-       scsi_io_completion(SCpnt, good_bytes, block_sectors << 9);
+       scsi_io_completion(SCpnt, good_bytes);
 }
 
 static int sr_init_command(struct scsi_cmnd * SCpnt)
index e46cd404bd7d2bdeffcbaba5e4147d53251d6efd..371f70d9aa92e993438a29a2981f182772f7478b 100644 (file)
@@ -143,7 +143,7 @@ struct scsi_cmnd {
 
 extern struct scsi_cmnd *scsi_get_command(struct scsi_device *, gfp_t);
 extern void scsi_put_command(struct scsi_cmnd *);
-extern void scsi_io_completion(struct scsi_cmnd *, unsigned int, unsigned int);
+extern void scsi_io_completion(struct scsi_cmnd *, unsigned int);
 extern void scsi_finish_command(struct scsi_cmnd *cmd);
 extern void scsi_req_abort_cmd(struct scsi_cmnd *cmd);