Dev, Vasu wrote: > >> -----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()
Ah, I was thinking of them as two issues like you mentioned below. The refcounting is for just the lifetime management of the ep struct memory. Then calling exch_done/fc_exch_mgr_delete_ep manages the mp resources like the id reuse. > 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. Do you mean you were running tests on that code path and then in some tests you ended up getting to fc_exch_mgr_free? I have hit the WARN_ON in fc_exch_mgr_free and errors in mempool_destroy or kmem_cache_destroy errors indicating there were still eps around somewhere. > > 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) Path looked ok. _______________________________________________ devel mailing list [email protected] http://www.open-fcoe.org/mailman/listinfo/devel
