From: Mike Christie <[EMAIL PROTECTED]>
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 <[EMAIL PROTECTED]>
---
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 c0579dc..ca3d585 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.4.1
_______________________________________________
devel mailing list
[email protected]
http://www.open-fcoe.org/mailman/listinfo/devel
