Hi,
Here's a simple patch to change sockets_in_use to make use of
per-cpu areas.
I have couple of questions though...
1. Was this var made per-cpu just to avoid atomic_t or locking
or are there real life workloads which cause too many sock_alloc
and sock_releases to cause cacheline bouncing?
2. Is this var required? since we can just sum up all proto->stats.inuse
and remove this var altogether? (This var is read only for /proc
reporting)
Thanks,
Kiran
diff -ruN -X dontdiff linux-2.5.52/net/socket.c sockets_in_use-2.5.52/net/socket.c
--- linux-2.5.52/net/socket.c Mon Dec 16 07:37:53 2002
+++ sockets_in_use-2.5.52/net/socket.c Mon Dec 23 11:48:44 2002
@@ -189,10 +189,7 @@
* Statistics counters of the socket lists
*/
-static union {
- int counter;
- char __pad[SMP_CACHE_BYTES];
-} sockets_in_use[NR_CPUS] __cacheline_aligned = {{0}};
+static DEFINE_PER_CPU(int, sockets_in_use);
/*
* Support routines. Move socket addresses back and forth across the kernel/user
@@ -475,7 +472,8 @@
inode->i_uid = current->fsuid;
inode->i_gid = current->fsgid;
- sockets_in_use[smp_processor_id()].counter++;
+ get_cpu_var(sockets_in_use)++;
+ put_cpu_var(sockets_in_use);
return sock;
}
@@ -511,7 +509,8 @@
if (sock->fasync_list)
printk(KERN_ERR "sock_release: fasync list not empty!\n");
- sockets_in_use[smp_processor_id()].counter--;
+ get_cpu_var(sockets_in_use)--;
+ put_cpu_var(sockets_in_use);
if (!sock->file) {
iput(SOCK_INODE(sock));
return;
@@ -1851,7 +1850,7 @@
int counter = 0;
for (cpu = 0; cpu < NR_CPUS; cpu++)
- counter += sockets_in_use[cpu].counter;
+ counter += per_cpu(sockets_in_use, cpu);
/* It can be negative, by the way. 8) */
if (counter < 0)
From: Ravikiran G Thirumalai <[email protected]>
Date: Mon, 23 Dec 2002 19:08:48 +0530
-static union {
- int counter;
- char __pad[SMP_CACHE_BYTES];
-} sockets_in_use[NR_CPUS] __cacheline_aligned = {{0}};
+static DEFINE_PER_CPU(int, sockets_in_use);
You have to provide an explicit initializer for DEFINE_PER_CPU
declarations or you break some platforms with older GCC's which
otherwise won't put it into the proper section.
On Mon, Dec 23, 2002 at 12:16:32PM -0800, David S. Miller wrote:
> You have to provide an explicit initializer for DEFINE_PER_CPU
> declarations or you break some platforms with older GCC's which
> otherwise won't put it into the proper section.
I wonder if "some platforms with older GCC's" will ever have these
issues resolved...
--cw
From: Chris Wedgwood <[email protected]>
Date: Mon, 23 Dec 2002 16:00:48 -0800
On Mon, Dec 23, 2002 at 12:16:32PM -0800, David S. Miller wrote:
> You have to provide an explicit initializer for DEFINE_PER_CPU
> declarations or you break some platforms with older GCC's which
> otherwise won't put it into the proper section.
I wonder if "some platforms with older GCC's" will ever have these
issues resolved...
I still don't have gcc-3.2.1 working properly on sparc64.
I hope to have it working soon, but this does mean that 2.6.x
cannot deprecate it, whereas 2.7.x certainly can.
On Mon, Dec 23, 2002 at 12:16:32PM -0800, David S. Miller wrote:
>...
> -static union {
> - int counter;
> - char __pad[SMP_CACHE_BYTES];
> -} sockets_in_use[NR_CPUS] __cacheline_aligned = {{0}};
> +static DEFINE_PER_CPU(int, sockets_in_use);
>
> You have to provide an explicit initializer for DEFINE_PER_CPU
> declarations or you break some platforms with older GCC's which
> otherwise won't put it into the proper section.
>
Ok, here's the modified patch...
Thanks,
Kiran
diff -ruN -X dontdiff linux-2.5.52/net/socket.c sockets_in_use-2.5.52/net/socket.c
--- linux-2.5.52/net/socket.c Mon Dec 16 07:37:53 2002
+++ sockets_in_use-2.5.52/net/socket.c Tue Dec 24 05:11:36 2002
@@ -189,10 +189,7 @@
* Statistics counters of the socket lists
*/
-static union {
- int counter;
- char __pad[SMP_CACHE_BYTES];
-} sockets_in_use[NR_CPUS] __cacheline_aligned = {{0}};
+static DEFINE_PER_CPU(int, sockets_in_use) = 0;
/*
* Support routines. Move socket addresses back and forth across the kernel/user
@@ -475,7 +472,8 @@
inode->i_uid = current->fsuid;
inode->i_gid = current->fsgid;
- sockets_in_use[smp_processor_id()].counter++;
+ get_cpu_var(sockets_in_use)++;
+ put_cpu_var(sockets_in_use);
return sock;
}
@@ -511,7 +509,8 @@
if (sock->fasync_list)
printk(KERN_ERR "sock_release: fasync list not empty!\n");
- sockets_in_use[smp_processor_id()].counter--;
+ get_cpu_var(sockets_in_use)--;
+ put_cpu_var(sockets_in_use);
if (!sock->file) {
iput(SOCK_INODE(sock));
return;
@@ -1851,7 +1850,7 @@
int counter = 0;
for (cpu = 0; cpu < NR_CPUS; cpu++)
- counter += sockets_in_use[cpu].counter;
+ counter += per_cpu(sockets_in_use, cpu);
/* It can be negative, by the way. 8) */
if (counter < 0)
From: Ravikiran G Thirumalai <[email protected]>
Date: Tue, 24 Dec 2002 12:18:52 +0530
Ok, here's the modified patch...
Ok I'll apply this when I get back from vacation in the
new year.