diff options
author | Herbert Xu <herbert@gondor.apana.org.au> | 2006-05-28 02:03:58 -0400 |
---|---|---|
committer | David S. Miller <davem@sunset.davemloft.net> | 2006-06-18 00:28:37 -0400 |
commit | 546be2405be119ef55467aace45f337a16e5d424 (patch) | |
tree | 9b09f0041f9f82a20ab25ace3c7360e4a4b7989f | |
parent | 9cb3528cdbffc513eb9fb8faa45d41e397355830 (diff) |
[IPSEC] xfrm: Undo afinfo lock proliferation
The number of locks used to manage afinfo structures can easily be reduced
down to one each for policy and state respectively. This is based on the
observation that the write locks are only held by module insertion/removal
which are very rare events so there is no need to further differentiate
between the insertion of modules like ipv6 versus esp6.
The removal of the read locks in xfrm4_policy.c/xfrm6_policy.c might look
suspicious at first. However, after you realise that nobody ever takes
the corresponding write lock you'll feel better :)
As far as I can gather it's an attempt to guard against the removal of
the corresponding modules. Since neither module can be unloaded at all
we can leave it to whoever fixes up IPv6 unloading :)
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: David S. Miller <davem@davemloft.net>
-rw-r--r-- | include/net/xfrm.h | 9 | ||||
-rw-r--r-- | net/ipv4/xfrm4_policy.c | 6 | ||||
-rw-r--r-- | net/ipv4/xfrm4_state.c | 1 | ||||
-rw-r--r-- | net/ipv6/xfrm6_policy.c | 6 | ||||
-rw-r--r-- | net/ipv6/xfrm6_state.c | 1 | ||||
-rw-r--r-- | net/xfrm/xfrm_policy.c | 58 | ||||
-rw-r--r-- | net/xfrm/xfrm_state.c | 9 |
7 files changed, 38 insertions, 52 deletions
diff --git a/include/net/xfrm.h b/include/net/xfrm.h index afa508d92c93..ed7c9747059d 100644 --- a/include/net/xfrm.h +++ b/include/net/xfrm.h | |||
@@ -204,8 +204,7 @@ struct xfrm_type; | |||
204 | struct xfrm_dst; | 204 | struct xfrm_dst; |
205 | struct xfrm_policy_afinfo { | 205 | struct xfrm_policy_afinfo { |
206 | unsigned short family; | 206 | unsigned short family; |
207 | rwlock_t lock; | 207 | struct xfrm_type *type_map[256]; |
208 | struct xfrm_type_map *type_map; | ||
209 | struct dst_ops *dst_ops; | 208 | struct dst_ops *dst_ops; |
210 | void (*garbage_collect)(void); | 209 | void (*garbage_collect)(void); |
211 | int (*dst_lookup)(struct xfrm_dst **dst, struct flowi *fl); | 210 | int (*dst_lookup)(struct xfrm_dst **dst, struct flowi *fl); |
@@ -232,7 +231,6 @@ extern int __xfrm_state_delete(struct xfrm_state *x); | |||
232 | 231 | ||
233 | struct xfrm_state_afinfo { | 232 | struct xfrm_state_afinfo { |
234 | unsigned short family; | 233 | unsigned short family; |
235 | rwlock_t lock; | ||
236 | struct list_head *state_bydst; | 234 | struct list_head *state_bydst; |
237 | struct list_head *state_byspi; | 235 | struct list_head *state_byspi; |
238 | int (*init_flags)(struct xfrm_state *x); | 236 | int (*init_flags)(struct xfrm_state *x); |
@@ -264,11 +262,6 @@ struct xfrm_type | |||
264 | u32 (*get_max_size)(struct xfrm_state *, int size); | 262 | u32 (*get_max_size)(struct xfrm_state *, int size); |
265 | }; | 263 | }; |
266 | 264 | ||
267 | struct xfrm_type_map { | ||
268 | rwlock_t lock; | ||
269 | struct xfrm_type *map[256]; | ||
270 | }; | ||
271 | |||
272 | extern int xfrm_register_type(struct xfrm_type *type, unsigned short family); | 265 | extern int xfrm_register_type(struct xfrm_type *type, unsigned short family); |
273 | extern int xfrm_unregister_type(struct xfrm_type *type, unsigned short family); | 266 | extern int xfrm_unregister_type(struct xfrm_type *type, unsigned short family); |
274 | extern struct xfrm_type *xfrm_get_type(u8 proto, unsigned short family); | 267 | extern struct xfrm_type *xfrm_get_type(u8 proto, unsigned short family); |
diff --git a/net/ipv4/xfrm4_policy.c b/net/ipv4/xfrm4_policy.c index 8604c747bca5..c0465284dfac 100644 --- a/net/ipv4/xfrm4_policy.c +++ b/net/ipv4/xfrm4_policy.c | |||
@@ -17,8 +17,6 @@ | |||
17 | static struct dst_ops xfrm4_dst_ops; | 17 | static struct dst_ops xfrm4_dst_ops; |
18 | static struct xfrm_policy_afinfo xfrm4_policy_afinfo; | 18 | static struct xfrm_policy_afinfo xfrm4_policy_afinfo; |
19 | 19 | ||
20 | static struct xfrm_type_map xfrm4_type_map = { .lock = RW_LOCK_UNLOCKED }; | ||
21 | |||
22 | static int xfrm4_dst_lookup(struct xfrm_dst **dst, struct flowi *fl) | 20 | static int xfrm4_dst_lookup(struct xfrm_dst **dst, struct flowi *fl) |
23 | { | 21 | { |
24 | return __ip_route_output_key((struct rtable**)dst, fl); | 22 | return __ip_route_output_key((struct rtable**)dst, fl); |
@@ -237,9 +235,7 @@ _decode_session4(struct sk_buff *skb, struct flowi *fl) | |||
237 | 235 | ||
238 | static inline int xfrm4_garbage_collect(void) | 236 | static inline int xfrm4_garbage_collect(void) |
239 | { | 237 | { |
240 | read_lock(&xfrm4_policy_afinfo.lock); | ||
241 | xfrm4_policy_afinfo.garbage_collect(); | 238 | xfrm4_policy_afinfo.garbage_collect(); |
242 | read_unlock(&xfrm4_policy_afinfo.lock); | ||
243 | return (atomic_read(&xfrm4_dst_ops.entries) > xfrm4_dst_ops.gc_thresh*2); | 239 | return (atomic_read(&xfrm4_dst_ops.entries) > xfrm4_dst_ops.gc_thresh*2); |
244 | } | 240 | } |
245 | 241 | ||
@@ -299,8 +295,6 @@ static struct dst_ops xfrm4_dst_ops = { | |||
299 | 295 | ||
300 | static struct xfrm_policy_afinfo xfrm4_policy_afinfo = { | 296 | static struct xfrm_policy_afinfo xfrm4_policy_afinfo = { |
301 | .family = AF_INET, | 297 | .family = AF_INET, |
302 | .lock = RW_LOCK_UNLOCKED, | ||
303 | .type_map = &xfrm4_type_map, | ||
304 | .dst_ops = &xfrm4_dst_ops, | 298 | .dst_ops = &xfrm4_dst_ops, |
305 | .dst_lookup = xfrm4_dst_lookup, | 299 | .dst_lookup = xfrm4_dst_lookup, |
306 | .find_bundle = __xfrm4_find_bundle, | 300 | .find_bundle = __xfrm4_find_bundle, |
diff --git a/net/ipv4/xfrm4_state.c b/net/ipv4/xfrm4_state.c index dbabf81a9b7b..81e1751c966e 100644 --- a/net/ipv4/xfrm4_state.c +++ b/net/ipv4/xfrm4_state.c | |||
@@ -131,7 +131,6 @@ __xfrm4_find_acq(u8 mode, u32 reqid, u8 proto, | |||
131 | 131 | ||
132 | static struct xfrm_state_afinfo xfrm4_state_afinfo = { | 132 | static struct xfrm_state_afinfo xfrm4_state_afinfo = { |
133 | .family = AF_INET, | 133 | .family = AF_INET, |
134 | .lock = RW_LOCK_UNLOCKED, | ||
135 | .init_flags = xfrm4_init_flags, | 134 | .init_flags = xfrm4_init_flags, |
136 | .init_tempsel = __xfrm4_init_tempsel, | 135 | .init_tempsel = __xfrm4_init_tempsel, |
137 | .state_lookup = __xfrm4_state_lookup, | 136 | .state_lookup = __xfrm4_state_lookup, |
diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c index 88c840f1beb6..ee715f2691e9 100644 --- a/net/ipv6/xfrm6_policy.c +++ b/net/ipv6/xfrm6_policy.c | |||
@@ -23,8 +23,6 @@ | |||
23 | static struct dst_ops xfrm6_dst_ops; | 23 | static struct dst_ops xfrm6_dst_ops; |
24 | static struct xfrm_policy_afinfo xfrm6_policy_afinfo; | 24 | static struct xfrm_policy_afinfo xfrm6_policy_afinfo; |
25 | 25 | ||
26 | static struct xfrm_type_map xfrm6_type_map = { .lock = RW_LOCK_UNLOCKED }; | ||
27 | |||
28 | static int xfrm6_dst_lookup(struct xfrm_dst **dst, struct flowi *fl) | 26 | static int xfrm6_dst_lookup(struct xfrm_dst **dst, struct flowi *fl) |
29 | { | 27 | { |
30 | int err = 0; | 28 | int err = 0; |
@@ -249,9 +247,7 @@ _decode_session6(struct sk_buff *skb, struct flowi *fl) | |||
249 | 247 | ||
250 | static inline int xfrm6_garbage_collect(void) | 248 | static inline int xfrm6_garbage_collect(void) |
251 | { | 249 | { |
252 | read_lock(&xfrm6_policy_afinfo.lock); | ||
253 | xfrm6_policy_afinfo.garbage_collect(); | 250 | xfrm6_policy_afinfo.garbage_collect(); |
254 | read_unlock(&xfrm6_policy_afinfo.lock); | ||
255 | return (atomic_read(&xfrm6_dst_ops.entries) > xfrm6_dst_ops.gc_thresh*2); | 251 | return (atomic_read(&xfrm6_dst_ops.entries) > xfrm6_dst_ops.gc_thresh*2); |
256 | } | 252 | } |
257 | 253 | ||
@@ -311,8 +307,6 @@ static struct dst_ops xfrm6_dst_ops = { | |||
311 | 307 | ||
312 | static struct xfrm_policy_afinfo xfrm6_policy_afinfo = { | 308 | static struct xfrm_policy_afinfo xfrm6_policy_afinfo = { |
313 | .family = AF_INET6, | 309 | .family = AF_INET6, |
314 | .lock = RW_LOCK_UNLOCKED, | ||
315 | .type_map = &xfrm6_type_map, | ||
316 | .dst_ops = &xfrm6_dst_ops, | 310 | .dst_ops = &xfrm6_dst_ops, |
317 | .dst_lookup = xfrm6_dst_lookup, | 311 | .dst_lookup = xfrm6_dst_lookup, |
318 | .find_bundle = __xfrm6_find_bundle, | 312 | .find_bundle = __xfrm6_find_bundle, |
diff --git a/net/ipv6/xfrm6_state.c b/net/ipv6/xfrm6_state.c index a5723024d3b3..b33296b3f6de 100644 --- a/net/ipv6/xfrm6_state.c +++ b/net/ipv6/xfrm6_state.c | |||
@@ -135,7 +135,6 @@ __xfrm6_find_acq(u8 mode, u32 reqid, u8 proto, | |||
135 | 135 | ||
136 | static struct xfrm_state_afinfo xfrm6_state_afinfo = { | 136 | static struct xfrm_state_afinfo xfrm6_state_afinfo = { |
137 | .family = AF_INET6, | 137 | .family = AF_INET6, |
138 | .lock = RW_LOCK_UNLOCKED, | ||
139 | .init_tempsel = __xfrm6_init_tempsel, | 138 | .init_tempsel = __xfrm6_init_tempsel, |
140 | .state_lookup = __xfrm6_state_lookup, | 139 | .state_lookup = __xfrm6_state_lookup, |
141 | .find_acq = __xfrm6_find_acq, | 140 | .find_acq = __xfrm6_find_acq, |
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c index b469c8b54613..44b64a593c01 100644 --- a/net/xfrm/xfrm_policy.c +++ b/net/xfrm/xfrm_policy.c | |||
@@ -46,45 +46,43 @@ static DEFINE_SPINLOCK(xfrm_policy_gc_lock); | |||
46 | 46 | ||
47 | static struct xfrm_policy_afinfo *xfrm_policy_get_afinfo(unsigned short family); | 47 | static struct xfrm_policy_afinfo *xfrm_policy_get_afinfo(unsigned short family); |
48 | static void xfrm_policy_put_afinfo(struct xfrm_policy_afinfo *afinfo); | 48 | static void xfrm_policy_put_afinfo(struct xfrm_policy_afinfo *afinfo); |
49 | static struct xfrm_policy_afinfo *xfrm_policy_lock_afinfo(unsigned int family); | ||
50 | static void xfrm_policy_unlock_afinfo(struct xfrm_policy_afinfo *afinfo); | ||
49 | 51 | ||
50 | int xfrm_register_type(struct xfrm_type *type, unsigned short family) | 52 | int xfrm_register_type(struct xfrm_type *type, unsigned short family) |
51 | { | 53 | { |
52 | struct xfrm_policy_afinfo *afinfo = xfrm_policy_get_afinfo(family); | 54 | struct xfrm_policy_afinfo *afinfo = xfrm_policy_lock_afinfo(family); |
53 | struct xfrm_type_map *typemap; | 55 | struct xfrm_type **typemap; |
54 | int err = 0; | 56 | int err = 0; |
55 | 57 | ||
56 | if (unlikely(afinfo == NULL)) | 58 | if (unlikely(afinfo == NULL)) |
57 | return -EAFNOSUPPORT; | 59 | return -EAFNOSUPPORT; |
58 | typemap = afinfo->type_map; | 60 | typemap = afinfo->type_map; |
59 | 61 | ||
60 | write_lock_bh(&typemap->lock); | 62 | if (likely(typemap[type->proto] == NULL)) |
61 | if (likely(typemap->map[type->proto] == NULL)) | 63 | typemap[type->proto] = type; |
62 | typemap->map[type->proto] = type; | ||
63 | else | 64 | else |
64 | err = -EEXIST; | 65 | err = -EEXIST; |
65 | write_unlock_bh(&typemap->lock); | 66 | xfrm_policy_unlock_afinfo(afinfo); |
66 | xfrm_policy_put_afinfo(afinfo); | ||
67 | return err; | 67 | return err; |
68 | } | 68 | } |
69 | EXPORT_SYMBOL(xfrm_register_type); | 69 | EXPORT_SYMBOL(xfrm_register_type); |
70 | 70 | ||
71 | int xfrm_unregister_type(struct xfrm_type *type, unsigned short family) | 71 | int xfrm_unregister_type(struct xfrm_type *type, unsigned short family) |
72 | { | 72 | { |
73 | struct xfrm_policy_afinfo *afinfo = xfrm_policy_get_afinfo(family); | 73 | struct xfrm_policy_afinfo *afinfo = xfrm_policy_lock_afinfo(family); |
74 | struct xfrm_type_map *typemap; | 74 | struct xfrm_type **typemap; |
75 | int err = 0; | 75 | int err = 0; |
76 | 76 | ||
77 | if (unlikely(afinfo == NULL)) | 77 | if (unlikely(afinfo == NULL)) |
78 | return -EAFNOSUPPORT; | 78 | return -EAFNOSUPPORT; |
79 | typemap = afinfo->type_map; | 79 | typemap = afinfo->type_map; |
80 | 80 | ||
81 | write_lock_bh(&typemap->lock); | 81 | if (unlikely(typemap[type->proto] != type)) |
82 | if (unlikely(typemap->map[type->proto] != type)) | ||
83 | err = -ENOENT; | 82 | err = -ENOENT; |
84 | else | 83 | else |
85 | typemap->map[type->proto] = NULL; | 84 | typemap[type->proto] = NULL; |
86 | write_unlock_bh(&typemap->lock); | 85 | xfrm_policy_unlock_afinfo(afinfo); |
87 | xfrm_policy_put_afinfo(afinfo); | ||
88 | return err; | 86 | return err; |
89 | } | 87 | } |
90 | EXPORT_SYMBOL(xfrm_unregister_type); | 88 | EXPORT_SYMBOL(xfrm_unregister_type); |
@@ -92,7 +90,7 @@ EXPORT_SYMBOL(xfrm_unregister_type); | |||
92 | struct xfrm_type *xfrm_get_type(u8 proto, unsigned short family) | 90 | struct xfrm_type *xfrm_get_type(u8 proto, unsigned short family) |
93 | { | 91 | { |
94 | struct xfrm_policy_afinfo *afinfo; | 92 | struct xfrm_policy_afinfo *afinfo; |
95 | struct xfrm_type_map *typemap; | 93 | struct xfrm_type **typemap; |
96 | struct xfrm_type *type; | 94 | struct xfrm_type *type; |
97 | int modload_attempted = 0; | 95 | int modload_attempted = 0; |
98 | 96 | ||
@@ -102,11 +100,9 @@ retry: | |||
102 | return NULL; | 100 | return NULL; |
103 | typemap = afinfo->type_map; | 101 | typemap = afinfo->type_map; |
104 | 102 | ||
105 | read_lock(&typemap->lock); | 103 | type = typemap[proto]; |
106 | type = typemap->map[proto]; | ||
107 | if (unlikely(type && !try_module_get(type->owner))) | 104 | if (unlikely(type && !try_module_get(type->owner))) |
108 | type = NULL; | 105 | type = NULL; |
109 | read_unlock(&typemap->lock); | ||
110 | if (!type && !modload_attempted) { | 106 | if (!type && !modload_attempted) { |
111 | xfrm_policy_put_afinfo(afinfo); | 107 | xfrm_policy_put_afinfo(afinfo); |
112 | request_module("xfrm-type-%d-%d", | 108 | request_module("xfrm-type-%d-%d", |
@@ -1306,17 +1302,31 @@ static struct xfrm_policy_afinfo *xfrm_policy_get_afinfo(unsigned short family) | |||
1306 | return NULL; | 1302 | return NULL; |
1307 | read_lock(&xfrm_policy_afinfo_lock); | 1303 | read_lock(&xfrm_policy_afinfo_lock); |
1308 | afinfo = xfrm_policy_afinfo[family]; | 1304 | afinfo = xfrm_policy_afinfo[family]; |
1309 | if (likely(afinfo != NULL)) | 1305 | if (unlikely(!afinfo)) |
1310 | read_lock(&afinfo->lock); | 1306 | read_unlock(&xfrm_policy_afinfo_lock); |
1311 | read_unlock(&xfrm_policy_afinfo_lock); | ||
1312 | return afinfo; | 1307 | return afinfo; |
1313 | } | 1308 | } |
1314 | 1309 | ||
1315 | static void xfrm_policy_put_afinfo(struct xfrm_policy_afinfo *afinfo) | 1310 | static void xfrm_policy_put_afinfo(struct xfrm_policy_afinfo *afinfo) |
1316 | { | 1311 | { |
1317 | if (unlikely(afinfo == NULL)) | 1312 | read_unlock(&xfrm_policy_afinfo_lock); |
1318 | return; | 1313 | } |
1319 | read_unlock(&afinfo->lock); | 1314 | |
1315 | static struct xfrm_policy_afinfo *xfrm_policy_lock_afinfo(unsigned int family) | ||
1316 | { | ||
1317 | struct xfrm_policy_afinfo *afinfo; | ||
1318 | if (unlikely(family >= NPROTO)) | ||
1319 | return NULL; | ||
1320 | write_lock_bh(&xfrm_policy_afinfo_lock); | ||
1321 | afinfo = xfrm_policy_afinfo[family]; | ||
1322 | if (unlikely(!afinfo)) | ||
1323 | write_unlock_bh(&xfrm_policy_afinfo_lock); | ||
1324 | return afinfo; | ||
1325 | } | ||
1326 | |||
1327 | static void xfrm_policy_unlock_afinfo(struct xfrm_policy_afinfo *afinfo) | ||
1328 | { | ||
1329 | write_unlock_bh(&xfrm_policy_afinfo_lock); | ||
1320 | } | 1330 | } |
1321 | 1331 | ||
1322 | static int xfrm_dev_event(struct notifier_block *this, unsigned long event, void *ptr) | 1332 | static int xfrm_dev_event(struct notifier_block *this, unsigned long event, void *ptr) |
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c index 93a2f36ad3db..ee62c239a7e3 100644 --- a/net/xfrm/xfrm_state.c +++ b/net/xfrm/xfrm_state.c | |||
@@ -1103,17 +1103,14 @@ static struct xfrm_state_afinfo *xfrm_state_get_afinfo(unsigned short family) | |||
1103 | return NULL; | 1103 | return NULL; |
1104 | read_lock(&xfrm_state_afinfo_lock); | 1104 | read_lock(&xfrm_state_afinfo_lock); |
1105 | afinfo = xfrm_state_afinfo[family]; | 1105 | afinfo = xfrm_state_afinfo[family]; |
1106 | if (likely(afinfo != NULL)) | 1106 | if (unlikely(!afinfo)) |
1107 | read_lock(&afinfo->lock); | 1107 | read_unlock(&xfrm_state_afinfo_lock); |
1108 | read_unlock(&xfrm_state_afinfo_lock); | ||
1109 | return afinfo; | 1108 | return afinfo; |
1110 | } | 1109 | } |
1111 | 1110 | ||
1112 | static void xfrm_state_put_afinfo(struct xfrm_state_afinfo *afinfo) | 1111 | static void xfrm_state_put_afinfo(struct xfrm_state_afinfo *afinfo) |
1113 | { | 1112 | { |
1114 | if (unlikely(afinfo == NULL)) | 1113 | read_unlock(&xfrm_state_afinfo_lock); |
1115 | return; | ||
1116 | read_unlock(&afinfo->lock); | ||
1117 | } | 1114 | } |
1118 | 1115 | ||
1119 | /* Temporarily located here until net/xfrm/xfrm_tunnel.c is created */ | 1116 | /* Temporarily located here until net/xfrm/xfrm_tunnel.c is created */ |