Re: xfrm_state locking regression...
From: Timo Teräs <hidden>
Date: 2008-09-03 06:27:43
David Miller wrote:
From: Herbert Xu <herbert@gondor.apana.org.au> Date: Wed, 3 Sep 2008 15:50:41 +1000quoted
On Wed, Sep 03, 2008 at 08:45:01AM +0300, Timo Teräs wrote:quoted
quoted
ipsec: Move state dump list unlinking into GC This patch moves the unlinking of x->all (for dumping) into the GC and changes the lock to xfrm_cfg_mutex.Shouldn't this also change the locking in all places where x->all is used?You're right. I'd forgotten about the larval states for some reason, probably because I just added them to the list :) So let's keep xfrm_state_lock and just move it.How about we un-crap the reference counting? Every list an object lives on is an external reference, and that is the rule that is violated by this ->all list handling. And that's why we need to take a single shared lock twice just to get rid of an xfrm_state object now.
Well, it's just another list keeping a reference like ->bydst, ->bysrc and ->byspi. The actual amount of external references is still correct (the walking code calls _hold() when it returns while keeping an external pointer). The difference is that node should not be unlinked from ->all until all other references are gone. For other lists the unlinking can be done earlier since they are used only for lookups. Any good other ways to enumerate to list entries while allowing to keep a temporary "iterator"? The previous method was crap too. - Timo