[RFC][PATCH] Turn part of SNMP accounting macros into functions

[RFC][PATCH] Turn part of SNMP accounting macros into functions

[Pavel Emelyanov]

After turning IP_XXX_STATS, TCP_XXX_STATS and NET_XXX_STATS from
macros into functions the net/ipv4/built-in.o shrank significantly:

add/remove: 14/0 grow/shrink: 0/67 up/down: 482/-2246 (-1764)

Turning the CONFIG_NET_NS option on makes this shrink even larger:

add/remove: 14/0 grow/shrink: 0/67 up/down: 478/-2646 (-2168)

So the question is - what was the reason to keep those as macros?
I thought about the possible performance questions, but netperf
didn't show any (I admit I just cannot cook it properly).

The sample patch is here, but it's not good (EXPORTs for ipv6
and a better place for functions rather than net/ipv4/af_inet.c
are required).

Signed-off-by: Pavel Emelyanov <xemul@openvz.org>

diff --git a/include/net/ip.h b/include/net/ip.h
index 250e6ef..ad8ef7b 100644
--- a/include/net/ip.h
+++ b/include/net/ip.h
@@ -156,14 +156,16 @@ struct ipv4_config
 };
 
 extern struct ipv4_config ipv4_config;
-#define IP_INC_STATS(net, field)	SNMP_INC_STATS((net)->mib.ip_statistics, field)
-#define IP_INC_STATS_BH(net, field)	SNMP_INC_STATS_BH((net)->mib.ip_statistics, field)
-#define IP_ADD_STATS_BH(net, field, val) SNMP_ADD_STATS_BH((net)->mib.ip_statistics, field, val)
-#define NET_INC_STATS(net, field)	SNMP_INC_STATS((net)->mib.net_statistics, field)
-#define NET_INC_STATS_BH(net, field)	SNMP_INC_STATS_BH((net)->mib.net_statistics, field)
-#define NET_INC_STATS_USER(net, field) 	SNMP_INC_STATS_USER((net)->mib.net_statistics, field)
-#define NET_ADD_STATS_BH(net, field, adnd) SNMP_ADD_STATS_BH((net)->mib.net_statistics, field, adnd)
-#define NET_ADD_STATS_USER(net, field, adnd) SNMP_ADD_STATS_USER((net)->mib.net_statistics, field, adnd)
+
+void IP_INC_STATS(struct net *net, int field);
+void IP_INC_STATS_BH(struct net *net, int field);
+void IP_ADD_STATS_BH(struct net *net, int field, int val);
+
+void NET_INC_STATS(struct net *net, int field);
+void NET_INC_STATS_BH(struct net *net, int field);
+void NET_INC_STATS_USER(struct net *net, int field);
+void NET_ADD_STATS_BH(struct net *net, int field, int val);
+void NET_ADD_STATS_USER(struct net *net, int field, int val);
 
 extern unsigned long snmp_fold_field(void *mib[], int offt);
 extern int snmp_mib_init(void *ptr[2], size_t mibsize);
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 8983386..b973e68 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -267,10 +267,10 @@ static inline int tcp_too_many_orphans(struct sock *sk, int num)
 
 extern struct proto tcp_prot;
 
-#define TCP_INC_STATS(net, field)	SNMP_INC_STATS((net)->mib.tcp_statistics, field)
-#define TCP_INC_STATS_BH(net, field)	SNMP_INC_STATS_BH((net)->mib.tcp_statistics, field)
-#define TCP_DEC_STATS(net, field)	SNMP_DEC_STATS((net)->mib.tcp_statistics, field)
-#define TCP_ADD_STATS_USER(net, field, val) SNMP_ADD_STATS_USER((net)->mib.tcp_statistics, field, val)
+void TCP_INC_STATS(struct net *net, int field);
+void TCP_INC_STATS_BH(struct net *net, int field);
+void TCP_DEC_STATS(struct net *net, int field);
+void TCP_ADD_STATS_USER(struct net *net, int field, int val);
 
 extern void			tcp_v4_err(struct sk_buff *skb, u32);
 
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 8a3ac1f..413adda 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1580,3 +1580,63 @@ EXPORT_SYMBOL(inet_stream_connect);
 EXPORT_SYMBOL(inet_stream_ops);
 EXPORT_SYMBOL(inet_unregister_protosw);
 EXPORT_SYMBOL(sysctl_ip_nonlocal_bind);
