From dac00c0b83a1f60b01b281ac22567621521469e5 Mon Sep 17 00:00:00 2001 From: Mike Christie Date: Thu, 14 Aug 2008 12:50:55 -0500 Subject: [PATCH 08/23] libfc: have fc_exch notify fc_fcp of abort responses This has fc_exch.c report abort responses if the ep was for a FCP command. I did all the handling for device data frames in fc_fcp.c and did not add any new refcounting, so we will need Vasu's synchronized done calls in the fc_fcp_recv->fc_fcp_complete->exch_done path if the command was abnormally completed (also need them in the lun reset and other eh paths). I was not able to test the abts response handling paths yet. For some reason the target I am using is not sending responses. I have been trying to hack the stgt fcoe target to get difference values, but I am not done yet. This patch definately needs more testing and a good review. One issue I stumbled with was when fc_exch.c decided to abort the exchange for a scsi command. In this case are we still supposed to drop the device_data frames? I coded it so we would. Signed-off-by: Mike Christie --- drivers/scsi/libfc/fc_exch.c | 41 ++++++++++--- drivers/scsi/libfc/fc_fcp.c | 136 +++++++++++++++++++++++++++--------------- 2 files changed, 121 insertions(+), 56 deletions(-) diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c index 71682bd..41bbe4b 100644 --- a/drivers/scsi/libfc/fc_exch.c +++ b/drivers/scsi/libfc/fc_exch.c @@ -85,6 +85,7 @@ struct fc_exch { u32 r_a_tov; /* r_a_tov from rport (msec) */ u8 seq_id; /* next sequence ID to use */ u32 f_ctl; /* F_CTL flags for sequences */ + u8 fh_type; /* frame type */ enum fc_class class; /* class of service */ struct fc_seq seq; /* single sequence */ @@ -389,7 +390,6 @@ int fc_seq_exch_abort(const struct fc_seq *req_sp) error = fc_seq_send(ep->lp, sp, fp, FC_FC_END_SEQ); } else error = -ENOBUFS; - return error; } EXPORT_SYMBOL(fc_seq_exch_abort); @@ -418,13 +418,17 @@ static void fc_exch_timeout(unsigned long ep_arg) spin_unlock_bh(&ep->ex_lock); } else { resp = ep->resp; - ep->resp = NULL; arg = ep->resp_arg; + /* + * For FCP commands fc_fcp.c will manage the completion. + * For all others fc_exch.c will handle. + */ + if (ep->fh_type != FC_TYPE_FCP) + ep->resp = NULL; spin_unlock_bh(&ep->ex_lock); - fc_seq_exch_abort(sp); - if (resp) resp(sp, ERR_PTR(-FC_EX_TIMEOUT), arg); + fc_seq_exch_abort(sp); } /* @@ -1266,8 +1270,11 @@ static void fc_exch_recv_resp(struct fc_exch_mgr *mp, struct fc_frame *fp) */ static void fc_exch_abts_resp(struct fc_exch *ep, struct fc_frame *fp) { + void (*resp)(struct fc_seq *, struct fc_frame *fp, void *arg); + void *ex_resp_arg; struct fc_frame_header *fh; struct fc_ba_acc *ap; + struct fc_seq *sp; u16 low; u16 high; @@ -1302,9 +1309,24 @@ static void fc_exch_abts_resp(struct fc_exch *ep, struct fc_frame *fp) default: break; } - if (ntoh24(fh->fh_f_ctl) & FC_FC_LAST_SEQ) + + resp = ep->resp; + ex_resp_arg = ep->resp_arg; + + /* do we need to do some other checks here. Can we reuse more of + * fc_exch_recv_seq_resp + */ + sp = &ep->seq; + /* + * do we want to check END_SEQ as well as LAST_SEQ here? + */ + if (fh->fh_type != FC_TYPE_FCP && ntoh24(fh->fh_f_ctl) & FC_FC_LAST_SEQ) fc_exch_complete_locked(ep); spin_unlock_bh(&ep->ex_lock); + + if (resp) + resp(sp, fp, ex_resp_arg); + fc_frame_free(fp); } @@ -1774,9 +1796,10 @@ void fc_exch_done(struct fc_seq *sp) spin_lock_bh(&ep->ex_lock); ep->esb_stat |= ESB_ST_COMPLETE; ep->resp = NULL; - if (del_timer(&ep->ex_timer)) - atomic_dec(&ep->ex_refcnt); /* drop hold for timer */ - + if (!(ep->esb_stat & ESB_ST_REC_QUAL)) { + if (del_timer(&ep->ex_timer)) + atomic_dec(&ep->ex_refcnt); /* drop hold for timer */ + } spin_unlock_bh(&ep->ex_lock); fc_exch_release(fc_seq_exch(sp)); } @@ -1852,6 +1875,8 @@ struct fc_seq *fc_exch_seq_send(struct fc_lport *lp, fc_exch_timer_set_locked(ep, timer_msec); sp->f_ctl = f_ctl; /* save for possible abort */ ep->f_ctl &= ~FC_FC_FIRST_SEQ; /* not first seq */ + ep->fh_type = fh->fh_type; /* save for possbile timeout handling */ + if (f_ctl & FC_FC_SEQ_INIT) ep->esb_stat &= ~ESB_ST_SEQ_INIT; spin_unlock_bh(&ep->ex_lock); diff --git a/drivers/scsi/libfc/fc_fcp.c b/drivers/scsi/libfc/fc_fcp.c index c34f67e..9bb0ed4 100644 --- a/drivers/scsi/libfc/fc_fcp.c +++ b/drivers/scsi/libfc/fc_fcp.c @@ -142,7 +142,6 @@ static void fc_fcp_resp(struct fc_fcp_pkt *, struct fc_frame *); 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_abort_internal(struct fc_fcp_pkt *); 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 *); @@ -286,6 +285,15 @@ static void fc_fcp_retry(struct fc_fcp_pkt *fsp) fc_fcp_complete(fsp); } +static int fc_fcp_send_abort(struct fc_fcp_pkt *fsp) +{ + if (!fsp->seq_ptr) + return -EINVAL; + + fsp->state |= FC_SRB_ABORT_PENDING; + return fsp->lp->tt.seq_exch_abort(fsp->seq_ptr); +} + /* * Receive SCSI data from target. * Called after receiving solicited data. @@ -442,7 +450,7 @@ static void fc_fcp_send_data(struct fc_fcp_pkt *fsp, struct fc_seq *sp, FC_DBG("xfer-ready past end. len %zx offset %zx\n", len, offset); } - fc_abort_internal(fsp); + fc_fcp_send_abort(fsp); return; } else if (offset != fsp->xfer_len) { /* @@ -585,6 +593,24 @@ static void fc_fcp_send_data(struct fc_fcp_pkt *fsp, struct fc_seq *sp, fsp->xfer_len += len; /* premature count? */ } +static void fc_fcp_abts_resp(struct fc_fcp_pkt *fsp, struct fc_frame_header *fh) +{ + /* + * we will let the command timeout and scsi-ml escalate if + * the abort was rejected + */ + if (fh->fh_r_ctl == FC_RCTL_BA_ACC) { + fsp->state |= FC_SRB_ABORTED; + fsp->state &= ~FC_SRB_ABORT_PENDING; + fsp->status_code = FC_CMD_ABORTED; + + if (fsp->wait_for_comp) + complete(&fsp->tm_done); + else + fc_fcp_complete(fsp); + } +} + /* * exch mgr calls this routine to process scsi * exchanges. @@ -614,7 +640,12 @@ static void fc_fcp_recv(struct fc_seq *sp, struct fc_frame *fp, void *arg) goto out; fsp->last_pkt_time = jiffies; - if (fsp->state & FC_SRB_ABORT_PENDING) + if (fh->fh_type == FC_TYPE_BLS) { + fc_fcp_abts_resp(fsp, fh); + goto unlock; + } + + if (fsp->state & (FC_SRB_ABORTED | FC_SRB_ABORT_PENDING)) goto unlock; if (r_ctl == FC_RCTL_DD_DATA_DESC) { @@ -777,7 +808,10 @@ static void fc_fcp_complete(struct fc_fcp_pkt *fsp) struct fc_seq *sp; u32 f_ctl; - if (fsp->state & (FC_SRB_ABORT_PENDING | FC_SRB_ABORTED)) + if (fsp->state & FC_SRB_ABORT_PENDING) + return; + + if (fsp->state & FC_SRB_ABORTED) fsp->status_code = FC_CMD_ABORTED; else { /* @@ -986,22 +1020,24 @@ static void fc_fcp_error(struct fc_fcp_pkt *fsp, struct fc_frame *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) { + fsp->state |= FC_SRB_ABORT_PENDING; + goto unlock; + } + FC_DBG("unknown error %ld\n", PTR_ERR(fp)); + /* + * clear abort pending, because the lower layer + * decided to force completion. + */ + fsp->state &= ~FC_SRB_ABORT_PENDING; fsp->status_code = FC_CMD_PLOGO; fc_fcp_complete(fsp); +unlock: fc_fcp_unlock_pkt(fsp); } -static void fc_abort_internal(struct fc_fcp_pkt *fsp) -{ - fsp->state |= FC_SRB_ABORT_PENDING; - fsp->cdb_status = -1; - if (fsp->lp->tt.seq_exch_abort(fsp->seq_ptr)) - fc_fcp_complete(fsp); /* abort couldn't be sent */ - else - fsp->seq_ptr = NULL; -} - /* * Scsi abort handler- calls to send an abort * and then wait for abort completion @@ -1010,12 +1046,8 @@ static int fc_fcp_pkt_abort(struct fc_lport *lp, struct fc_fcp_pkt *fsp) { int rc = FAILED; - if (!fsp->seq_ptr) - return rc; - if (lp->tt.seq_exch_abort(fsp->seq_ptr)) - return rc; - - fsp->state |= FC_SRB_ABORT_PENDING; + if (fc_fcp_send_abort(fsp)) + return FAILED; init_completion(&fsp->tm_done); fsp->wait_for_comp = 1; @@ -1023,11 +1055,7 @@ static int fc_fcp_pkt_abort(struct fc_lport *lp, struct fc_fcp_pkt *fsp) spin_unlock_bh(&fsp->scsi_pkt_lock); rc = wait_for_completion_timeout(&fsp->tm_done, FC_SCSI_TM_TOV); spin_lock_bh(&fsp->scsi_pkt_lock); - - if (fsp->seq_ptr) { - lp->tt.exch_done(fsp->seq_ptr); - fsp->seq_ptr = NULL; - } + fsp->wait_for_comp = 0; if (!rc) { FC_DBG("target abort cmd failed\n"); @@ -1035,9 +1063,7 @@ static int fc_fcp_pkt_abort(struct fc_lport *lp, struct fc_fcp_pkt *fsp) } else if (fsp->state & FC_SRB_ABORTED) { FC_DBG("target abort cmd passed\n"); rc = SUCCESS; - - fsp->status_code = FC_CMD_ABORTED; - fc_io_compl(fsp); + fc_fcp_complete(fsp); } return rc; @@ -1165,8 +1191,20 @@ static int fc_lun_reset(struct fc_lport *lp, struct fc_fcp_pkt *fsp, static void fc_tm_done(struct fc_seq *sp, struct fc_frame *fp, void *arg) { struct fc_fcp_pkt *fsp = arg; + struct fc_frame_header *fh; spin_lock_bh(&fsp->scsi_pkt_lock); + if (IS_ERR(fp)) { + /* + * If there is an error just let it timeout or wait + * for TMF to be aborted if it timedout. + * + * scsi-eh will escalate for when either happens. + */ + spin_unlock_bh(&fsp->scsi_pkt_lock); + return; + } + /* * raced with eh timeout handler. * @@ -1179,16 +1217,9 @@ static void fc_tm_done(struct fc_seq *sp, struct fc_frame *fp, void *arg) return; } - if (IS_ERR(fp)) { - /* - * If there is an error just let it timeout. - * scsi-eh will escalate for us. - */ - spin_unlock_bh(&fsp->scsi_pkt_lock); - return; - } - - fc_fcp_resp(fsp, fp); + fh = fc_frame_header_get(fp); + if (fh->fh_type != FC_TYPE_BLS) + fc_fcp_resp(fsp, fp); fsp->seq_ptr = NULL; fsp->lp->tt.exch_done(sp); fc_frame_free(fp); @@ -1500,18 +1531,14 @@ out: */ static void fc_timeout_error(struct fc_fcp_pkt *fsp) { - struct fc_lport *lp = fsp->lp; - - fsp->state |= FC_SRB_ABORT_PENDING; - if (fsp->seq_ptr) - lp->tt.seq_exch_abort(fsp->seq_ptr); - - fsp->seq_ptr = NULL; fsp->status_code = FC_CMD_TIME_OUT; fsp->cdb_status = 0; fsp->io_status = 0; - - fc_io_compl(fsp); + /* + * if this fails then we let the scsi command timer fire and + * scsi-ml escalate. + */ + fc_fcp_send_abort(fsp); } /* @@ -1603,6 +1630,7 @@ retry: static void fc_fcp_srr_resp(struct fc_seq *sp, struct fc_frame *fp, void *arg) { struct fc_fcp_pkt *fsp = arg; + struct fc_frame_header *fh; u16 ox_id; u16 rx_id; @@ -1614,6 +1642,19 @@ static void fc_fcp_srr_resp(struct fc_seq *sp, struct fc_frame *fp, void *arg) if (fc_fcp_lock_pkt(fsp)) goto out; + fh = fc_frame_header_get(fp); + /* + * BUG? fc_fcp_srr_error calls exch_done which would release + * the ep. But if fc_fcp_srr_error had got -FC_EX_TIMEOUT, + * then fc_exch_timeout would be sending an abort. The exch_done + * call by fc_fcp_srr_error would prevent fc_exch.c from seeing + * an abort response though. + */ + if (fh->fh_type == FC_TYPE_BLS) { + fc_fcp_unlock_pkt(fsp); + return; + } + fsp->recov_seq = NULL; fsp->lp->tt.seq_get_xids(fsp->seq_ptr, &ox_id, &rx_id); @@ -1925,7 +1966,6 @@ int fc_eh_abort(struct scsi_cmnd *sc_cmd) goto release_pkt; } - sp->state |= FC_SRB_ABORT_PENDING; rc = fc_fcp_pkt_abort(lp, sp); fc_fcp_unlock_pkt(sp); -- 1.5.4.1