Dev, Vasu wrote: >> The locking and refcount use is really weird now though. Would it be >> cleaner or more standard to just say that you need to always grab the >> lock when accessing the ex_refcount instead of sometimes doing it and >> sometimes not (too confusing). > > Currently exch ref count can be changed(inc/dec) without having need to hold > exch lock, but this can be revised as suggested above to always have exch > lock for changing exch ref. However as you mentioned in other email that > exch ref and locking is still nicer than fsp pkt locking and doesn't need > expensive del_timer_sync() as used in FCP.
I am not sure what you mean by combining the two issues? For the FCP path we do not need the pkt lock for the actual release or hold of the refcount. We actually nornally never have the pkt lock when we do the final release, because fc_fcp_unlock_pkt drops the lock then drops the refcount. My comment about the del timer and refcounts is just about not needing to use the del_timer_sync with the exahnge code because the timer operation should always have a refcount. My comment about taking the lock around the exhange hold/release is just about what the locks and refcounts are protecting? I mean it seems like we are trying to do the two operations: exchange mp->exches lookup + hold and mp->exches clear + release atomically. We currently do both the lookup and hold under the mp->em_lock, but we do the release without the lock and take it only if the refcount is zero so we can clear the mp->exches array among other things. And this is racey because it seems like we want do drop the refcount and clear the mp->exches atomically. So with your patch we now drop the count without the lock, but if zero grab the lock to check the refcont again which seems weird because normally when a refcount goes to zero it means that we are going to free it. I mean if we were strict and we used krefs this would not be possible, and if you tried to send a patch for kref_put to do an extra check it is easier to imagine Christoph telling us whatever we are trying to do is crap over Greg happily taking it. _______________________________________________ devel mailing list [email protected] http://www.open-fcoe.org/mailman/listinfo/devel
