Vasu Dev wrote:
> This will ensure exch will get freed if exch ref dropped to zero
> in fc_exch_mgr_delete_ep().
That's ... interesting. The count will never be different at the end of
fc_exch_mgr_delete_ep() than it is at the beginning. The hold and
release you add here are working around the fact that
fc_exch_done_locked() decrements the count without checking to see if it
became zero.
I was looking at the same problem yesterday, and I think a better
solution is to never decrement the count without testing, and to protect
against it going to 0 in the middle of fc_exch_done_locked() by adding a
hold and release in fc_exch_done().
A hold and release in fc_exch_done() is essentially the same fix as what
you put in fc_exch_mgr_delete_ep(), but it makes a little more sense to
me while reading the code. Plus, it makes it safe to do a real release
in fc_exch_done_locked() instead of just the atomic_dec(). I'll send
out my patch.
Also I tried converting direct use of an atomic_t for the refcount to a
kref. Looks good to me so far, but I'm not sure the code is correct
everywhere.
- Chris
> Also moved calling exch_put() to fc_exch_release() since this is
> the time actual exchange is freed, this will also call exch_put()
> without mp lock from fc_exch_release(), therefore less mp locking
> period.
>
> Signed-off-by: Vasu Dev <[EMAIL PROTECTED]>
> ---
>
> drivers/scsi/libfc/fc_exch.c | 6 ++++--
> 1 files changed, 4 insertions(+), 2 deletions(-)
>
>
> diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c
> index a1d364a..60b2f52 100644
> --- a/drivers/scsi/libfc/fc_exch.c
> +++ b/drivers/scsi/libfc/fc_exch.c
> @@ -300,6 +300,8 @@ static void fc_exch_release(struct fc_exch *ep)
>
> WARN_ON(!ep->esb_stat & ESB_ST_COMPLETE);
> WARN_ON(timer_pending(&ep->ex_timer));
> + if (ep->lp->tt.exch_put)
> + ep->lp->tt.exch_put(ep->lp, mp, ep->xid);
> mempool_free(ep, mp->ep_pool);
> }
> }
> @@ -563,15 +565,15 @@ static void fc_exch_mgr_delete_ep(struct fc_exch *ep)
> {
> struct fc_exch_mgr *mp;
>
> + fc_exch_hold(ep);
> 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);
> + fc_exch_release(ep);
> }
>
> static int fc_exch_done_locked(struct fc_exch *ep)
>
> _______________________________________________
> devel mailing list
> [email protected]
> http://www.open-fcoe.org/mailman/listinfo/devel
>
_______________________________________________
devel mailing list
[email protected]
http://www.open-fcoe.org/mailman/listinfo/devel
