diff options
author | Herbert Xu <herbert@gondor.apana.org.au> | 2005-03-31 05:18:16 -0800 |
---|---|---|
committer | David S. Miller <davem@sunset.davemloft.net> | 2005-03-31 05:18:16 -0800 |
commit | 71bf6e6d7605727cc287644a11da8e70fd2c63b6 (patch) | |
tree | c6819aa5a88febd13827f4764eaf7dba85510bcf | |
parent | 102bbba64bde80e5cf25119096dba81adcf53933 (diff) | |
download | history-71bf6e6d7605727cc287644a11da8e70fd2c63b6.tar.gz |
[IPSEC]: Move xfrm_flush_bundles into xfrm_state GC
Fixes ABBA deadlock noticed by Patrick McHardy.
The locking in xfrm_state/xfrm_policy has always struck me as being
an overkill. A lot of the locks should be replaced by rules that
ensure the validity of most operations while a ref count is held.
Now I have an excuse to do just that :)
For 2.6.12 let's go for a simpler fix that breaks the dead lock.
__xfrm_state_delete does not need to flush the bundles immediately.
In fact, it is more efficient if we delay the flush to the GC worker
since the flush is not dependent on any particular xfrm state. By
delaying it we can do one single flush even when you're deleteing
the entire xfrm state list.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: David S. Miller <davem@davemloft.net>
-rw-r--r-- | net/xfrm/xfrm_state.c | 13 |
1 files changed, 11 insertions, 2 deletions
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c index 78d6c52a65ccbf..102bb826e2736a 100644 --- a/net/xfrm/xfrm_state.c +++ b/net/xfrm/xfrm_state.c @@ -48,6 +48,8 @@ static struct work_struct xfrm_state_gc_work; static struct list_head xfrm_state_gc_list = LIST_HEAD_INIT(xfrm_state_gc_list); static DEFINE_SPINLOCK(xfrm_state_gc_lock); +static int xfrm_state_gc_flush_bundles; + static void __xfrm_state_delete(struct xfrm_state *x); static struct xfrm_state_afinfo *xfrm_state_get_afinfo(unsigned short family); @@ -81,6 +83,11 @@ static void xfrm_state_gc_task(void *data) struct list_head *entry, *tmp; struct list_head gc_list = LIST_HEAD_INIT(gc_list); + if (xfrm_state_gc_flush_bundles) { + xfrm_state_gc_flush_bundles = 0; + xfrm_flush_bundles(); + } + spin_lock_bh(&xfrm_state_gc_lock); list_splice_init(&xfrm_state_gc_list, &gc_list); spin_unlock_bh(&xfrm_state_gc_lock); @@ -228,8 +235,10 @@ static void __xfrm_state_delete(struct xfrm_state *x) * our caller holds. A larger value means that * there are DSTs attached to this xfrm_state. */ - if (atomic_read(&x->refcnt) > 2) - xfrm_flush_bundles(); + if (atomic_read(&x->refcnt) > 2) { + xfrm_state_gc_flush_bundles = 1; + schedule_work(&xfrm_state_gc_work); + } /* All xfrm_state objects are created by xfrm_state_alloc. * The xfrm_state_alloc call gives a reference, and that |