+
+void NET_INC_STATS(struct net *net, int field)
+{
+	SNMP_INC_STATS((net)->mib.net_statistics, field);
+}
+
+void NET_INC_STATS_BH(struct net *net, int field)
+{
+	SNMP_INC_STATS_BH((net)->mib.net_statistics, field);
+}
+
+void NET_INC_STATS_USER(struct net *net, int field)
+{
+	SNMP_INC_STATS_USER((net)->mib.net_statistics, field);
+}
+
+void NET_ADD_STATS_BH(struct net *net, int field, int adnd)
+{
+	SNMP_ADD_STATS_BH((net)->mib.net_statistics, field, adnd);
+}
+
+void NET_ADD_STATS_USER(struct net *net, int field, int adnd)
+{
+	SNMP_ADD_STATS_USER((net)->mib.net_statistics, field, adnd);
+}
+
+void IP_INC_STATS(struct net *net, int field)
+{
+	SNMP_INC_STATS((net)->mib.ip_statistics, field);
+}
+
+void IP_INC_STATS_BH(struct net *net, int field)
+{
+	SNMP_INC_STATS_BH((net)->mib.ip_statistics, field);
+}
+
+void IP_ADD_STATS_BH(struct net *net, int field, int val)
+{
+	SNMP_ADD_STATS_BH((net)->mib.ip_statistics, field, val);
+}
+
+void TCP_INC_STATS(struct net *net, int field)
+{
+	SNMP_INC_STATS((net)->mib.tcp_statistics, field);
+}
+
+void TCP_INC_STATS_BH(struct net *net, int field)
+{
+	SNMP_INC_STATS_BH((net)->mib.tcp_statistics, field);
+}
+
+void TCP_DEC_STATS(struct net *net, int field)
+{
+	SNMP_DEC_STATS((net)->mib.tcp_statistics, field);
+}
+
+void TCP_ADD_STATS_USER(struct net *net, int field, int val)
+{
+	SNMP_ADD_STATS_USER((net)->mib.tcp_statistics, field, val);
+}

Re: [RFC][PATCH] Turn part of SNMP accounting macros into functions

[Stephen Hemminger]

On Wed, 27 Aug 2008 20:31:25 +0400
Pavel Emelyanov <xemul@openvz.org> wrote:

