From 94ccdf204a50165e5a1064c98bb1c70fbf4c4f6c Mon Sep 17 00:00:00 2001 From: Mike Christie Date: Wed, 13 Aug 2008 07:08:01 -0500 Subject: [PATCH 09/23] libfc: remove extra recount check in fc_exch_release The extra refcount check just is not right as far as refcounting goes. We either should use refcounts correctly or just not use them at all (use locks and state bits). This patch has us remove the the ep from the mp array before we do the final release when we are completing the ep, so that if a frame is being read in while we are completing it then either the ep find path will either find the ep and get a proper ref or it will not find it and the frames can be dropped. This also merges exch_done with exch_complete. The only major difference is that exch_complete adjusts the existing recovery timer if there is a recovery qualifier for the ep. It seems like a ok optimization, but not needed since we already had to abort the ep and restart it from the beginning so performance is already screwed. Signed-off-by: Mike Christie --- drivers/scsi/libfc/fc_exch.c | 114 ++++++++++++++++------------------------- 1 files changed, 45 insertions(+), 69 deletions(-) diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c index 41bbe4b..2729f0d 100644 --- a/drivers/scsi/libfc/fc_exch.c +++ b/drivers/scsi/libfc/fc_exch.c @@ -289,29 +289,10 @@ static void fc_exch_release(struct fc_exch *ep) struct fc_exch_mgr *mp; if (atomic_dec_and_test(&ep->ex_refcnt)) { + mp = ep->em; + WARN_ON(!ep->esb_stat & ESB_ST_COMPLETE); WARN_ON(timer_pending(&ep->ex_timer)); - mp = ep->em; - spin_lock_bh(&mp->em_lock); - /* - * ensure ep->ex_refcnt is still zero - * if not then skip freeing exchange - * since some new incoming frame took - * hold of this, which is unlikely. - * Eliminating possibility of ep->ex_refcnt - * going up from zero would be better solution. XXX - */ - if (atomic_read(&ep->ex_refcnt)) { - spin_unlock_bh(&mp->em_lock); - return; - } - if (ep->lp->tt.exch_put) - ep->lp->tt.exch_put(ep->lp, mp, ep->xid); - WARN_ON(mp->total_exches <= 0); - mp->total_exches--; - mp->exches[ep->xid - mp->min_xid] = NULL; - list_del(&ep->ex_list); - spin_unlock_bh(&mp->em_lock); kmem_cache_free(mp->em_cache, ep); } } @@ -562,44 +543,48 @@ static struct fc_exch *fc_exch_find(struct fc_exch_mgr *mp, u16 xid) return ep; } -/* - * Mark exchange complete - internal version called with ex_lock held. - */ -static void fc_exch_complete_locked(struct fc_exch *ep) +static void fc_exch_mgr_delete_ep(struct fc_exch *ep) { + struct fc_exch_mgr *mp; + + mp = ep->em; + spin_lock_bh(&mp->em_lock); + if (ep->lp->tt.exch_put) + ep->lp->tt.exch_put(ep->lp, mp, ep->xid); + WARN_ON(mp->total_exches <= 0); + mp->total_exches--; + mp->exches[ep->xid - mp->min_xid] = NULL; + list_del(&ep->ex_list); + spin_unlock_bh(&mp->em_lock); +} + +static int fc_exch_done_locked(struct fc_exch *ep) +{ + int rc = 1; + ep->esb_stat |= ESB_ST_COMPLETE; ep->resp = NULL; - - /* - * Assuming in-order delivery, the timeout for RRQ is 0, not R_A_TOV. - * Here, we allow a short time for frames which may have been - * re-ordered in various kernel queues or due to interrupt balancing. - * Also, using a timer here allows us to issue the RRQ after the - * exchange lock is dropped. - */ - if (unlikely(ep->esb_stat & ESB_ST_REC_QUAL)) { - fc_exch_timer_set_locked(ep, 10); - } else { - /* - * delete pending timer and drop hold for timer - */ + if (!(ep->esb_stat & ESB_ST_REC_QUAL)) { if (del_timer(&ep->ex_timer)) - atomic_dec(&ep->ex_refcnt); - atomic_dec(&ep->ex_refcnt); + atomic_dec(&ep->ex_refcnt); /* drop hold for timer */ + atomic_dec(&ep->ex_refcnt); /* drop hold from alloc */ + rc = 0; } + return rc; } -/* - * Mark exchange complete. - * The state may be available for ILS Read Exchange Status (RES) for a time. - * The caller doesn't necessarily hold the exchange. - */ -static void fc_exch_complete(struct fc_exch *ep) +void fc_exch_done(struct fc_seq *sp) { + struct fc_exch *ep = fc_seq_exch(sp); + int rc; + spin_lock_bh(&ep->ex_lock); - fc_exch_complete_locked(ep); + rc = fc_exch_done_locked(ep); spin_unlock_bh(&ep->ex_lock); + if (!rc) + fc_exch_mgr_delete_ep(ep); } +EXPORT_SYMBOL(fc_exch_done); /* * Allocate a new exchange as responder. @@ -1171,6 +1156,7 @@ static void fc_exch_recv_seq_resp(struct fc_exch_mgr *mp, struct fc_frame *fp) u32 f_ctl; void (*resp)(struct fc_seq *, struct fc_frame *fp, void *arg); void *ex_resp_arg; + int rc; ep = fc_exch_find(mp, ntohs(fh->fh_ox_id)); if (!ep) { @@ -1214,9 +1200,11 @@ static void fc_exch_recv_seq_resp(struct fc_exch_mgr *mp, struct fc_frame *fp) (f_ctl & (FC_FC_LAST_SEQ | FC_FC_END_SEQ)) == (FC_FC_LAST_SEQ | FC_FC_END_SEQ)) { spin_lock_bh(&ep->ex_lock); - fc_exch_complete_locked(ep); + rc = fc_exch_done_locked(ep); WARN_ON(fc_seq_exch(sp) != ep); spin_unlock_bh(&ep->ex_lock); + if (!rc) + fc_exch_mgr_delete_ep(ep); } /* @@ -1277,6 +1265,7 @@ static void fc_exch_abts_resp(struct fc_exch *ep, struct fc_frame *fp) struct fc_seq *sp; u16 low; u16 high; + int rc = 1; fh = fc_frame_header_get(fp); if (fc_exch_debug) @@ -1320,9 +1309,12 @@ static void fc_exch_abts_resp(struct fc_exch *ep, struct fc_frame *fp) /* * 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); + if (fh->fh_type != FC_TYPE_FCP && + ntoh24(fh->fh_f_ctl) & FC_FC_LAST_SEQ) + rc = fc_exch_done_locked(ep); spin_unlock_bh(&ep->ex_lock); + if (!rc) + fc_exch_mgr_delete_ep(ep); if (resp) resp(sp, fp, ex_resp_arg); @@ -1618,6 +1610,7 @@ static void fc_exch_rrq_resp(struct fc_seq *sp, struct fc_frame *fp, void *arg) FC_DBG("LS_RJT for RRQ"); break; case ELS_LS_ACC: + fc_exch_done(&aborted_ep->seq); fc_exch_release(aborted_ep); /* drop hold for rec qual */ break; default: @@ -1788,23 +1781,6 @@ void fc_exch_mgr_free(struct fc_exch_mgr *mp) } EXPORT_SYMBOL(fc_exch_mgr_free); -void fc_exch_done(struct fc_seq *sp) -{ - struct fc_exch *ep; - - ep = fc_seq_exch(sp); - spin_lock_bh(&ep->ex_lock); - ep->esb_stat |= ESB_ST_COMPLETE; - ep->resp = NULL; - 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)); -} -EXPORT_SYMBOL(fc_exch_done); - struct fc_exch *fc_exch_get(struct fc_lport *lp, struct fc_frame *fp) { if (!lp || !lp->emp) @@ -1882,7 +1858,7 @@ struct fc_seq *fc_exch_seq_send(struct fc_lport *lp, spin_unlock_bh(&ep->ex_lock); return sp; err: - fc_exch_complete(ep); + fc_exch_done(sp); return NULL; } EXPORT_SYMBOL(fc_exch_seq_send); -- 1.5.5.1