[Open-FCoE] [PATCH 2/2] libfc: fixed some more possible fc_exch_reset() races

Vasu Dev vasu.dev at intel.com
Fri Sep 5 18:28:58 UTC 2008


Added exch lock for callers of exch_get()/fc_exch_alloc() to
prevent fc_exch_reset() from freeing exch while these callers
are is still working on just allocated exch.

This change impacted fc_exch_resp() call flow also
since fc_exch_resp() allocats new exchange, this patch
made fc_exch_resp() similar to fc_exch_find()
in terms of exch ref counting by having additional exch
ref to caller of fc_exch_resp() before dropping exch lock
from exchange allocation. This would allow fc_exch_reset()
running in fc_exch_resp() callers without exch being freed
by fc_exch_reset() while caller of fc_exch_resp() are still
working on exch.

This patch also associated exch hold from fc_exch_alloc()
to exch pointer in mp and then called fc_exch_release()
after removing exch pointer in mp to drop this hold
in fc_exch_mgr_delete_ep(). This change eliminated need
for extra exch hold in some error cases such as error
handling case in fc_exch_seq_send() and also removed
exch ref decrement without test in fc_exch_done_locked().

Signed-off-by: Vasu Dev <vasu.dev at intel.com>
---

 drivers/scsi/libfc/fc_exch.c |   28 +++++++++++++++++++---------
 1 files changed, 19 insertions(+), 9 deletions(-)


diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c
index f307008..3a630c5 100644
--- a/drivers/scsi/libfc/fc_exch.c
+++ b/drivers/scsi/libfc/fc_exch.c
@@ -322,7 +322,6 @@ static int fc_exch_done_locked(struct fc_exch *ep)
 		ep->state |= FC_EX_DONE;
 		if (del_timer(&ep->ex_timer))
 			atomic_dec(&ep->ex_refcnt); /* drop hold for timer */
-		atomic_dec(&ep->ex_refcnt); /* drop hold from alloc */
 		rc = 0;
 	}
 	return rc;
@@ -339,6 +338,7 @@ static void fc_exch_mgr_delete_ep(struct fc_exch *ep)
 	mp->exches[ep->xid - mp->min_xid] = NULL;
 	list_del(&ep->ex_list);
 	spin_unlock_bh(&mp->em_lock);
+	fc_exch_release(ep);	/* drop hold for exch in mp */
 }
 
 /*
@@ -553,6 +553,7 @@ struct fc_exch *fc_exch_alloc(struct fc_exch_mgr *mp, u16 xid)
 	fc_seq_alloc(ep, ep->seq_id++);
 	mp->total_exches++;
 	spin_unlock_bh(&mp->em_lock);
+	fc_exch_hold(ep);	/* hold for exch in mp */
 
 	/*
 	 *  update exchange
@@ -567,7 +568,12 @@ struct fc_exch *fc_exch_alloc(struct fc_exch_mgr *mp, u16 xid)
 	spin_lock_init(&ep->ex_lock);
 	setup_timer(&ep->ex_timer, fc_exch_timeout, (unsigned long)ep);
 
-	fc_exch_hold(ep);	/* hold for caller */
+	/*
+	 * Hold exch lock for caller to prevent
+	 * fc_exch_reset() from releasing exch
+	 * while caller is still working on exch.
+	 */
+	spin_lock_bh(&ep->ex_lock);
 out:
 	return ep;
 }
@@ -652,6 +658,8 @@ static struct fc_exch *fc_exch_resp(struct fc_exch_mgr *mp, struct fc_frame *fp)
 			WARN_ON(rxid != FC_XID_UNKNOWN);
 			fh->fh_rx_id = htons(ep->rxid);
 		}
+		fc_exch_hold(ep);	/* hold for caller */
+		spin_unlock_bh(&ep->ex_lock);	/* lock from exch_get */
 	}
 	return ep;
 }
@@ -665,7 +673,7 @@ static enum fc_pf_rjt_reason
 fc_seq_lookup_recip(struct fc_exch_mgr *mp, struct fc_frame *fp)
 {
 	struct fc_frame_header *fh = fc_frame_header_get(fp);
-	struct fc_exch *ep = NULL, *new_ep = NULL;
+	struct fc_exch *ep = NULL;
 	struct fc_seq *sp = NULL;
 	enum fc_pf_rjt_reason reject = FC_RJT_NONE;
 	u32 f_ctl;
@@ -715,7 +723,7 @@ fc_seq_lookup_recip(struct fc_exch_mgr *mp, struct fc_frame *fp)
 				reject = FC_RJT_RX_ID;
 				goto rel;
 			}
-			new_ep = ep = fc_exch_resp(mp, fp);
+			ep = fc_exch_resp(mp, fp);
 			if (!ep) {
 				reject = FC_RJT_EXCH_EST;	/* XXX */
 				goto out;
@@ -758,9 +766,8 @@ fc_seq_lookup_recip(struct fc_exch_mgr *mp, struct fc_frame *fp)
 out:
 	return reject;
 rel:
-	fc_exch_release(ep);
-	if (new_ep)
-		fc_exch_release(new_ep);
+	fc_exch_done(&ep->seq);
+	fc_exch_release(ep);	/* hold from fc_exch_find/fc_exch_resp */
 	return reject;
 }
 
@@ -1850,6 +1857,7 @@ struct fc_seq *fc_exch_seq_send(struct fc_lport *lp,
 	struct fc_seq *sp = NULL;
 	struct fc_frame_header *fh;
 	u16 fill;
+	int rc = 1;
 
 	ep = lp->tt.exch_get(lp, fp);
 	if (!ep) {
@@ -1895,7 +1903,6 @@ struct fc_seq *fc_exch_seq_send(struct fc_lport *lp,
 	if (unlikely(lp->tt.frame_send(lp, fp)))
 		goto err;
 
-	spin_lock_bh(&ep->ex_lock);
 	if (timer_msec)
 		fc_exch_timer_set_locked(ep, timer_msec);
 	sp->f_ctl = f_ctl;	/* save for possible abort */
@@ -1907,7 +1914,10 @@ struct fc_seq *fc_exch_seq_send(struct fc_lport *lp,
 	spin_unlock_bh(&ep->ex_lock);
 	return sp;
 err:
-	fc_exch_done(sp);
+	rc = fc_exch_done_locked(ep);
+	spin_unlock_bh(&ep->ex_lock);
+	if (!rc)
+		fc_exch_mgr_delete_ep(ep);
 	return NULL;
 }
 EXPORT_SYMBOL(fc_exch_seq_send);




More information about the devel mailing list