> After turning IP_XXX_STATS, TCP_XXX_STATS and NET_XXX_STATS from
> macros into functions the net/ipv4/built-in.o shrank significantly:
> 
> add/remove: 14/0 grow/shrink: 0/67 up/down: 482/-2246 (-1764)
> 
> Turning the CONFIG_NET_NS option on makes this shrink even larger:
> 
> add/remove: 14/0 grow/shrink: 0/67 up/down: 478/-2646 (-2168)
> 
> So the question is - what was the reason to keep those as macros?
> I thought about the possible performance questions, but netperf
> didn't show any (I admit I just cannot cook it properly).
> 
> The sample patch is here, but it's not good (EXPORTs for ipv6
> and a better place for functions rather than net/ipv4/af_inet.c
> are required).
> 
> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
> 
> diff --git a/include/net/ip.h b/include/net/ip.h
> index 250e6ef..ad8ef7b 100644
> --- a/include/net/ip.h
> +++ b/include/net/ip.h
> @@ -156,14 +156,16 @@ struct ipv4_config
>  };
>  
>  extern struct ipv4_config ipv4_config;
> -#define IP_INC_STATS(net, field)	SNMP_INC_STATS((net)->mib.ip_statistics, field)
> -#define IP_INC_STATS_BH(net, field)	SNMP_INC_STATS_BH((net)->mib.ip_statistics, field)
> -#define IP_ADD_STATS_BH(net, field, val) SNMP_ADD_STATS_BH((net)->mib.ip_statistics, field, val)
> -#define NET_INC_STATS(net, field)	SNMP_INC_STATS((net)->mib.net_statistics, field)
> -#define NET_INC_STATS_BH(net, field)	SNMP_INC_STATS_BH((net)->mib.net_statistics, field)
> -#define NET_INC_STATS_USER(net, field) 	SNMP_INC_STATS_USER((net)->mib.net_statistics, field)
> -#define NET_ADD_STATS_BH(net, field, adnd) SNMP_ADD_STATS_BH((net)->mib.net_statistics, field, adnd)
> -#define NET_ADD_STATS_USER(net, field, adnd) SNMP_ADD_STATS_USER((net)->mib.net_statistics, field, adnd)
> +
> +void IP_INC_STATS(struct net *net, int field);
> +void IP_INC_STATS_BH(struct net *net, int field);
> +void IP_ADD_STATS_BH(struct net *net, int field, int val);
> +
> +void NET_INC_STATS(struct net *net, int field);
> +void NET_INC_STATS_BH(struct net *net, int field);
> +void NET_INC_STATS_USER(struct net *net, int field);
> +void NET_ADD_STATS_BH(struct net *net, int field, int val);
> +void NET_ADD_STATS_USER(struct net *net, int field, int val);
>  
>  extern unsigned long snmp_fold_field(void *mib[], int offt);
>  extern int snmp_mib_init(void *ptr[2], size_t mibsize);
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index 8983386..b973e68 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -267,10 +267,10 @@ static inline int tcp_too_many_orphans(struct sock *sk, int num)
>  
>  extern struct proto tcp_prot;
>  
> -#define TCP_INC_STATS(net, field)	SNMP_INC_STATS((net)->mib.tcp_statistics, field)
> -#define TCP_INC_STATS_BH(net, field)	SNMP_INC_STATS_BH((net)->mib.tcp_statistics, field)
> -#define TCP_DEC_STATS(net, field)	SNMP_DEC_STATS((net)->mib.tcp_statistics, field)
> -#define TCP_ADD_STATS_USER(net, field, val) SNMP_ADD_STATS_USER((net)->mib.tcp_statistics, field, val)
> +void TCP_INC_STATS(struct net *net, int field);
> +void TCP_INC_STATS_BH(struct net *net, int field);
> +void TCP_DEC_STATS(struct net *net, int field);
> +void TCP_ADD_STATS_USER(struct net *net, int field, int val);
>  
>  extern void			tcp_v4_err(struct sk_buff *skb, u32);
>  
> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> index 8a3ac1f..413adda 100644
> --- a/net/ipv4/af_inet.c
> +++ b/net/ipv4/af_inet.c
> @@ -1580,3 +1580,63 @@ EXPORT_SYMBOL(inet_stream_connect);
>  EXPORT_SYMBOL(inet_stream_ops);
>  EXPORT_SYMBOL(inet_unregister_protosw);
>  EXPORT_SYMBOL(sysctl_ip_nonlocal_bind);
> +
> +void NET_INC_STATS(struct net *net, int field)
> +{
> +	SNMP_INC_STATS((net)->mib.net_statistics, field);
> +}
> +
> +void NET_INC_STATS_BH(struct net *net, int field)
> +{
> +	SNMP_INC_STATS_BH((net)->mib.net_statistics, field);
> +}
> +
> +void NET_INC_STATS_USER(struct net *net, int field)
> +{
> +	SNMP_INC_STATS_USER((net)->mib.net_statistics, field);
> +}
> +
> +void NET_ADD_STATS_BH(struct net *net, int field, int adnd)
> +{
> +	SNMP_ADD_STATS_BH((net)->mib.net_statistics, field, adnd);
> +}
> +
> +void NET_ADD_STATS_USER(struct net *net, int field, int adnd)
> +{
> +	SNMP_ADD_STATS_USER((net)->mib.net_statistics, field, adnd);
> +}
> +
> +void IP_INC_STATS(struct net *net, int field)
> +{
> +	SNMP_INC_STATS((net)->mib.ip_statistics, field);
> +}
> +
> +void IP_INC_STATS_BH(struct net *net, int field)
> +{
> +	SNMP_INC_STATS_BH((net)->mib.ip_statistics, field);
> +}
> +
> +void IP_ADD_STATS_BH(struct net *net, int field, int val)
> +{
> +	SNMP_ADD_STATS_BH((net)->mib.ip_statistics, field, val);
> +}
> +
> +void TCP_INC_STATS(struct net *net, int field)
> +{
> +	SNMP_INC_STATS((net)->mib.tcp_statistics, field);
> +}
> +
> +void TCP_INC_STATS_BH(struct net *net, int field)
> +{
> +	SNMP_INC_STATS_BH((net)->mib.tcp_statistics, field);
> +}
> +
> +void TCP_DEC_STATS(struct net *net, int field)
> +{
> +	SNMP_DEC_STATS((net)->mib.tcp_statistics, field);
> +}
> +
> +void TCP_ADD_STATS_USER(struct net *net, int field, int val)
> +{
> +	SNMP_ADD_STATS_USER((net)->mib.tcp_statistics, field, val);

Change function names to lower case please.

Re: [RFC][PATCH] Turn part of SNMP accounting macros into functions

[Ilpo Järvinen]

On Wed, 27 Aug 2008, Pavel Emelyanov wrote:

> After turning IP_XXX_STATS, TCP_XXX_STATS and NET_XXX_STATS from
> macros into functions the net/ipv4/built-in.o shrank significantly:
> 
> add/remove: 14/0 grow/shrink: 0/67 up/down: 482/-2246 (-1764)
> 
> Turning the CONFIG_NET_NS option on makes this shrink even larger:
> 
> add/remove: 14/0 grow/shrink: 0/67 up/down: 478/-2646 (-2168)

Is there perhaps some DEBUG related config that could bloat these numbers?
...I didn't come up with anything obvious when I quickly looked the 
definations but I might have missed something.

> So the question is - what was the reason to keep those as macros?
> I thought about the possible performance questions, but netperf
> didn't show any (I admit I just cannot cook it properly).

IMHO, even if there would be some performance regression, only the
most hottest paths would need to have it inlined, the rest would be
quite fine having it as a function.

--
 i.

Re: [RFC][PATCH] Turn part of SNMP accounting macros into functions

[Pavel Emelyanov]

Ilpo Järvinen wrote:
> On Wed, 27 Aug 2008, Pavel Emelyanov wrote:
> 
>> After turning IP_XXX_STATS, TCP_XXX_STATS and NET_XXX_STATS from
>> macros into functions the net/ipv4/built-in.o shrank significantly:
>>
>> add/remove: 14/0 grow/shrink: 0/67 up/down: 482/-2246 (-1764)
>>
>> Turning the CONFIG_NET_NS option on makes this shrink even larger:
>>
>> add/remove: 14/0 grow/shrink: 0/67 up/down: 478/-2646 (-2168)
> 
> Is there perhaps some DEBUG related config that could bloat these numbers?

Nope. I only had a DEBUG_BUG_VERBOSE turned on in my experiments.

> ...I didn't come up with anything obvious when I quickly looked the 
> definations but I might have missed something.

Well, looking at NET_INC_STATS_BH as a function

000000a9 <NET_INC_STATS_BH>:
      a9:       8b 40 74                mov    0x74(%eax),%eax
      ac:       64 8b 0d 00 00 00 00    mov    %fs:0x0,%ecx
                        af: R_386_32    per_cpu__cpu_number
      b3:       f7 d0                   not    %eax
      b5:       8b 04 88                mov    (%eax,%ecx,4),%eax
      b8:       ff 04 90                incl   (%eax,%edx,4)
      bb:       c3                      ret

I guess that the heaviest thing there is smp_processor_id, which 
is 7 bytes out of 19 total.

Actually the complete bloat-o-meter.sh output showed that there were 
~10 functions, that became 8 bytes liter, ~10 that became 12-16 bytes
liter and so on. This makes me think, that the code in the macro
itself is pretty small, but is used widely in the networking code,
and this spread makes such a big bloat.

>> So the question is - what was the reason to keep those as macros?
>> I thought about the possible performance questions, but netperf
>> didn't show any (I admit I just cannot cook it properly).
> 
> IMHO, even if there would be some performance regression, only the
> most hottest paths would need to have it inlined, the rest would be
> quite fine having it as a function.

OK, thanks Ilpo. I'll then look at how to make this more beautiful 
and send to Dave.

> --
>  i.
> 

Re: [RFC][PATCH] Turn part of SNMP accounting macros into functions

[Ilpo Järvinen]

On Wed, 27 Aug 2008, Pavel Emelyanov wrote:

> Actually the complete bloat-o-meter.sh output showed that there were 
> ~10 functions, that became 8 bytes liter, ~10 that became 12-16 bytes
> liter and so on. This makes me think, that the code in the macro
> itself is pretty small, but is used widely in the networking code,
> and this spread makes such a big bloat.

I once measured all static inlines in the headers and the largest bloaters  
where of that type where the function itself is quite small but it's 
called from so many places that it add up.

-- 
 i.

Re: [RFC][PATCH] Turn part of SNMP accounting macros into functions

[Eric Dumazet]

Pavel Emelyanov a écrit :
> After turning IP_XXX_STATS, TCP_XXX_STATS and NET_XXX_STATS from
> macros into functions the net/ipv4/built-in.o shrank significantly:
> 
> add/remove: 14/0 grow/shrink: 0/67 up/down: 482/-2246 (-1764)
> 
> Turning the CONFIG_NET_NS option on makes this shrink even larger:
> 
> add/remove: 14/0 grow/shrink: 0/67 up/down: 478/-2646 (-2168)
> 
> So the question is - what was the reason to keep those as macros?
> I thought about the possible performance questions, but netperf
> didn't show any (I admit I just cannot cook it properly).
> 
> The sample patch is here, but it's not good (EXPORTs for ipv6
> and a better place for functions rather than net/ipv4/af_inet.c
> are required).
> 
> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
> 


