From bc949a71195e7764834d4511f8d6e16aca9d5113 Mon Sep 17 00:00:00 2001 From: Mike Christie Date: Thu, 14 Aug 2008 21:13:58 -0500 Subject: [PATCH 11/23] libfc: make sure we call exch_done when retrying commands. If fc_exch.c calls the resp function with -FC_EX_CLOSED then for FCP commands we must clean up the command by calling exch_done. Today we are calling the abort code, but if the abort response is lost then we will never clean up the command due to a bug in fc_exch.c that will be fixed later (well two bugs, one casued by the abort patches and one that was there before). Also it seems like if lp->state == LPORT_ST_LOGO then we want to handle errors that are returned from fc_exch.c. For at least -FC_EX_CLOSED we must to be able to clean up commands. For other errors it does not look like it hurts. As far as aborting the command vs just cleaning up. For the scsi eh path we have already tried an abort so we get to this point because of a port reset. If this is called for some other reason, I was not sure if we needed to abort the command or not. It looks like in fcp4 4.10 table 4 if we do a reset/LIP or LOGO/PLOGO or PRLI/PRLO then those terminate the exchanges and no abort is needed. I was not sure if we always do one of those in every place this the exch mgmr reset function can be called. For example in the rmmod case I do not think we do, so the resources may not get cleaned up, but that may be a corner case that will be fixed later when shutdown is fixed. And when we relogin eventually those resources would get cleaned up then right? So maybe it does not matter at all. I also merged the fc_fcp_retry and fc_fcp_retry_cmd calls, because fc_fcp_retry needs to cleanup the fc_exch.c resrouces. And all commands in this path are then retried with DID_IMM_RETRY, so we we retry until the transport class fails the command or until the scsi command timer fires. Signed-off-by: Mike Christie --- drivers/scsi/libfc/fc_fcp.c | 89 +++++++++++++++++-------------------------- 1 files changed, 35 insertions(+), 54 deletions(-) diff --git a/drivers/scsi/libfc/fc_fcp.c b/drivers/scsi/libfc/fc_fcp.c index 51064ad..c519b43 100644 --- a/drivers/scsi/libfc/fc_fcp.c +++ b/drivers/scsi/libfc/fc_fcp.c @@ -143,7 +143,6 @@ static void fc_fcp_complete(struct fc_fcp_pkt *); static void fc_tm_done(struct fc_seq *, struct fc_frame *, void *); static void fc_fcp_error(struct fc_fcp_pkt *fsp, struct fc_frame *fp); static void fc_timeout_error(struct fc_fcp_pkt *); -static void fc_fcp_retry_cmd(struct fc_fcp_pkt *); static int fc_fcp_send_cmd(struct fc_fcp_pkt *); static void fc_fcp_timeout(unsigned long data); static void fc_fcp_rec(struct fc_fcp_pkt *); @@ -270,16 +269,6 @@ static void fc_fcp_timer_set(struct fc_fcp_pkt *fsp, unsigned long delay) mod_timer(&fsp->timer, jiffies + delay); } -/* - * End a request with a retry suggestion. - */ -static void fc_fcp_retry(struct fc_fcp_pkt *fsp) -{ - fsp->status_code = FC_ERROR; - fsp->io_status = SUGGEST_RETRY << 24; - fc_fcp_complete(fsp); -} - static int fc_fcp_send_abort(struct fc_fcp_pkt *fsp) { if (!fsp->seq_ptr) @@ -290,6 +279,23 @@ static int fc_fcp_send_abort(struct fc_fcp_pkt *fsp) } /* + * Retry command. + * An abort isn't needed. + */ +static void fc_fcp_retry_cmd(struct fc_fcp_pkt *fsp) +{ + if (fsp->seq_ptr) { + fsp->lp->tt.exch_done(fsp->seq_ptr); + fsp->seq_ptr = NULL; + } + + fsp->state &= ~FC_SRB_ABORT_PENDING; + fsp->io_status = SUGGEST_RETRY << 24; + fsp->status_code = FC_ERROR; + fc_fcp_complete(fsp); +} + +/* * Receive SCSI data from target. * Called after receiving solicited data. */ @@ -326,7 +332,7 @@ static void fc_fcp_recv_data(struct fc_fcp_pkt *fsp, struct fc_frame *fp) "len %zx offset %zx " "data_len %x\n", len, offset, fsp->data_len); } - fc_fcp_retry(fsp); + fc_fcp_retry_cmd(fsp); return; } if (offset != fsp->xfer_len) @@ -396,7 +402,7 @@ crc_err: * Otherwise, ignore it. */ if (fsp->state & FC_SRB_DISCONTIG) - fc_fcp_retry(fsp); + fc_fcp_retry_cmd(fsp); return; } } @@ -581,7 +587,7 @@ static void fc_fcp_send_data(struct fc_fcp_pkt *fsp, struct fc_seq *sp, if (error) { WARN_ON(1); /* send error should be rare */ - fc_fcp_retry(fsp); + fc_fcp_retry_cmd(fsp); return; } } @@ -1007,16 +1013,20 @@ retry: */ static void fc_fcp_error(struct fc_fcp_pkt *fsp, struct fc_frame *fp) { - struct fc_lport *lp = fsp->lp; - - if (lp->state == LPORT_ST_LOGO) - return; + int error = PTR_ERR(fp); if (fc_fcp_lock_pkt(fsp)) return; - /* exch layer decided to abort exchange - will wait for response */ - if (PTR_ERR(fp) == -FC_EX_TIMEOUT) { + switch (error) { + case -FC_EX_CLOSED: + fc_fcp_retry_cmd(fsp); + goto unlock; + case -FC_EX_TIMEOUT: + /* + * exch layer decided to abort exchange - + * will wait for response + */ fsp->state |= FC_SRB_ABORT_PENDING; goto unlock; } @@ -1483,15 +1493,12 @@ static void fc_fcp_rec_error(struct fc_fcp_pkt *fsp, struct fc_frame *fp) struct fc_lport *lp = fsp->lp; int error = PTR_ERR(fp); - if (lp->state == LPORT_ST_LOGO) - return; - if (fc_fcp_lock_pkt(fsp)) goto out; switch (error) { case -FC_EX_CLOSED: - fc_timeout_error(fsp); + fc_fcp_retry_cmd(fsp); break; default: @@ -1537,31 +1544,6 @@ static void fc_timeout_error(struct fc_fcp_pkt *fsp) } /* - * Retry command. - * An abort isn't needed. - * - * We treat it like a timeout because the command did not complete - - * presumably due to cmd packet loss. We will fail the command and - * have scsi-ml decide if we should retry or not. - * - * TODO: Instead we could continue to retry the command until the scsi - * command fires, or add port level counters to determine - * when to mark it as failed (the latter would be useful in the class eh - * for lpfc and qla2xxx). - * - */ -static void fc_fcp_retry_cmd(struct fc_fcp_pkt *fsp) -{ - if (fsp->seq_ptr) { - fsp->lp->tt.exch_done(fsp->seq_ptr); - fsp->seq_ptr = NULL; - } - - fsp->status_code = FC_CMD_TIME_OUT; - fc_fcp_complete(fsp); -} - -/* * Sequence retransmission request. * This is called after receiving status but insufficient data, or * when expecting status but the request has timed out. @@ -1616,7 +1598,7 @@ static void fc_fcp_srr(struct fc_fcp_pkt *fsp, enum fc_rctl r_ctl, u32 offset) fc_fcp_pkt_hold(fsp); /* hold for outstanding SRR */ return; retry: - fc_fcp_retry(fsp); + fc_fcp_retry_cmd(fsp); } /* @@ -1677,17 +1659,16 @@ static void fc_fcp_srr_error(struct fc_fcp_pkt *fsp, struct fc_frame *fp) fsp->lp->tt.exch_done(fsp->recov_seq); fsp->recov_seq = NULL; switch (PTR_ERR(fp)) { - case -FC_EX_CLOSED: /* e.g., link failure */ - fc_timeout_error(fsp); - break; case -FC_EX_TIMEOUT: if (fsp->recov_retry++ < FC_MAX_RECOV_RETRY) fc_fcp_rec(fsp); else fc_timeout_error(fsp); break; + case -FC_EX_CLOSED: /* e.g., link failure */ + /* fall through */ default: - fc_fcp_retry(fsp); + fc_fcp_retry_cmd(fsp); break; } fc_fcp_unlock_pkt(fsp); -- 1.5.5.1