2022-03-08 12:23:00

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [saeed:net-next 154/185] net/smc/smc_sysctl.h:23:16: warning: no previous prototype for 'smc_sysctl_net_init'

On Tue, 8 Mar 2022 13:16:55 +0800 dust.li wrote:
> >vim +/smc_sysctl_net_init +23 net/smc/smc_sysctl.h
> >
> > 22
> > > 23 int __net_init smc_sysctl_net_init(struct net *net)
> > 24 {
> > 25 net->smc.sysctl_autocorking_size = SMC_AUTOCORKING_DEFAULT_SIZE;
> > 26 return 0;
> > 27 }
> > 28
> > > 29 void __net_exit smc_sysctl_net_exit(struct net *net) { }
> > 30
>
> Hi Jakub:
>
> Sorry to bother again on this !
> Looks like we still need to add 'static inline' or add an extra
> declaration for these 2 functions if we want to get rid of these warnings.
> What do you think ?

Sorry my comment was pretty unclear an unnecessary.

I meant that you don't need the __net_init annotation, it still
needs to be a static inline. So this is what I meant:

diff --git a/net/smc/smc_sysctl.h b/net/smc/smc_sysctl.h
index 1d554300604d..0becc11bd2f4 100644
--- a/net/smc/smc_sysctl.h
+++ b/net/smc/smc_sysctl.h
@@ -20,13 +20,13 @@ void __net_exit smc_sysctl_net_exit(struct net *net);

#else

-int __net_init smc_sysctl_net_init(struct net *net)
+static inline int smc_sysctl_net_init(struct net *net)
{
net->smc.sysctl_autocorking_size = SMC_AUTOCORKING_DEFAULT_SIZE;
return 0;
}

-void __net_exit smc_sysctl_net_exit(struct net *net) { }
+static inline void smc_sysctl_net_exit(struct net *net) { }

#endif /* CONFIG_SYSCTL */



But really it does not matter if the __net_init / exit is there,
so this works too:

diff --git a/net/smc/smc_sysctl.h b/net/smc/smc_sysctl.h
index 1d554300604d..6979e7173669 100644
--- a/net/smc/smc_sysctl.h
+++ b/net/smc/smc_sysctl.h
@@ -20,13 +20,13 @@ void __net_exit smc_sysctl_net_exit(struct net *net);

#else

-int __net_init smc_sysctl_net_init(struct net *net)
+static inline int __net_init smc_sysctl_net_init(struct net *net)
{
net->smc.sysctl_autocorking_size = SMC_AUTOCORKING_DEFAULT_SIZE;
return 0;
}

-void __net_exit smc_sysctl_net_exit(struct net *net) { }
+static inline void __net_exit smc_sysctl_net_exit(struct net *net) { }

#endif /* CONFIG_SYSCTL */


2022-03-09 03:30:20

by Dust Li

[permalink] [raw]
Subject: Re: [saeed:net-next 154/185] net/smc/smc_sysctl.h:23:16: warning: no previous prototype for 'smc_sysctl_net_init'

On Mon, Mar 07, 2022 at 09:25:20PM -0800, Jakub Kicinski wrote:
>On Tue, 8 Mar 2022 13:16:55 +0800 dust.li wrote:
>> >vim +/smc_sysctl_net_init +23 net/smc/smc_sysctl.h
>> >
>> > 22
>> > > 23 int __net_init smc_sysctl_net_init(struct net *net)
>> > 24 {
>> > 25 net->smc.sysctl_autocorking_size = SMC_AUTOCORKING_DEFAULT_SIZE;
>> > 26 return 0;
>> > 27 }
>> > 28
>> > > 29 void __net_exit smc_sysctl_net_exit(struct net *net) { }
>> > 30
>>
>> Hi Jakub:
>>
>> Sorry to bother again on this !
>> Looks like we still need to add 'static inline' or add an extra
>> declaration for these 2 functions if we want to get rid of these warnings.
>> What do you think ?
>
>Sorry my comment was pretty unclear an unnecessary.
>
>I meant that you don't need the __net_init annotation, it still
>needs to be a static inline. So this is what I meant:
>
>diff --git a/net/smc/smc_sysctl.h b/net/smc/smc_sysctl.h
>index 1d554300604d..0becc11bd2f4 100644
>--- a/net/smc/smc_sysctl.h
>+++ b/net/smc/smc_sysctl.h
>@@ -20,13 +20,13 @@ void __net_exit smc_sysctl_net_exit(struct net *net);
>
> #else
>
>-int __net_init smc_sysctl_net_init(struct net *net)
>+static inline int smc_sysctl_net_init(struct net *net)
> {
> net->smc.sysctl_autocorking_size = SMC_AUTOCORKING_DEFAULT_SIZE;
> return 0;
> }
>
>-void __net_exit smc_sysctl_net_exit(struct net *net) { }
>+static inline void smc_sysctl_net_exit(struct net *net) { }
>
> #endif /* CONFIG_SYSCTL */
>
>
>
>But really it does not matter if the __net_init / exit is there,
>so this works too:

Another small comment:

When re-compile with the W=1, I found '__net_exit' has noinline
annotation when CONFIG_NET_NS not set:

#define __net_exit __ref
#define __ref __section(".ref.text") noinline

If '__net_exit' is used together with 'static inline', the compiler
would give another warning:
net/smc/smc_sysctl.h:29:58: warning: ignoring attribute ‘gnu_inline’
because it conflicts with attribute ‘noinline’ [-Wattributes]
29 | static inline void __net_exit smc_sysctl_net_exit(struct net
*net) { }

So I think your first version would be good.

Thanks.

>
>diff --git a/net/smc/smc_sysctl.h b/net/smc/smc_sysctl.h
>index 1d554300604d..6979e7173669 100644
>--- a/net/smc/smc_sysctl.h
>+++ b/net/smc/smc_sysctl.h
>@@ -20,13 +20,13 @@ void __net_exit smc_sysctl_net_exit(struct net *net);
>
> #else
>
>-int __net_init smc_sysctl_net_init(struct net *net)
>+static inline int __net_init smc_sysctl_net_init(struct net *net)
> {
> net->smc.sysctl_autocorking_size = SMC_AUTOCORKING_DEFAULT_SIZE;
> return 0;
> }
>
>-void __net_exit smc_sysctl_net_exit(struct net *net) { }
>+static inline void __net_exit smc_sysctl_net_exit(struct net *net) { }
>
> #endif /* CONFIG_SYSCTL */
>