I dont know, but passing all those "struct net *net" to every 
network function in the kernel sounds overkill, especially for
!CONFIG_NET_NS users. This is pure bloat.

We could define two macros so that function prototypes dont include
useless pointers, especially on arches where only first and second
parameter is passed in eax and edx register ;)

#ifdef CONFIG_NET_NS
# define VNETPTR ,struct net *net
# define NETPTR  net
#else
# define VNETPTR
# defint NETPTR  &init_net
#endif

...
void TCP_DEC_STATS(int field VNETPTR);
...
void TCP_DEC_STATS(int field VNETPTR)
{
	SNMP_DEC_STATS((NETPTR)->mib.tcp_statistics, field);
}
...

Re: [RFC][PATCH] Turn part of SNMP accounting macros into functions

[Pavel Emelyanov]

Eric Dumazet wrote:
> Pavel Emelyanov a écrit :
>> After turning IP_XXX_STATS, TCP_XXX_STATS and NET_XXX_STATS from
>> macros into functions the net/ipv4/built-in.o shrank significantly:
>>
>> add/remove: 14/0 grow/shrink: 0/67 up/down: 482/-2246 (-1764)
>>
>> Turning the CONFIG_NET_NS option on makes this shrink even larger:
>>
>> add/remove: 14/0 grow/shrink: 0/67 up/down: 478/-2646 (-2168)
>>
>> So the question is - what was the reason to keep those as macros?
>> I thought about the possible performance questions, but netperf
>> didn't show any (I admit I just cannot cook it properly).
>>
>> The sample patch is here, but it's not good (EXPORTs for ipv6
>> and a better place for functions rather than net/ipv4/af_inet.c
>> are required).
>>
>> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
>>
> 
> 
> I dont know, but passing all those "struct net *net" to every 
> network function in the kernel sounds overkill, especially for
> !CONFIG_NET_NS users. This is pure bloat.
> 
> We could define two macros so that function prototypes dont include
> useless pointers, especially on arches where only first and second
> parameter is passed in eax and edx register ;)
> 
> #ifdef CONFIG_NET_NS
> # define VNETPTR ,struct net *net
> # define NETPTR  net
> #else
> # define VNETPTR
> # defint NETPTR  &init_net
> #endif
> 
> ...
> void TCP_DEC_STATS(int field VNETPTR);
> ...
> void TCP_DEC_STATS(int field VNETPTR)
> {
> 	SNMP_DEC_STATS((NETPTR)->mib.tcp_statistics, field);
> }
> ...

