Commit b4e93a73 authored by Matthew Wilcox's avatar Matthew Wilcox Committed by James Bottomley

[SCSI] Simplify error handling

Use wait_for_completion_timeout() instead of using a timer (as
Christoph Hellwig did for aic7xxx).

That lets me eliminate the sym_eh_wait structure; the struct completion,
the old_done pointer and the to_do flag can be folded into the sym_ucmd
(which overrides the scsi_pointer in scsi_cmnd).

The sym_eh_done() function becomes much simpler as the timeout handling
is done in sym_eh_handler() directly.

The host_lock can be unlocked earlier, and I cache the host in
a local variable to make accesses to it quicker.
Signed-off-by: default avatarMatthew Wilcox <matthew@wil.cx>
Signed-off-by: default avatarJames Bottomley <James.Bottomley@SteelEye.com>
parent c2349df9
...@@ -136,25 +136,15 @@ static void sym2_setup_params(void) ...@@ -136,25 +136,15 @@ static void sym2_setup_params(void)
static struct scsi_transport_template *sym2_transport_template = NULL; static struct scsi_transport_template *sym2_transport_template = NULL;
/*
* Used by the eh thread to wait for command completion.
* It is allocated on the eh thread stack.
*/
struct sym_eh_wait {
struct completion done;
struct timer_list timer;
void (*old_done)(struct scsi_cmnd *);
int to_do;
int timed_out;
};
/* /*
* Driver private area in the SCSI command structure. * Driver private area in the SCSI command structure.
*/ */
struct sym_ucmd { /* Override the SCSI pointer structure */ struct sym_ucmd { /* Override the SCSI pointer structure */
struct completion done;
void (*old_done)(struct scsi_cmnd *);
dma_addr_t data_mapping; dma_addr_t data_mapping;
u_char data_mapped; int to_do;
struct sym_eh_wait *eh_wait; u_char data_mapped; /* corresponds to data_mapping above */
}; };
#define SYM_UCMD_PTR(cmd) ((struct sym_ucmd *)(&(cmd)->SCp)) #define SYM_UCMD_PTR(cmd) ((struct sym_ucmd *)(&(cmd)->SCp))
...@@ -713,40 +703,19 @@ static void sym53c8xx_timer(unsigned long npref) ...@@ -713,40 +703,19 @@ static void sym53c8xx_timer(unsigned long npref)
#define SYM_EH_DO_WAIT 2 #define SYM_EH_DO_WAIT 2
/* /*
* Our general completion handler. * scsi_done() alias when error recovery is in progress.
*/ */
static void __sym_eh_done(struct scsi_cmnd *cmd, int timed_out) static void sym_eh_done(struct scsi_cmnd *cmd)
{ {
struct sym_eh_wait *ep = SYM_UCMD_PTR(cmd)->eh_wait; struct sym_ucmd *ucmd = SYM_UCMD_PTR(cmd);
if (!ep) BUILD_BUG_ON(sizeof(struct scsi_pointer) < sizeof(struct sym_ucmd));
return;
/* Try to avoid a race here (not 100% safe) */ cmd->scsi_done = ucmd->old_done;
if (!timed_out) {
ep->timed_out = 0;
if (ep->to_do == SYM_EH_DO_WAIT && !del_timer(&ep->timer))
return;
}
/* Revert everything */
SYM_UCMD_PTR(cmd)->eh_wait = NULL;
cmd->scsi_done = ep->old_done;
/* Wake up the eh thread if it wants to sleep */ if (ucmd->to_do == SYM_EH_DO_WAIT)
if (ep->to_do == SYM_EH_DO_WAIT) complete(&ucmd->done);
complete(&ep->done);
} }
/*
* scsi_done() alias when error recovery is in progress.
*/
static void sym_eh_done(struct scsi_cmnd *cmd) { __sym_eh_done(cmd, 0); }
/*
* Some timeout handler to avoid waiting too long.
*/
static void sym_eh_timeout(u_long p) { __sym_eh_done((struct scsi_cmnd *)p, 1); }
/* /*
* Generic method for our eh processing. * Generic method for our eh processing.
* The 'op' argument tells what we have to do. * The 'op' argument tells what we have to do.
...@@ -754,14 +723,15 @@ static void sym_eh_timeout(u_long p) { __sym_eh_done((struct scsi_cmnd *)p, 1); ...@@ -754,14 +723,15 @@ static void sym_eh_timeout(u_long p) { __sym_eh_done((struct scsi_cmnd *)p, 1);
static int sym_eh_handler(int op, char *opname, struct scsi_cmnd *cmd) static int sym_eh_handler(int op, char *opname, struct scsi_cmnd *cmd)
{ {
struct sym_hcb *np = SYM_SOFTC_PTR(cmd); struct sym_hcb *np = SYM_SOFTC_PTR(cmd);
struct sym_ucmd *ucmd = SYM_UCMD_PTR(cmd);
struct Scsi_Host *host = cmd->device->host;
SYM_QUEHEAD *qp; SYM_QUEHEAD *qp;
int to_do = SYM_EH_DO_IGNORE; int to_do = SYM_EH_DO_IGNORE;
int sts = -1; int sts = -1;
struct sym_eh_wait eh, *ep = &eh;
dev_warn(&cmd->device->sdev_gendev, "%s operation started.\n", opname); dev_warn(&cmd->device->sdev_gendev, "%s operation started.\n", opname);
spin_lock_irq(cmd->device->host->host_lock); spin_lock_irq(host->host_lock);
/* This one is queued in some place -> to wait for completion */ /* This one is queued in some place -> to wait for completion */
FOR_EACH_QUEUED_ELEMENT(&np->busy_ccbq, qp) { FOR_EACH_QUEUED_ELEMENT(&np->busy_ccbq, qp) {
struct sym_ccb *cp = sym_que_entry(qp, struct sym_ccb, link_ccbq); struct sym_ccb *cp = sym_que_entry(qp, struct sym_ccb, link_ccbq);
...@@ -772,10 +742,9 @@ static int sym_eh_handler(int op, char *opname, struct scsi_cmnd *cmd) ...@@ -772,10 +742,9 @@ static int sym_eh_handler(int op, char *opname, struct scsi_cmnd *cmd)
} }
if (to_do == SYM_EH_DO_WAIT) { if (to_do == SYM_EH_DO_WAIT) {
init_completion(&ep->done); init_completion(&ucmd->done);
ep->old_done = cmd->scsi_done; ucmd->old_done = cmd->scsi_done;
cmd->scsi_done = sym_eh_done; cmd->scsi_done = sym_eh_done;
SYM_UCMD_PTR(cmd)->eh_wait = ep;
} }
/* Try to proceed the operation we have been asked for */ /* Try to proceed the operation we have been asked for */
...@@ -802,28 +771,19 @@ static int sym_eh_handler(int op, char *opname, struct scsi_cmnd *cmd) ...@@ -802,28 +771,19 @@ static int sym_eh_handler(int op, char *opname, struct scsi_cmnd *cmd)
/* On error, restore everything and cross fingers :) */ /* On error, restore everything and cross fingers :) */
if (sts) { if (sts) {
SYM_UCMD_PTR(cmd)->eh_wait = NULL; cmd->scsi_done = ucmd->old_done;
cmd->scsi_done = ep->old_done;
to_do = SYM_EH_DO_IGNORE; to_do = SYM_EH_DO_IGNORE;
} }
ep->to_do = to_do; ucmd->to_do = to_do;
spin_unlock_irq(host->host_lock);
/* Wait for completion with locks released, as required by kernel */
if (to_do == SYM_EH_DO_WAIT) { if (to_do == SYM_EH_DO_WAIT) {
init_timer(&ep->timer); if (!wait_for_completion_timeout(&ucmd->done, 5*HZ)) {
ep->timer.expires = jiffies + (5*HZ); ucmd->to_do = SYM_EH_DO_IGNORE;
ep->timer.function = sym_eh_timeout;
ep->timer.data = (u_long)cmd;
ep->timed_out = 1; /* Be pessimistic for once :) */
add_timer(&ep->timer);
spin_unlock_irq(np->s.host->host_lock);
wait_for_completion(&ep->done);
spin_lock_irq(np->s.host->host_lock);
if (ep->timed_out)
sts = -2; sts = -2;
} }
spin_unlock_irq(cmd->device->host->host_lock); }
dev_warn(&cmd->device->sdev_gendev, "%s operation %s.\n", opname, dev_warn(&cmd->device->sdev_gendev, "%s operation %s.\n", opname,
sts==0 ? "complete" :sts==-2 ? "timed-out" : "failed"); sts==0 ? "complete" :sts==-2 ? "timed-out" : "failed");
return sts ? SCSI_FAILED : SCSI_SUCCESS; return sts ? SCSI_FAILED : SCSI_SUCCESS;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment