net: make ip_rt_acct a normal percpu var

There's no reason for this to be dynamically allocated: we panic if allocation fails anyway.

Signed-off-by Rusty Russell <[email protected]>

diff -r 643c2cdfdb62 include/net/route.h

include/net/route.h +2 -1

@@ -34,6 +34,7 @@
 #include <linux/ip.h>
 #include <linux/cache.h>
 #include <linux/security.h>
+#include <linux/percpu.h>
 
 #ifndef __KERNEL__
 #warning This file is not supposed to be used outside of kernel.

@@ -105,7 +106,7 @@
         unsigned int out_hlist_search;
 };
 
-extern struct ip_rt_acct *ip_rt_acct;
+DECLARE_PER_CPU(struct ip_rt_acct[256], ip_rt_acct);
 
 struct in_device;
 extern int		ip_rt_init(void);

diff -r 643c2cdfdb62 net/ipv4/ip_input.c

net/ipv4/ip_input.c +1 -1

@@ -339,7 +339,7 @@
 
 #ifdef CONFIG_NET_CLS_ROUTE
 	if (unlikely(skb->dst->tclassid)) {
-		struct ip_rt_acct *st = per_cpu_ptr(ip_rt_acct, smp_processor_id());
+		struct ip_rt_acct *st = __get_cpu_var(ip_rt_acct);
 		u32 idx = skb->dst->tclassid;
 		st[idx&0xFF].o_packets++;
 		st[idx&0xFF].o_bytes+=skb->len;

diff -r 643c2cdfdb62 net/ipv4/route.c

net/ipv4/route.c +2 -8

@@ -541,7 +541,7 @@
 			unsigned int j;
 			u32 *src;
 
-			src = ((u32 *) per_cpu_ptr(ip_rt_acct, i)) + offset;
+			src = ((u32 *)per_cpu(ip_rt_acct, i)) + offset;
 			for (j = 0; j < length/4; j++)
 				dst[j] += src[j];
 		}

@@ -3237,7 +3237,7 @@
 
 
 #ifdef CONFIG_NET_CLS_ROUTE
-struct ip_rt_acct *ip_rt_acct __read_mostly;
+DEFINE_PER_CPU(struct ip_rt_acct[256], ip_rt_acct);
 #endif /* CONFIG_NET_CLS_ROUTE */
 
 static __initdata unsigned long rhash_entries;

@@ -3253,12 +3253,6 @@
 int __init ip_rt_init(void)
 {
 	int rc = 0;
-
-#ifdef CONFIG_NET_CLS_ROUTE
-	ip_rt_acct = __alloc_percpu(256 * sizeof(struct ip_rt_acct));
-	if (!ip_rt_acct)
-		panic("IP: failed to allocate ip_rt_acct\n");
-#endif
 
 	ipv4_dst_ops.kmem_cachep =
 		kmem_cache_create("ip_dst_cache", sizeof(struct rtable), 0,

���{.n�+�������+%�����ݶ��w��{.n�+���z�^����ܨ}���Ơz�&j:+v�������w����2�ޙ���&�)ߡ�a������z��z�ޗ�+��ݢj��w��f

Quoted 4 lines · Rusty Russell

There's no reason for this to be dynamically allocated: we panic if allocation fails anyway.

Signed-off-by Rusty Russell <[email protected]>

Well, since we dont have "per_cpu bss", this also grow the size of vmlinux file, with a block of 4096 nul bytes.

Maybe with current machines/boot loaders, we dont care anymore, I dont know...

Quoted 6 lines

Quoted 4 lines · Rusty Russell

There's no reason for this to be dynamically allocated: we panic if allocation fails anyway.

Signed-off-by Rusty Russell <[email protected]>

Quoted 2 lines

Well, since we dont have "per_cpu bss", this also grow the size of vmlinux file, with a block of 4096 nul bytes.

...

Quoted 2 lines

Maybe with current machines/boot loaders, we dont care anymore, I dont know...

Good q. If people care about on-disk size, maybe we should look at bzImage size?

Do you really need this to forward some work you are doing? If not can we just let sleeping dogs lie on this one? :)

Quoted 2 lines

Do you really need this to forward some work you are doing? If not can we just let sleeping dogs lie on this one? :)

Yes, I have patches to convert the dynamic percpu data to use the same mechanism as static percpu data. Unfortunately we don't have a mechanism for enlarging the percpu region (which is why this wasn't done earlier), so we use a heuristic to figure out how much extra percpu region to allocate at boot.

And 4k makes this one of the Big Pigs in dynamic per-cpu allocations.

(SNMP mibs are even worse, but that's a separate debate...)

I can try to implement a bss-like DEFINE_PER_CPU_ZERO(), but it seems silly to talk about tight boot loader size restrictions for SMP kernels.

We make a big fuss (rightly) about a few hundred bytes and this sucker is FOUR KILOBYTES.

Really for the time being I'd rather see this converted to a num_possible_cpus() sized normal kzalloc() and direct indexing. I don't want the networking to bloat up the main kernel image by so much.

Quoted 2 lines · On Thursday 20 November 2008 08:50:23 David Miller wrote:

Do you really need this to forward some work you are doing? If not can we just let sleeping dogs lie on this one? :)

Yes, I have patches to convert the dynamic percpu data to use the same mechanism as static percpu data. Unfortunately we don't have a mechanism for enlarging the percpu region (which is why this wasn't done earlier), so we use a heuristic to figure out how much extra percpu region to allocate at boot.

And 4k makes this one of the Big Pigs in dynamic per-cpu allocations.

(SNMP mibs are even worse, but that's a separate debate...)

I can try to implement a bss-like DEFINE_PER_CPU_ZERO(), but it seems silly to talk about tight boot loader size restrictions for SMP kernels.

Quoted 10 lines · Rusty Russell

Quoted 2 lines · On Thursday 20 November 2008 08:50:23 David Miller wrote:

Do you really need this to forward some work you are doing? If not can we just let sleeping dogs lie on this one? :)

Yes, I have patches to convert the dynamic percpu data to use the same mechanism as static percpu data. Unfortunately we don't have a mechanism for enlarging the percpu region (which is why this wasn't done earlier), so we use a heuristic to figure out how much extra percpu region to allocate at boot.

Quoted 6 lines

And 4k makes this one of the Big Pigs in dynamic per-cpu allocations.

(SNMP mibs are even worse, but that's a separate debate...)

I can try to implement a bss-like DEFINE_PER_CPU_ZERO(), but it seems silly to talk about tight boot loader size restrictions for SMP kernels.

Then, if we really want to run 4096 cpus on a machine, we dont want to allocate 16 MBytes of memory for these ip_rt_acct counters, or even more for SNMP mibs.

Maybe its time to design a new mechanism, to avoid the basic "one variable" shared by all cpus, and avoid the overkill "one separate variable for each cpu", and loop 4096 times to do the sum of this variable...