Re: xfrm_state locking regression...

["Timo Teräs" <timo.teras@iki.fi>]

David Miller wrote:
> From: Herbert Xu <herbert@gondor.apana.org.au>
> Date: Wed, 3 Sep 2008 15:50:41 +1000
> 
>> On Wed, Sep 03, 2008 at 08:45:01AM +0300, Timo Teräs wrote:
>>>> 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