I agree with this, but I've checked whether compiling out the first argument
of XXX_STATS would help and found out that there's almost no difference (-9
bytes) in the net/ipv4/built-in.o for NET_NS=n case. 

After turning the macros into functions, compiling the first argument out gives
us 500 more bytes out. Good catch, but I have a question - the way you proposed
to implement the function itself is rather nice, but how would you implement
the *call* to that function in order to make it variable-arguments depending on
the config option?

I see the similar way:

+#ifdef CONFIG_NET_NS
+#define NET_ARG(net) net,
+#else
+#define NET_ARG(net)
+#endif
...
-	TCP_INC_STATS(sock_net(sk), field);
+	TCP_INC_STATS(NET_ARG(sock_net(sk) field);

but do these 500 bytes compensate for this ugly style?

Re: [RFC][PATCH] Turn part of SNMP accounting macros into functions

[Eric Dumazet]

Pavel Emelyanov a écrit :
> Eric Dumazet wrote:
>>
>> I dont know, but passing all those "struct net *net" to every 
>> network function in the kernel sounds overkill, especially for
>> !CONFIG_NET_NS users. This is pure bloat.
>>
>> We could define two macros so that function prototypes dont include
>> useless pointers, especially on arches where only first and second
>> parameter is passed in eax and edx register ;)
>>
>> #ifdef CONFIG_NET_NS
>> # define VNETPTR ,struct net *net
>> # define NETPTR  net
>> #else
>> # define VNETPTR
>> # defint NETPTR  &init_net
>> #endif
>>
>> ...
>> void TCP_DEC_STATS(int field VNETPTR);
>> ...
>> void TCP_DEC_STATS(int field VNETPTR)
>> {
>> 	SNMP_DEC_STATS((NETPTR)->mib.tcp_statistics, field);
>> }
>> ...
> 
> I agree with this, but I've checked whether compiling out the first argument
> of XXX_STATS would help and found out that there's almost no difference (-9
> bytes) in the net/ipv4/built-in.o for NET_NS=n case. 
> 
> After turning the macros into functions, compiling the first argument out gives
> us 500 more bytes out. Good catch, but I have a question - the way you proposed
> to implement the function itself is rather nice, but how would you implement
> the *call* to that function in order to make it variable-arguments depending on
> the config option?
> 
> I see the similar way:
> 
> +#ifdef CONFIG_NET_NS
> +#define NET_ARG(net) net,
> +#else
> +#define NET_ARG(net)
> +#endif
> ...
> -	TCP_INC_STATS(sock_net(sk), field);
> +	TCP_INC_STATS(NET_ARG(sock_net(sk) field);
> 
> but do these 500 bytes compensate for this ugly style?

500 bytes here, but what about other uses in kernel ?

Sometime I wonder if a *global* pointer ('current' pointer for example is not passed to every kernel function that want access to current struture) would save many kbytes in vmlinux text. This way, !CONFIG_NET_NS overhead would completely vanish.

Oh well...

Re: [RFC][PATCH] Turn part of SNMP accounting macros into functions

[Pavel Emelyanov]

> Sometime I wonder if a *global* pointer ('current' pointer for example 
> is not passed to every kernel function that want access to current
> struture) would save many kbytes in vmlinux text. This way, !CONFIG_NET_NS 
> overhead would completely vanish.

:) I'm glad to hear, that we're not alone with such a view of how
this have to work. We at OpenVZ proposed the current_net macro from
the very beginning, but other netns developers decided to go the other
way :(

> Oh well...

Oh well, indeed...

Re: [RFC][PATCH] Turn part of SNMP accounting macros into functions

[Herbert Xu]

Pavel Emelyanov <xemul@openvz.org> wrote:
> After turning IP_XXX_STATS, TCP_XXX_STATS and NET_XXX_STATS from
> macros into functions the net/ipv4/built-in.o shrank significantly:
> 
> add/remove: 14/0 grow/shrink: 0/67 up/down: 482/-2246 (-1764)
> 
> Turning the CONFIG_NET_NS option on makes this shrink even larger:
> 
> add/remove: 14/0 grow/shrink: 0/67 up/down: 478/-2646 (-2168)
> 
> So the question is - what was the reason to keep those as macros?
> I thought about the possible performance questions, but netperf
> didn't show any (I admit I just cannot cook it properly).
> 
> The sample patch is here, but it's not good (EXPORTs for ipv6
> and a better place for functions rather than net/ipv4/af_inet.c
> are required).
> 
> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>

