[SCSI] use a completion in scsi_send_eh_cmnd

scsi_send_eh_cmnd currently uses a semaphore and an overload of eh_timer
to either get a completion for a command for a timeout.
Switch to using a completion and wait_for_completion_timeout to simply
the code and not having to deal with the races ourselves.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: James Bottomley <James.Bottomley@SteelEye.com>

3 files changed, 32 insertions, 81 deletions

diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index 5a30485d5038..18c5d2523014 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -417,43 +417,15 @@ static int scsi_eh_completed_normally(struct scsi_cmnd *scmd)
 }
 
 /**
- * scsi_eh_times_out - timeout function for error handling.
- * @scmd:       Cmd that is timing out.
- *
- * Notes:
- *    During error handling, the kernel thread will be sleeping waiting
- *    for some action to complete on the device.  our only job is to
- *    record that it timed out, and to wake up the thread.
- **/
-static void scsi_eh_times_out(struct scsi_cmnd *scmd)
-{
-        scmd->eh_eflags |= SCSI_EH_REC_TIMEOUT;
-        SCSI_LOG_ERROR_RECOVERY(3, printk("%s: scmd:%p\n", __FUNCTION__,
-                                          scmd));
-
-        up(scmd->device->host->eh_action);
-}
-
-/**
  * scsi_eh_done - Completion function for error handling.
  * @scmd:       Cmd that is done.
  **/
 static void scsi_eh_done(struct scsi_cmnd *scmd)
 {
-        /*
-         * if the timeout handler is already running, then just set the
-         * flag which says we finished late, and return.  we have no
-         * way of stopping the timeout handler from running, so we must
-         * always defer to it.
-         */
-        if (del_timer(&scmd->eh_timeout)) {
-                scmd->request->rq_status = RQ_SCSI_DONE;
-
-                SCSI_LOG_ERROR_RECOVERY(3, printk("%s scmd: %p result: %x\n",
-                                           __FUNCTION__, scmd, scmd->result));
-
-                up(scmd->device->host->eh_action);
-        }
+        SCSI_LOG_ERROR_RECOVERY(3,
+                printk("%s scmd: %p result: %x\n",
+                        __FUNCTION__, scmd, scmd->result));
+        complete(scmd->device->host->eh_action);
 }
 
 /**
@@ -461,10 +433,6 @@ static void scsi_eh_done(struct scsi_cmnd *scmd)
  * @scmd:       SCSI Cmd to send.
  * @timeout:    Timeout for cmd.
  *
- * Notes:
- *    The initialization of the structures is quite a bit different in
- *    this case, and furthermore, there is a different completion handler
- *    vs scsi_dispatch_cmd.
  * Return value:
  *    SUCCESS or FAILED or NEEDS_RETRY
  **/
@@ -472,24 +440,16 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, int timeout)
 {
         struct scsi_device *sdev = scmd->device;
         struct Scsi_Host *shost = sdev->host;
-        DECLARE_MUTEX_LOCKED(sem);
+        DECLARE_COMPLETION(done);
+        unsigned long timeleft;
         unsigned long flags;
-        int rtn = SUCCESS;
+        int rtn;
 
-        /*
-         * we will use a queued command if possible, otherwise we will
-         * emulate the queuing and calling of completion function ourselves.
-         */
         if (sdev->scsi_level <= SCSI_2)
                 scmd->cmnd[1] = (scmd->cmnd[1] & 0x1f) |
                         (sdev->lun << 5 & 0xe0);
 
-        scsi_add_timer(scmd, timeout, scsi_eh_times_out);
-
-        /*
-         * set up the semaphore so we wait for the command to complete.
-         */
-        shost->eh_action = &sem;
+        shost->eh_action = &done;
         scmd->request->rq_status = RQ_SCSI_BUSY;
 
         spin_lock_irqsave(shost->host_lock, flags);
@@ -497,47 +457,29 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, int timeout)
         shost->hostt->queuecommand(scmd, scsi_eh_done);
         spin_unlock_irqrestore(shost->host_lock, flags);
 
-        down(&sem);
-        scsi_log_completion(scmd, SUCCESS);
+        timeleft = wait_for_completion_timeout(&done, timeout);
 
