Updating exch ref count based on timer_pending check can mess
up exch ref count if timer fires just after this check. Instead count
on del/mod_timer() return values to update exch ref count.
Also fixed a very unlikely but still possible race in fc_exch_release().
Signed-off-by: Vasu Dev <[EMAIL PROTECTED]>
---
drivers/scsi/libfc/fc_exch.c | 46 +++++++++++++++++++++++-------------------
1 files changed, 25 insertions(+), 21 deletions(-)
diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c
index 4e552c0..342af7e 100644
--- a/drivers/scsi/libfc/fc_exch.c
+++ b/drivers/scsi/libfc/fc_exch.c
@@ -288,11 +288,21 @@ static void fc_exch_release(struct fc_exch *ep)
if (atomic_dec_and_test(&ep->ex_refcnt)) {
WARN_ON(!ep->esb_stat & ESB_ST_COMPLETE);
- del_timer(&ep->ex_timer);
+ 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.
+ */
+ 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);
- spin_lock_bh(&mp->em_lock);
WARN_ON(mp->total_exches <= 0);
mp->total_exches--;
mp->exches[ep->xid - mp->min_xid] = NULL;
@@ -305,12 +315,11 @@ static void fc_exch_release(struct fc_exch *ep)
/*
* Internal version of fc_exch_timer_set - used with lock held.
*/
-static void fc_exch_timer_set_locked(struct fc_exch *ep,
- unsigned int timer_msec)
+static inline void fc_exch_timer_set_locked(struct fc_exch *ep,
+ unsigned int timer_msec)
{
- if (!timer_pending(&ep->ex_timer))
+ if (!mod_timer(&ep->ex_timer, jiffies + msecs_to_jiffies(timer_msec)))
fc_exch_hold(ep); /* hold for timer */
- mod_timer(&ep->ex_timer, jiffies + msecs_to_jiffies(timer_msec));
}
/*
@@ -559,13 +568,11 @@ static void fc_exch_complete_locked(struct fc_exch *ep)
if (unlikely(ep->esb_stat & ESB_ST_REC_QUAL)) {
fc_exch_timer_set_locked(ep, 10);
} else {
- if (timer_pending(&ep->ex_timer)) {
- del_timer(&ep->ex_timer);
- /*
- * drop hold for timer
- */
+ /*
+ * delete pending timer and drop hold for timer
+ */
+ if (del_timer(&ep->ex_timer))
atomic_dec(&ep->ex_refcnt);
- }
atomic_dec(&ep->ex_refcnt);
}
}
@@ -1410,10 +1417,9 @@ static void fc_exch_reset(struct fc_exch *ep)
if (ep->esb_stat & ESB_ST_COMPLETE)
resp = NULL;
arg = ep->resp_arg;
- if (timer_pending(&ep->ex_timer)) {
- del_timer(&ep->ex_timer);
+ if (del_timer(&ep->ex_timer))
atomic_dec(&ep->ex_refcnt); /* drop hold for timer */
- }
+
sp = &ep->seq;
ep->esb_stat |= ESB_ST_COMPLETE;
spin_unlock_bh(&ep->ex_lock);
@@ -1635,10 +1641,9 @@ static void fc_exch_els_rrq(struct fc_seq *sp, struct
fc_frame *fp)
ep->esb_stat &= ~ESB_ST_REC_QUAL;
atomic_dec(&ep->ex_refcnt); /* drop hold for rec qual */
}
- if ((ep->esb_stat & ESB_ST_COMPLETE) && timer_pending(&ep->ex_timer)) {
- del_timer(&ep->ex_timer);
+ if ((ep->esb_stat & ESB_ST_COMPLETE) && (del_timer(&ep->ex_timer)))
atomic_dec(&ep->ex_refcnt); /* drop hold for timer */
- }
+
spin_unlock_bh(&ep->ex_lock);
/*
@@ -1728,10 +1733,9 @@ void fc_exch_done(struct fc_seq *sp)
spin_lock_bh(&ep->ex_lock);
ep->esb_stat |= ESB_ST_COMPLETE;
ep->resp = NULL;
- if (timer_pending(&ep->ex_timer)) {
- del_timer(&ep->ex_timer);
+ 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));
}
_______________________________________________
devel mailing list
[email protected]
http://www.open-fcoe.org/mailman/listinfo/devel