What happened to Christoph Lameter's new per-cpu stuff? That would
allows us to turn these into a single inc/add instruction.

So I think we should put this patch on the back-burner until the
status of the per-cpu stuff is settled.

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [RFC][PATCH] Turn part of SNMP accounting macros into functions

[Pavel Emelyanov]

Herbert Xu wrote:
> Pavel Emelyanov <xemul@openvz.org> wrote:
>> After turning IP_XXX_STATS, TCP_XXX_STATS and NET_XXX_STATS from
>> macros into functions the net/ipv4/built-in.o shrank significantly:
>>
>> add/remove: 14/0 grow/shrink: 0/67 up/down: 482/-2246 (-1764)
>>
>> Turning the CONFIG_NET_NS option on makes this shrink even larger:
>>
>> add/remove: 14/0 grow/shrink: 0/67 up/down: 478/-2646 (-2168)
>>
>> So the question is - what was the reason to keep those as macros?
>> I thought about the possible performance questions, but netperf
>> didn't show any (I admit I just cannot cook it properly).
>>
>> The sample patch is here, but it's not good (EXPORTs for ipv6
>> and a better place for functions rather than net/ipv4/af_inet.c
>> are required).
>>
>> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
> 
> What happened to Christoph Lameter's new per-cpu stuff? That would
> allows us to turn these into a single inc/add instruction.

I've never heard about it, but if you tell, that this would
make things *that* simple, then of course I will delay with
this patch.

> So I think we should put this patch on the back-burner until the
> status of the per-cpu stuff is settled.

OK, thanks.

> Thanks,

Re: [RFC][PATCH] Turn part of SNMP accounting macros into functions

[Christoph Lameter]

Herbert Xu wrote:

> What happened to Christoph Lameter's new per-cpu stuff? That would
> allows us to turn these into a single inc/add instruction.
> 
> So I think we should put this patch on the back-burner until the
> status of the per-cpu stuff is settled.

Well the zero based patchset is still not in for x86_64. The percpu stuff
would only work for x86_32. 32 bit uses the available segment register for per
cpu stuff. 64 bit wastes it on the pda.

Maybe its okay if itll just work for 32bit and it would put pressure on x86_64
to get the crap fixed up?