>-----Original Message----- >From: Mike Christie [mailto:[EMAIL PROTECTED] >Sent: Tuesday, August 26, 2008 6:24 PM >To: Dev, Vasu >Cc: [email protected] >Subject: Re: [Open-FCoE] [PATCH 3/4] libfc: Added exch release >fc_exch_mgr_delete_ep() > >Vasu Dev wrote: >> This will ensure exch will get freed if exch ref dropped to zero >> in fc_exch_mgr_delete_ep(). > > >> struct fc_exch_mgr *mp; >> > >If we can call fc_exch_mgr_delete_ep() while the last refcount is >dropped then if the last fc_exch_release is done here before the hold >then it will be freed when we call fc_exch_hold and we could be doing >the hold on freed memory. > >I do not think we should be able to get to this point.
Correct, otherwise we are already accessing wrong memory in fc_exch_mgr_delete_ep(). >Everyone calling >it should have a valid ref (or if called from a resp handler calling a >exch_done then the caller of the resp handler should have a refcount to >the ep). If not then the ref counting is not right. > The exch ref is mostly correct except in fc_exch_recv_req(), the fc_exch_recv_req() passed new exchage to upper layer without any additional exch hold and release. So without this patch ep was not freed from fc_exch_recv_req() when upper layer called fc_exc_done(). I'm okay either way on this patch applied or not but now it is kind of weired that ep is deleted from mp and along with mp->total_exches is also updated, that means now mp->total_exches could be zero while some ep are still around till their final fc_exch_release() is called. I guess it may be okay since mp->total_exches is keeping track of only exches in mp. So may be now we need another ep tracking for its memory since ep memory was not freed in fc_exch_recv_req() code paths and it went undetected though my unit test was hitting fc_exch_recv_req() code paths. I fixed exch ref in fc_exch_recv_req() path instead depending on this patch in attached fixed-exch-rel.patch and similarly we always need to ensure correct exch ref in other code paths without this patch. (no intentional new line in this para, to test my ongoing mail server issue causing paragraph appearing as long line without explicit new line) > >> + 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
