* [Patch net-next 1/2] xfrm: replace rwlock on xfrm_state_afinfo with rcu @ 2013-01-16 8:05 Cong Wang 2013-01-16 8:05 ` [Patch net-next 2/2] xfrm: replace rwlock on xfrm_km_list " Cong Wang 2013-01-16 13:57 ` [Patch net-next 1/2] xfrm: replace rwlock on xfrm_state_afinfo with rcu YOSHIFUJI Hideaki 0 siblings, 2 replies; 6+ messages in thread From: Cong Wang @ 2013-01-16 8:05 UTC (permalink / raw) To: netdev; +Cc: Steffen Klassert, Herbert Xu, David S. Miller, Cong Wang From: Cong Wang <amwang@redhat.com> Similar to commit 418a99ac6ad487dc9c42e6b0e85f941af56330f2 (Replace rwlock on xfrm_policy_afinfo with rcu), the rwlock on xfrm_state_afinfo can be replaced by RCU too. Cc: Steffen Klassert <steffen.klassert@secunet.com> Cc: Herbert Xu <herbert@gondor.apana.org.au> Cc: "David S. Miller" <davem@davemloft.net> Signed-off-by: Cong Wang <amwang@redhat.com> --- net/xfrm/xfrm_state.c | 33 ++++++++++++++++----------------- 1 files changed, 16 insertions(+), 17 deletions(-) diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c index 3459692..f567716 100644 --- a/net/xfrm/xfrm_state.c +++ b/net/xfrm/xfrm_state.c @@ -158,8 +158,8 @@ out_unlock: mutex_unlock(&hash_resize_mutex); } -static DEFINE_RWLOCK(xfrm_state_afinfo_lock); -static struct xfrm_state_afinfo *xfrm_state_afinfo[NPROTO]; +static DEFINE_SPINLOCK(xfrm_state_afinfo_lock); +static struct xfrm_state_afinfo __rcu *xfrm_state_afinfo[NPROTO]; static DEFINE_SPINLOCK(xfrm_state_gc_lock); @@ -173,17 +173,16 @@ static struct xfrm_state_afinfo *xfrm_state_lock_afinfo(unsigned int family) struct xfrm_state_afinfo *afinfo; if (unlikely(family >= NPROTO)) return NULL; - write_lock_bh(&xfrm_state_afinfo_lock); + spin_lock_bh(&xfrm_state_afinfo_lock); afinfo = xfrm_state_afinfo[family]; if (unlikely(!afinfo)) - write_unlock_bh(&xfrm_state_afinfo_lock); + spin_unlock_bh(&xfrm_state_afinfo_lock); return afinfo; } static void xfrm_state_unlock_afinfo(struct xfrm_state_afinfo *afinfo) - __releases(xfrm_state_afinfo_lock) { - write_unlock_bh(&xfrm_state_afinfo_lock); + spin_unlock_bh(&xfrm_state_afinfo_lock); } int xfrm_register_type(const struct xfrm_type *type, unsigned short family) @@ -1848,12 +1847,12 @@ int xfrm_state_register_afinfo(struct xfrm_state_afinfo *afinfo) return -EINVAL; if (unlikely(afinfo->family >= NPROTO)) return -EAFNOSUPPORT; - write_lock_bh(&xfrm_state_afinfo_lock); + spin_lock_bh(&xfrm_state_afinfo_lock); if (unlikely(xfrm_state_afinfo[afinfo->family] != NULL)) err = -ENOBUFS; else - xfrm_state_afinfo[afinfo->family] = afinfo; - write_unlock_bh(&xfrm_state_afinfo_lock); + rcu_assign_pointer(xfrm_state_afinfo[afinfo->family], afinfo); + spin_unlock_bh(&xfrm_state_afinfo_lock); return err; } EXPORT_SYMBOL(xfrm_state_register_afinfo); @@ -1865,14 +1864,15 @@ int xfrm_state_unregister_afinfo(struct xfrm_state_afinfo *afinfo) return -EINVAL; if (unlikely(afinfo->family >= NPROTO)) return -EAFNOSUPPORT; - write_lock_bh(&xfrm_state_afinfo_lock); + spin_lock_bh(&xfrm_state_afinfo_lock); if (likely(xfrm_state_afinfo[afinfo->family] != NULL)) { if (unlikely(xfrm_state_afinfo[afinfo->family] != afinfo)) err = -EINVAL; else - xfrm_state_afinfo[afinfo->family] = NULL; + RCU_INIT_POINTER(xfrm_state_afinfo[afinfo->family], NULL); } - write_unlock_bh(&xfrm_state_afinfo_lock); + spin_unlock_bh(&xfrm_state_afinfo_lock); + synchronize_rcu(); return err; } EXPORT_SYMBOL(xfrm_state_unregister_afinfo); @@ -1882,17 +1882,16 @@ static struct xfrm_state_afinfo *xfrm_state_get_afinfo(unsigned int family) struct xfrm_state_afinfo *afinfo; if (unlikely(family >= NPROTO)) return NULL; - read_lock(&xfrm_state_afinfo_lock); - afinfo = xfrm_state_afinfo[family]; + rcu_read_lock(); + afinfo = rcu_dereference(xfrm_state_afinfo[family]); if (unlikely(!afinfo)) - read_unlock(&xfrm_state_afinfo_lock); + rcu_read_unlock(); return afinfo; } static void xfrm_state_put_afinfo(struct xfrm_state_afinfo *afinfo) - __releases(xfrm_state_afinfo_lock) { - read_unlock(&xfrm_state_afinfo_lock); + rcu_read_unlock(); } /* Temporarily located here until net/xfrm/xfrm_tunnel.c is created */ -- 1.7.7.6 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Patch net-next 2/2] xfrm: replace rwlock on xfrm_km_list with rcu 2013-01-16 8:05 [Patch net-next 1/2] xfrm: replace rwlock on xfrm_state_afinfo with rcu Cong Wang @ 2013-01-16 8:05 ` Cong Wang 2013-01-17 8:34 ` [Patch net-next 3/2] xfrm: use separated locks to protect pointers of struct xfrm_state_afinfo Cong Wang 2013-01-16 13:57 ` [Patch net-next 1/2] xfrm: replace rwlock on xfrm_state_afinfo with rcu YOSHIFUJI Hideaki 1 sibling, 1 reply; 6+ messages in thread From: Cong Wang @ 2013-01-16 8:05 UTC (permalink / raw) To: netdev; +Cc: Steffen Klassert, Herbert Xu, David S. Miller, Cong Wang From: Cong Wang <amwang@redhat.com> Cc: Steffen Klassert <steffen.klassert@secunet.com> Cc: Herbert Xu <herbert@gondor.apana.org.au> Cc: "David S. Miller" <davem@davemloft.net> Signed-off-by: Cong Wang <amwang@redhat.com> --- net/xfrm/xfrm_state.c | 58 +++++++++++++++++++++++++----------------------- 1 files changed, 30 insertions(+), 28 deletions(-) diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c index f567716..b40baf7 100644 --- a/net/xfrm/xfrm_state.c +++ b/net/xfrm/xfrm_state.c @@ -1647,27 +1647,26 @@ static void xfrm_replay_timer_handler(unsigned long data) } static LIST_HEAD(xfrm_km_list); -static DEFINE_RWLOCK(xfrm_km_lock); void km_policy_notify(struct xfrm_policy *xp, int dir, const struct km_event *c) { struct xfrm_mgr *km; - read_lock(&xfrm_km_lock); - list_for_each_entry(km, &xfrm_km_list, list) + rcu_read_lock(); + list_for_each_entry_rcu(km, &xfrm_km_list, list) if (km->notify_policy) km->notify_policy(xp, dir, c); - read_unlock(&xfrm_km_lock); + rcu_read_unlock(); } void km_state_notify(struct xfrm_state *x, const struct km_event *c) { struct xfrm_mgr *km; - read_lock(&xfrm_km_lock); - list_for_each_entry(km, &xfrm_km_list, list) + rcu_read_lock(); + list_for_each_entry_rcu(km, &xfrm_km_list, list) if (km->notify) km->notify(x, c); - read_unlock(&xfrm_km_lock); + rcu_read_unlock(); } EXPORT_SYMBOL(km_policy_notify); @@ -1697,13 +1696,13 @@ int km_query(struct xfrm_state *x, struct xfrm_tmpl *t, struct xfrm_policy *pol) int err = -EINVAL, acqret; struct xfrm_mgr *km; - read_lock(&xfrm_km_lock); - list_for_each_entry(km, &xfrm_km_list, list) { + rcu_read_lock(); + list_for_each_entry_rcu(km, &xfrm_km_list, list) { acqret = km->acquire(x, t, pol); if (!acqret) err = acqret; } - read_unlock(&xfrm_km_lock); + rcu_read_unlock(); return err; } EXPORT_SYMBOL(km_query); @@ -1713,14 +1712,14 @@ int km_new_mapping(struct xfrm_state *x, xfrm_address_t *ipaddr, __be16 sport) int err = -EINVAL; struct xfrm_mgr *km; - read_lock(&xfrm_km_lock); - list_for_each_entry(km, &xfrm_km_list, list) { + rcu_read_lock(); + list_for_each_entry_rcu(km, &xfrm_km_list, list) { if (km->new_mapping) err = km->new_mapping(x, ipaddr, sport); if (!err) break; } - read_unlock(&xfrm_km_lock); + rcu_read_unlock(); return err; } EXPORT_SYMBOL(km_new_mapping); @@ -1749,15 +1748,15 @@ int km_migrate(const struct xfrm_selector *sel, u8 dir, u8 type, int ret; struct xfrm_mgr *km; - read_lock(&xfrm_km_lock); - list_for_each_entry(km, &xfrm_km_list, list) { + rcu_read_lock(); + list_for_each_entry_rcu(km, &xfrm_km_list, list) { if (km->migrate) { ret = km->migrate(sel, dir, type, m, num_migrate, k); if (!ret) err = ret; } } - read_unlock(&xfrm_km_lock); + rcu_read_unlock(); return err; } EXPORT_SYMBOL(km_migrate); @@ -1769,15 +1768,15 @@ int km_report(struct net *net, u8 proto, struct xfrm_selector *sel, xfrm_address int ret; struct xfrm_mgr *km; - read_lock(&xfrm_km_lock); - list_for_each_entry(km, &xfrm_km_list, list) { + rcu_read_lock(); + list_for_each_entry_rcu(km, &xfrm_km_list, list) { if (km->report) { ret = km->report(net, proto, sel, addr); if (!ret) err = ret; } } - read_unlock(&xfrm_km_lock); + rcu_read_unlock(); return err; } EXPORT_SYMBOL(km_report); @@ -1801,14 +1800,14 @@ int xfrm_user_policy(struct sock *sk, int optname, u8 __user *optval, int optlen goto out; err = -EINVAL; - read_lock(&xfrm_km_lock); - list_for_each_entry(km, &xfrm_km_list, list) { + rcu_read_lock(); + list_for_each_entry_rcu(km, &xfrm_km_list, list) { pol = km->compile_policy(sk, optname, data, optlen, &err); if (err >= 0) break; } - read_unlock(&xfrm_km_lock); + rcu_read_unlock(); if (err >= 0) { xfrm_sk_policy_insert(sk, err, pol); @@ -1822,20 +1821,23 @@ out: } EXPORT_SYMBOL(xfrm_user_policy); +static DEFINE_SPINLOCK(xfrm_km_lock); + int xfrm_register_km(struct xfrm_mgr *km) { - write_lock_bh(&xfrm_km_lock); - list_add_tail(&km->list, &xfrm_km_list); - write_unlock_bh(&xfrm_km_lock); + spin_lock_bh(&xfrm_km_lock); + list_add_tail_rcu(&km->list, &xfrm_km_list); + spin_unlock_bh(&xfrm_km_lock); return 0; } EXPORT_SYMBOL(xfrm_register_km); int xfrm_unregister_km(struct xfrm_mgr *km) { - write_lock_bh(&xfrm_km_lock); - list_del(&km->list); - write_unlock_bh(&xfrm_km_lock); + spin_lock_bh(&xfrm_km_lock); + list_del_rcu(&km->list); + spin_unlock_bh(&xfrm_km_lock); + synchronize_rcu(); return 0; } EXPORT_SYMBOL(xfrm_unregister_km); -- 1.7.7.6 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Patch net-next 3/2] xfrm: use separated locks to protect pointers of struct xfrm_state_afinfo 2013-01-16 8:05 ` [Patch net-next 2/2] xfrm: replace rwlock on xfrm_km_list " Cong Wang @ 2013-01-17 8:34 ` Cong Wang 2013-01-18 8:54 ` Steffen Klassert 0 siblings, 1 reply; 6+ messages in thread From: Cong Wang @ 2013-01-17 8:34 UTC (permalink / raw) To: netdev; +Cc: Steffen Klassert, Herbert Xu, David S. Miller, Cong Wang (On top of my previous RCU convert patches) afinfo->type_map and afinfo->mode_map deserve separated locks, they are different things. We should just take RCU read lock to protect afinfo itself, but not for the inner pointers. Cc: Steffen Klassert <steffen.klassert@secunet.com> Cc: Herbert Xu <herbert@gondor.apana.org.au> Cc: "David S. Miller" <davem@davemloft.net> Signed-off-by: Cong Wang <amwang@redhat.com> --- diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c index b40baf7..727e357 100644 --- a/net/xfrm/xfrm_state.c +++ b/net/xfrm/xfrm_state.c @@ -168,57 +168,45 @@ int __xfrm_state_delete(struct xfrm_state *x); int km_query(struct xfrm_state *x, struct xfrm_tmpl *t, struct xfrm_policy *pol); void km_state_expired(struct xfrm_state *x, int hard, u32 portid); -static struct xfrm_state_afinfo *xfrm_state_lock_afinfo(unsigned int family) -{ - struct xfrm_state_afinfo *afinfo; - if (unlikely(family >= NPROTO)) - return NULL; - spin_lock_bh(&xfrm_state_afinfo_lock); - afinfo = xfrm_state_afinfo[family]; - if (unlikely(!afinfo)) - spin_unlock_bh(&xfrm_state_afinfo_lock); - return afinfo; -} - -static void xfrm_state_unlock_afinfo(struct xfrm_state_afinfo *afinfo) -{ - spin_unlock_bh(&xfrm_state_afinfo_lock); -} - +static DEFINE_SPINLOCK(xfrm_type_lock); int xfrm_register_type(const struct xfrm_type *type, unsigned short family) { - struct xfrm_state_afinfo *afinfo = xfrm_state_lock_afinfo(family); + struct xfrm_state_afinfo *afinfo = xfrm_state_get_afinfo(family); const struct xfrm_type **typemap; int err = 0; if (unlikely(afinfo == NULL)) return -EAFNOSUPPORT; typemap = afinfo->type_map; + spin_lock_bh(&xfrm_type_lock); if (likely(typemap[type->proto] == NULL)) typemap[type->proto] = type; else err = -EEXIST; - xfrm_state_unlock_afinfo(afinfo); + spin_unlock_bh(&xfrm_type_lock); + xfrm_state_put_afinfo(afinfo); return err; } EXPORT_SYMBOL(xfrm_register_type); int xfrm_unregister_type(const struct xfrm_type *type, unsigned short family) { - struct xfrm_state_afinfo *afinfo = xfrm_state_lock_afinfo(family); + struct xfrm_state_afinfo *afinfo = xfrm_state_get_afinfo(family); const struct xfrm_type **typemap; int err = 0; if (unlikely(afinfo == NULL)) return -EAFNOSUPPORT; typemap = afinfo->type_map; + spin_lock_bh(&xfrm_type_lock); if (unlikely(typemap[type->proto] != type)) err = -ENOENT; else typemap[type->proto] = NULL; - xfrm_state_unlock_afinfo(afinfo); + spin_unlock_bh(&xfrm_type_lock); + xfrm_state_put_afinfo(afinfo); return err; } EXPORT_SYMBOL(xfrm_unregister_type); @@ -255,6 +243,7 @@ static void xfrm_put_type(const struct xfrm_type *type) module_put(type->owner); } +static DEFINE_SPINLOCK(xfrm_mode_lock); int xfrm_register_mode(struct xfrm_mode *mode, int family) { struct xfrm_state_afinfo *afinfo; @@ -264,12 +253,13 @@ int xfrm_register_mode(struct xfrm_mode *mode, int family) if (unlikely(mode->encap >= XFRM_MODE_MAX)) return -EINVAL; - afinfo = xfrm_state_lock_afinfo(family); + afinfo = xfrm_state_get_afinfo(family); if (unlikely(afinfo == NULL)) return -EAFNOSUPPORT; err = -EEXIST; modemap = afinfo->mode_map; + spin_lock_bh(&xfrm_mode_lock); if (modemap[mode->encap]) goto out; @@ -282,7 +272,8 @@ int xfrm_register_mode(struct xfrm_mode *mode, int family) err = 0; out: - xfrm_state_unlock_afinfo(afinfo); + spin_unlock_bh(&xfrm_mode_lock); + xfrm_state_put_afinfo(afinfo); return err; } EXPORT_SYMBOL(xfrm_register_mode); @@ -296,19 +287,21 @@ int xfrm_unregister_mode(struct xfrm_mode *mode, int family) if (unlikely(mode->encap >= XFRM_MODE_MAX)) return -EINVAL; - afinfo = xfrm_state_lock_afinfo(family); + afinfo = xfrm_state_get_afinfo(family); if (unlikely(afinfo == NULL)) return -EAFNOSUPPORT; err = -ENOENT; modemap = afinfo->mode_map; + spin_lock_bh(&xfrm_mode_lock); if (likely(modemap[mode->encap] == mode)) { modemap[mode->encap] = NULL; module_put(mode->afinfo->owner); err = 0; } - xfrm_state_unlock_afinfo(afinfo); + spin_unlock_bh(&xfrm_mode_lock); + xfrm_state_put_afinfo(afinfo); return err; } EXPORT_SYMBOL(xfrm_unregister_mode); ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Patch net-next 3/2] xfrm: use separated locks to protect pointers of struct xfrm_state_afinfo 2013-01-17 8:34 ` [Patch net-next 3/2] xfrm: use separated locks to protect pointers of struct xfrm_state_afinfo Cong Wang @ 2013-01-18 8:54 ` Steffen Klassert 0 siblings, 0 replies; 6+ messages in thread From: Steffen Klassert @ 2013-01-18 8:54 UTC (permalink / raw) To: Cong Wang; +Cc: netdev, Herbert Xu, David S. Miller On Thu, Jan 17, 2013 at 04:34:11PM +0800, Cong Wang wrote: > (On top of my previous RCU convert patches) > > afinfo->type_map and afinfo->mode_map deserve separated locks, > they are different things. > > We should just take RCU read lock to protect afinfo itself, > but not for the inner pointers. > > Cc: Steffen Klassert <steffen.klassert@secunet.com> > Cc: Herbert Xu <herbert@gondor.apana.org.au> > Cc: "David S. Miller" <davem@davemloft.net> > Signed-off-by: Cong Wang <amwang@redhat.com> All Aplied to ipsec-next, thanks! ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Patch net-next 1/2] xfrm: replace rwlock on xfrm_state_afinfo with rcu 2013-01-16 8:05 [Patch net-next 1/2] xfrm: replace rwlock on xfrm_state_afinfo with rcu Cong Wang 2013-01-16 8:05 ` [Patch net-next 2/2] xfrm: replace rwlock on xfrm_km_list " Cong Wang @ 2013-01-16 13:57 ` YOSHIFUJI Hideaki 2013-01-17 6:22 ` Cong Wang 1 sibling, 1 reply; 6+ messages in thread From: YOSHIFUJI Hideaki @ 2013-01-16 13:57 UTC (permalink / raw) To: Cong Wang Cc: netdev, Steffen Klassert, Herbert Xu, David S. Miller, YOSHIFUJI Hideaki Cong Wang wrote: > From: Cong Wang <amwang@redhat.com> > > Similar to commit 418a99ac6ad487dc9c42e6b0e85f941af56330f2 > (Replace rwlock on xfrm_policy_afinfo with rcu), the rwlock > on xfrm_state_afinfo can be replaced by RCU too. > > Cc: Steffen Klassert <steffen.klassert@secunet.com> > Cc: Herbert Xu <herbert@gondor.apana.org.au> > Cc: "David S. Miller" <davem@davemloft.net> > Signed-off-by: Cong Wang <amwang@redhat.com> > --- > net/xfrm/xfrm_state.c | 33 ++++++++++++++++----------------- > 1 files changed, 16 insertions(+), 17 deletions(-) > > diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c > index 3459692..f567716 100644 > --- a/net/xfrm/xfrm_state.c > +++ b/net/xfrm/xfrm_state.c : > @@ -173,17 +173,16 @@ static struct xfrm_state_afinfo *xfrm_state_lock_afinfo(unsigned int family) > struct xfrm_state_afinfo *afinfo; > if (unlikely(family >= NPROTO)) > return NULL; > - write_lock_bh(&xfrm_state_afinfo_lock); > + spin_lock_bh(&xfrm_state_afinfo_lock); > afinfo = xfrm_state_afinfo[family]; > if (unlikely(!afinfo)) > - write_unlock_bh(&xfrm_state_afinfo_lock); > + spin_unlock_bh(&xfrm_state_afinfo_lock); > return afinfo; > } > > static void xfrm_state_unlock_afinfo(struct xfrm_state_afinfo *afinfo) > - __releases(xfrm_state_afinfo_lock) > { > - write_unlock_bh(&xfrm_state_afinfo_lock); > + spin_unlock_bh(&xfrm_state_afinfo_lock); > } > > int xfrm_register_type(const struct xfrm_type *type, unsigned short family) Why removing __releases() hint? --yoshfuji ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Patch net-next 1/2] xfrm: replace rwlock on xfrm_state_afinfo with rcu 2013-01-16 13:57 ` [Patch net-next 1/2] xfrm: replace rwlock on xfrm_state_afinfo with rcu YOSHIFUJI Hideaki @ 2013-01-17 6:22 ` Cong Wang 0 siblings, 0 replies; 6+ messages in thread From: Cong Wang @ 2013-01-17 6:22 UTC (permalink / raw) To: YOSHIFUJI Hideaki; +Cc: netdev, Steffen Klassert, Herbert Xu, David S. Miller On Wed, 2013-01-16 at 22:57 +0900, YOSHIFUJI Hideaki wrote: > Cong Wang wrote: > > > > static void xfrm_state_unlock_afinfo(struct xfrm_state_afinfo *afinfo) > > - __releases(xfrm_state_afinfo_lock) > > { > > - write_unlock_bh(&xfrm_state_afinfo_lock); > > + spin_unlock_bh(&xfrm_state_afinfo_lock); > > } > > > > int xfrm_register_type(const struct xfrm_type *type, unsigned short family) > > Why removing __releases() hint? There is no __acquires matched: % git grep __acquires -- net/xfrm <nothing output> ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-01-18 8:54 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-01-16 8:05 [Patch net-next 1/2] xfrm: replace rwlock on xfrm_state_afinfo with rcu Cong Wang 2013-01-16 8:05 ` [Patch net-next 2/2] xfrm: replace rwlock on xfrm_km_list " Cong Wang 2013-01-17 8:34 ` [Patch net-next 3/2] xfrm: use separated locks to protect pointers of struct xfrm_state_afinfo Cong Wang 2013-01-18 8:54 ` Steffen Klassert 2013-01-16 13:57 ` [Patch net-next 1/2] xfrm: replace rwlock on xfrm_state_afinfo with rcu YOSHIFUJI Hideaki 2013-01-17 6:22 ` Cong Wang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).