| Message ID | 1412109249-7432-1-git-send-email-fabf@skynet.be |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Fabian Frederick <fabf@skynet.be> Date: Tue, 30 Sep 2014 22:34:08 +0200 > lowpan_frags_sysctl_register is only called by __init lowpan_net_frag_init > (part of the lowpan module). > > Signed-off-by: Fabian Frederick <fabf@skynet.be> 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
Hi, On Tue, Sep 30, 2014 at 10:34:08PM +0200, Fabian Frederick wrote: > lowpan_frags_sysctl_register is only called by __init lowpan_net_frag_init > (part of the lowpan module). > > Signed-off-by: Fabian Frederick <fabf@skynet.be> > --- > This is untested. > > net/ieee802154/reassembly.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/net/ieee802154/reassembly.c b/net/ieee802154/reassembly.c > index 32755cb..30ec608 100644 > --- a/net/ieee802154/reassembly.c > +++ b/net/ieee802154/reassembly.c > @@ -485,7 +485,7 @@ static void __net_exit lowpan_frags_ns_sysctl_unregister(struct net *net) > > static struct ctl_table_header *lowpan_ctl_header; > > -static int lowpan_frags_sysctl_register(void) > +static int __init lowpan_frags_sysctl_register(void) yes right, but there is more lacks of missing "__init". See below. > { > lowpan_ctl_header = register_net_sysctl(&init_net, > "net/ieee802154/6lowpan", > @@ -498,7 +498,7 @@ static void lowpan_frags_sysctl_unregister(void) > unregister_net_sysctl_table(lowpan_ctl_header); > } > #else > -static inline int lowpan_frags_ns_sysctl_register(struct net *net) > +static inline int __init lowpan_frags_ns_sysctl_register(struct net *net) This is wrong, it's callback from "struct pernet_operations lowpan_frags_ops". > { > return 0; > } Your patch adds "__init" now for two different functions and we have two different declarations for these functions, depends if CONFIG_SYSCTL is enabled. Now if CONFIG_SYSCTL select we have as lowpan_frags_sysctl_register declaration: static int __init lowpan_frags_sysctl_register(void) if not: static inline int lowpan_frags_sysctl_register(void) Same for lowpan_frags_ns_sysctl_register and vice versa for CONFIG_SYSCTL. Your changes are for two different functions (Don't know if you realized that): "lowpan_frags_sysctl_register" and "lowpan_frags_ns_sysctl_register", which makes no sense. Also lowpan_frags_ns_sysctl_register isn't called by module init which is wrong. To make "correct" cleanup for this "__init" should be add to the functions, for both declarations if CONFIG_SYSCTL is set or not: - lowpan_net_frag_init - lowpan_frags_sysctl_register I see now it's already applied, David please revert this change. Are you fine to apply a correct version of this to wpan-next tree, next time? - Alex -- 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 01 October 2014 at 02:25 Alexander Aring <alex.aring@gmail.com> wrote: > > > Hi, > > On Tue, Sep 30, 2014 at 10:34:08PM +0200, Fabian Frederick wrote: > > lowpan_frags_sysctl_register is only called by __init lowpan_net_frag_init > > (part of the lowpan module). > > > > Signed-off-by: Fabian Frederick <fabf@skynet.be> > > --- > > This is untested. > > > > net/ieee802154/reassembly.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/net/ieee802154/reassembly.c b/net/ieee802154/reassembly.c > > index 32755cb..30ec608 100644 > > --- a/net/ieee802154/reassembly.c > > +++ b/net/ieee802154/reassembly.c > > @@ -485,7 +485,7 @@ static void __net_exit > > lowpan_frags_ns_sysctl_unregister(struct net *net) > > > > static struct ctl_table_header *lowpan_ctl_header; > > > > -static int lowpan_frags_sysctl_register(void) > > +static int __init lowpan_frags_sysctl_register(void) > > yes right, but there is more lacks of missing "__init". See below. > > > { > > lowpan_ctl_header = register_net_sysctl(&init_net, > > "net/ieee802154/6lowpan", > > @@ -498,7 +498,7 @@ static void lowpan_frags_sysctl_unregister(void) > > unregister_net_sysctl_table(lowpan_ctl_header); > > } > > #else > > -static inline int lowpan_frags_ns_sysctl_register(struct net *net) > > +static inline int __init lowpan_frags_ns_sysctl_register(struct net *net) > > This is wrong, it's callback from "struct pernet_operations lowpan_frags_ops". > > > { > > return 0; > > } > > > Your patch adds "__init" now for two different functions and we have > two different declarations for these functions, depends if CONFIG_SYSCTL is > enabled. > > Now if CONFIG_SYSCTL select we have as lowpan_frags_sysctl_register > declaration: > > static int __init lowpan_frags_sysctl_register(void) > > if not: > > static inline int lowpan_frags_sysctl_register(void) > > Same for lowpan_frags_ns_sysctl_register and vice versa for CONFIG_SYSCTL. > > Your changes are for two different functions (Don't know if you realized > that): Hi Alexander, I didn't see _ns_ . I'll send the right fix based on wpan-next. Thanks, Fabian > > "lowpan_frags_sysctl_register" and "lowpan_frags_ns_sysctl_register", > which makes no sense. Also lowpan_frags_ns_sysctl_register isn't called > by module init which is wrong. > > > To make "correct" cleanup for this "__init" should be add to the functions, > for > both declarations if CONFIG_SYSCTL is set or not: > > - lowpan_net_frag_init > - lowpan_frags_sysctl_register > > > I see now it's already applied, David please revert this change. Are you > fine to apply a correct version of this to wpan-next tree, next time? > > - Alex -- 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
From: Fabian Frederick <fabf@skynet.be> Date: Wed, 1 Oct 2014 06:36:49 +0200 (CEST) > I didn't see _ns_ . I'll send the right fix based on wpan-next. You'll have to send it to me, because I applied your patch already to net-next. -- 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
diff --git a/net/ieee802154/reassembly.c b/net/ieee802154/reassembly.c index 32755cb..30ec608 100644 --- a/net/ieee802154/reassembly.c +++ b/net/ieee802154/reassembly.c @@ -485,7 +485,7 @@ static void __net_exit lowpan_frags_ns_sysctl_unregister(struct net *net) static struct ctl_table_header *lowpan_ctl_header; -static int lowpan_frags_sysctl_register(void) +static int __init lowpan_frags_sysctl_register(void) { lowpan_ctl_header = register_net_sysctl(&init_net, "net/ieee802154/6lowpan", @@ -498,7 +498,7 @@ static void lowpan_frags_sysctl_unregister(void) unregister_net_sysctl_table(lowpan_ctl_header); } #else -static inline int lowpan_frags_ns_sysctl_register(struct net *net) +static inline int __init lowpan_frags_ns_sysctl_register(struct net *net) { return 0; }
lowpan_frags_sysctl_register is only called by __init lowpan_net_frag_init (part of the lowpan module). Signed-off-by: Fabian Frederick <fabf@skynet.be> --- This is untested. net/ieee802154/reassembly.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)