Thread (95 messages) 95 messages, 4 authors, 2008-10-01

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 +1000
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help