+        scmd->request->rq_status = RQ_SCSI_DONE;
         shost->eh_action = NULL;
 
-        /*
-         * see if timeout.  if so, tell the host to forget about it.
-         * in other words, we don't want a callback any more.
-         */
-        if (scmd->eh_eflags & SCSI_EH_REC_TIMEOUT) {
-                scmd->eh_eflags &= ~SCSI_EH_REC_TIMEOUT;
-
-                /*
-                 * as far as the low level driver is
-                 * concerned, this command is still active, so
-                 * we must give the low level driver a chance
-                 * to abort it. (db) 
-                 *
-                 * FIXME(eric) - we are not tracking whether we could
-                 * abort a timed out command or not.  not sure how
-                 * we should treat them differently anyways.
-                 */
-                if (shost->hostt->eh_abort_handler)
-                        shost->hostt->eh_abort_handler(scmd);
-                        
-                scmd->request->rq_status = RQ_SCSI_DONE;
-                rtn = FAILED;
-        }
+        scsi_log_completion(scmd, SUCCESS);
 
-        SCSI_LOG_ERROR_RECOVERY(3, printk("%s: scmd: %p, rtn:%x\n",
-                                          __FUNCTION__, scmd, rtn));
+        SCSI_LOG_ERROR_RECOVERY(3,
+                printk("%s: scmd: %p, timeleft: %ld\n",
+                        __FUNCTION__, scmd, timeleft));
 
         /*
-         * now examine the actual status codes to see whether the command
-         * actually did complete normally.
+         * If there is time left scsi_eh_done got called, and we will
+         * examine the actual status codes to see whether the command
+         * actually did complete normally, else tell the host to forget
+         * about this command.
          */
-        if (rtn == SUCCESS) {
+        if (timeleft) {
                 rtn = scsi_eh_completed_normally(scmd);
                 SCSI_LOG_ERROR_RECOVERY(3,
                         printk("%s: scsi_eh_completed_normally %x\n",
                                __FUNCTION__, rtn));
+
                 switch (rtn) {
                 case SUCCESS:
                 case NEEDS_RETRY:
@@ -547,6 +489,15 @@ static int scsi_send_eh_cmnd(struct scsi_cmnd *scmd, int timeout)
                         rtn = FAILED;
                         break;
                 }
+        } else {
+                /*
+                 * FIXME(eric) - we are not tracking whether we could
+                 * abort a timed out command or not.  not sure how
+                 * we should treat them differently anyways.
+                 */
+                if (shost->hostt->eh_abort_handler)
+                        shost->hostt->eh_abort_handler(scmd);
+                rtn = FAILED;
         }
 
         return rtn;
diff --git a/drivers/scsi/scsi_priv.h b/drivers/scsi/scsi_priv.h
index d05f778d31a8..d632d9e1493c 100644
--- a/drivers/scsi/scsi_priv.h
+++ b/drivers/scsi/scsi_priv.h
@@ -22,7 +22,6 @@ struct Scsi_Host;
  * Scsi Error Handler Flags
  */
 #define SCSI_EH_CANCEL_CMD      0x0001  /* Cancel this cmd */
-#define SCSI_EH_REC_TIMEOUT     0x0002  /* EH retry timed out */
 
 #define SCSI_SENSE_VALID(scmd) \
         (((scmd)->sense_buffer[0] & 0x70) == 0x70)
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 9984d3fbb1f0..6cbb1982ed03 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -7,6 +7,7 @@
 #include <linux/workqueue.h>
 
 struct block_device;
+struct completion;
 struct module;
 struct scsi_cmnd;
 struct scsi_device;
@@ -467,8 +468,8 @@ struct Scsi_Host {
 
         struct list_head        eh_cmd_q;
         struct task_struct    * ehandler;  /* Error recovery thread. */
-        struct semaphore      * eh_action; /* Wait for specific actions on the
-                                          host. */
+        struct completion     * eh_action; /* Wait for specific actions on the
+                                              host. */
         wait_queue_head_t       host_wait;
         struct scsi_host_template *hostt;
         struct scsi_transport_template *transportt;