| Message ID | 1258124479-1167-1-git-send-email-remi@remlab.net |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Rémi Denis-Courmont <remi@remlab.net> Date: Fri, 13 Nov 2009 17:01:18 +0200 > From: Rémi Denis-Courmont <remi.denis-courmont@nokia.com> > > Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com> Applied. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Nov 13, 2009 at 05:01:18PM +0200, Rémi Denis-Courmont wrote: > From: Rémi Denis-Courmont <remi.denis-courmont@nokia.com> > > Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com> > --- > net/phonet/af_phonet.c | 20 +++++++++++--------- > 1 files changed, 11 insertions(+), 9 deletions(-) > > diff --git a/net/phonet/af_phonet.c b/net/phonet/af_phonet.c > index 8d3a55b..ed65da2 100644 > --- a/net/phonet/af_phonet.c > +++ b/net/phonet/af_phonet.c > @@ -35,7 +35,6 @@ > > /* Transport protocol registration */ > static struct phonet_protocol *proto_tab[PHONET_NPROTO] __read_mostly; > -static DEFINE_SPINLOCK(proto_tab_lock); > > static struct phonet_protocol *phonet_proto_get(int protocol) > { > @@ -44,11 +43,11 @@ static struct phonet_protocol *phonet_proto_get(int protocol) > if (protocol >= PHONET_NPROTO) > return NULL; > > - spin_lock(&proto_tab_lock); > + rcu_read_lock(); > pp = proto_tab[protocol]; Don't we need an rcu_dereference() in here somewhere? Perhaps something like the following? pp = rcu_dereference(proto_tab[protocol]); Thanx, Paul > if (pp && !try_module_get(pp->prot->owner)) > pp = NULL; > - spin_unlock(&proto_tab_lock); > + rcu_read_unlock(); > > return pp; > } > @@ -439,6 +438,8 @@ static struct packet_type phonet_packet_type __read_mostly = { > .func = phonet_rcv, > }; > > +static DEFINE_MUTEX(proto_tab_lock); > + > int __init_or_module phonet_proto_register(int protocol, > struct phonet_protocol *pp) > { > @@ -451,12 +452,12 @@ int __init_or_module phonet_proto_register(int protocol, > if (err) > return err; > > - spin_lock(&proto_tab_lock); > + mutex_lock(&proto_tab_lock); > if (proto_tab[protocol]) > err = -EBUSY; > else > - proto_tab[protocol] = pp; > - spin_unlock(&proto_tab_lock); > + rcu_assign_pointer(proto_tab[protocol], pp); > + mutex_unlock(&proto_tab_lock); > > return err; > } > @@ -464,10 +465,11 @@ EXPORT_SYMBOL(phonet_proto_register); > > void phonet_proto_unregister(int protocol, struct phonet_protocol *pp) > { > - spin_lock(&proto_tab_lock); > + mutex_lock(&proto_tab_lock); > BUG_ON(proto_tab[protocol] != pp); > - proto_tab[protocol] = NULL; > - spin_unlock(&proto_tab_lock); > + rcu_assign_pointer(proto_tab[protocol], NULL); > + mutex_unlock(&proto_tab_lock); > + synchronize_rcu(); > proto_unregister(pp->prot); > } > EXPORT_SYMBOL(phonet_proto_unregister); > -- > 1.6.3.3 > > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Le lundi 16 novembre 2009 19:26:17 Paul E. McKenney, vous avez écrit : > On Fri, Nov 13, 2009 at 05:01:18PM +0200, Rémi Denis-Courmont wrote: > > From: Rémi Denis-Courmont <remi.denis-courmont@nokia.com> > > > > Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com> > > --- > > net/phonet/af_phonet.c | 20 +++++++++++--------- > > 1 files changed, 11 insertions(+), 9 deletions(-) > > > > diff --git a/net/phonet/af_phonet.c b/net/phonet/af_phonet.c > > index 8d3a55b..ed65da2 100644 > > --- a/net/phonet/af_phonet.c > > +++ b/net/phonet/af_phonet.c > > @@ -35,7 +35,6 @@ > > > > /* Transport protocol registration */ > > static struct phonet_protocol *proto_tab[PHONET_NPROTO] __read_mostly; > > -static DEFINE_SPINLOCK(proto_tab_lock); > > > > static struct phonet_protocol *phonet_proto_get(int protocol) > > { > > @@ -44,11 +43,11 @@ static struct phonet_protocol *phonet_proto_get(int > > protocol) if (protocol >= PHONET_NPROTO) > > return NULL; > > > > - spin_lock(&proto_tab_lock); > > + rcu_read_lock(); > > pp = proto_tab[protocol]; > > Don't we need an rcu_dereference() in here somewhere? > > Perhaps something like the following? Err yes, we do.
diff --git a/net/phonet/af_phonet.c b/net/phonet/af_phonet.c index 8d3a55b..ed65da2 100644 --- a/net/phonet/af_phonet.c +++ b/net/phonet/af_phonet.c @@ -35,7 +35,6 @@ /* Transport protocol registration */ static struct phonet_protocol *proto_tab[PHONET_NPROTO] __read_mostly; -static DEFINE_SPINLOCK(proto_tab_lock); static struct phonet_protocol *phonet_proto_get(int protocol) { @@ -44,11 +43,11 @@ static struct phonet_protocol *phonet_proto_get(int protocol) if (protocol >= PHONET_NPROTO) return NULL; - spin_lock(&proto_tab_lock); + rcu_read_lock(); pp = proto_tab[protocol]; if (pp && !try_module_get(pp->prot->owner)) pp = NULL; - spin_unlock(&proto_tab_lock); + rcu_read_unlock(); return pp; } @@ -439,6 +438,8 @@ static struct packet_type phonet_packet_type __read_mostly = { .func = phonet_rcv, }; +static DEFINE_MUTEX(proto_tab_lock); + int __init_or_module phonet_proto_register(int protocol, struct phonet_protocol *pp) { @@ -451,12 +452,12 @@ int __init_or_module phonet_proto_register(int protocol, if (err) return err; - spin_lock(&proto_tab_lock); + mutex_lock(&proto_tab_lock); if (proto_tab[protocol]) err = -EBUSY; else - proto_tab[protocol] = pp; - spin_unlock(&proto_tab_lock); + rcu_assign_pointer(proto_tab[protocol], pp); + mutex_unlock(&proto_tab_lock); return err; } @@ -464,10 +465,11 @@ EXPORT_SYMBOL(phonet_proto_register); void phonet_proto_unregister(int protocol, struct phonet_protocol *pp) { - spin_lock(&proto_tab_lock); + mutex_lock(&proto_tab_lock); BUG_ON(proto_tab[protocol] != pp); - proto_tab[protocol] = NULL; - spin_unlock(&proto_tab_lock); + rcu_assign_pointer(proto_tab[protocol], NULL); + mutex_unlock(&proto_tab_lock); + synchronize_rcu(); proto_unregister(pp->prot); } EXPORT_SYMBOL(phonet_proto_unregister);