From 7dfdc9a52b4219fba8240750e36de5db860ddd5f Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Mon, 31 Oct 2005 18:49:52 +0100 Subject: [PATCH] [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 Signed-off-by: James Bottomley --- drivers/scsi/scsi_error.c | 107 +++++++++++--------------------------- drivers/scsi/scsi_priv.h | 1 - include/scsi/scsi_host.h | 5 +- 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 @@ -416,44 +416,16 @@ static int scsi_eh_completed_normally(struct scsi_cmnd *scmd) return FAILED; } -/** - * 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; + scsi_log_completion(scmd, SUCCESS); - /* - * 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_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 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;