Home » Mailing list archives » linux-kernel » 2007 » December » 3

Re: [PATCH 3/3] [UDP6]: Counter increment on BH mode

Previous thread: [ANNOUNCE] The Linux Test Project has been Released for NOVEMBER 2007 by Subrata Modak on Monday, December 3, 2007 - 7:29 am. (1 message)

Next thread: hibernation issue with kernel 2.6.23-gentoo-r3 at T41 by Toralf on Monday, December 3, 2007 - 9:19 am. (11 messages)
[view original message]
From: Herbert Xu <herbert@...>
To: Alexey Kuznetsov <kuznet@...>
Cc: Wang Chen <wangchen@...>, Gerrit Renker <gerrit@...>, <davem@...>, <andi@...>, <netdev@...>, Linux Kernel Mailing List <linux-kernel@...>, Christoph Lameter <clameter@...>, Ingo Molnar <mingo@...>
Subject: Re: [PATCH 3/3] [UDP6]: Counter increment on BH mode
Date: Monday, December 3, 2007 - 7:54 am

Yes that would certainly be the obvious fix.  In other words, just use

smp_processor_id instead of the raw version.  I suppose the only issue

would be that disabling/enabling preemption isn't exactly cost-free and

we're going to be doing that for every single increment.


Hmm, wasn't someone else talking about a non-atomic version of atomic

ops lately (i.e., atomic with respect to the local CPU only)? Perhaps

this is the killer app for it :)


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

--
[view original message]
From: Herbert Xu <herbert@...>
To: Alexey Kuznetsov <kuznet@...>
Cc: Wang Chen <wangchen@...>, Gerrit Renker <gerrit@...>, <davem@...>, <andi@...>, <netdev@...>, Linux Kernel Mailing List <linux-kernel@...>, Christoph Lameter <clameter@...>, Ingo Molnar <mingo@...>
Subject: Re: [PATCH 3/3] [UDP6]: Counter increment on BH mode
Date: Monday, December 3, 2007 - 9:17 am

Never mind, we already have that in local_t and as Alexey correctly

points out, USER is still going to be the expensive variant with the

preempt_disable (well until BH gets threaded).  So how about this patch?


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

--

diff --git a/include/net/snmp.h b/include/net/snmp.h

index ea206bf..9444b54 100644

--- a/include/net/snmp.h

+++ b/include/net/snmp.h

@@ -23,6 +23,7 @@


 #include <linux/cache.h>

 #include <linux/snmp.h>

+#include <linux/smp.h>


 /*

  * Mibs are stored in array of unsigned long.

@@ -137,7 +138,10 @@ struct linux_mib {

 #define SNMP_INC_STATS_OFFSET_BH(mib, field, offset)	\

 	(per_cpu_ptr(mib[0], raw_smp_processor_id())->mibs[field + (offset)]++)

 #define SNMP_INC_STATS_USER(mib, field) \

-	(per_cpu_ptr(mib[1], raw_smp_processor_id())->mibs[field]++)

+	do { \

+		per_cpu_ptr(mib[1], get_cpu())->mibs[field]++; \

+		put_cpu(); \

+	} while (0)

 #define SNMP_INC_STATS(mib, field) 	\

 	(per_cpu_ptr(mib[!in_softirq()], raw_smp_processor_id())->mibs[field]++)

 #define SNMP_DEC_STATS(mib, field) 	\

@@ -145,6 +149,9 @@ struct linux_mib {

 #define SNMP_ADD_STATS_BH(mib, field, addend) 	\

 	(per_cpu_ptr(mib[0], raw_smp_processor_id())->mibs[field] += addend)

 #define SNMP_ADD_STATS_USER(mib, field, addend) 	\

-	(per_cpu_ptr(mib[1], raw_smp_processor_id())->mibs[field] += addend)

+	do { \

+		per_cpu_ptr(mib[1], get_cpu())->mibs[field] += addend; \

+		put_cpu(); \

+	} while (0)


 #endif

--
[view original message]
From: Herbert Xu <herbert@...>
To: Alexey Kuznetsov <kuznet@...>
Cc: Wang Chen <wangchen@...>, Gerrit Renker <gerrit@...>, <davem@...>, <andi@...>, <netdev@...>, Linux Kernel Mailing List <linux-kernel@...>, Christoph Lameter <clameter@...>, Ingo Molnar <mingo@...>
Subject: Re: [PATCH 3/3] [UDP6]: Counter increment on BH mode
Date: Saturday, December 15, 2007 - 9:58 am

I didn't hear any objections so here is the patch again.


[SNMP]: Fix SNMP counters with PREEMPT


The SNMP macros use raw_smp_processor_id() in process context

which is illegal because the process may be preempted and then

migrated to another CPU.


This patch makes it use get_cpu/put_cpu to disable preemption.


Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>


Cheers,

--

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

--

diff --git a/include/net/snmp.h b/include/net/snmp.h

index ea206bf..9444b54 100644

--- a/include/net/snmp.h

+++ b/include/net/snmp.h

@@ -23,6 +23,7 @@


 #include <linux/cache.h>

 #include <linux/snmp.h>

+#include <linux/smp.h>


 /*

  * Mibs are stored in array of unsigned long.

@@ -137,7 +138,10 @@ struct linux_mib {

 #define SNMP_INC_STATS_OFFSET_BH(mib, field, offset)	\

 	(per_cpu_ptr(mib[0], raw_smp_processor_id())->mibs[field + (offset)]++)

 #define SNMP_INC_STATS_USER(mib, field) \

-	(per_cpu_ptr(mib[1], raw_smp_processor_id())->mibs[field]++)

+	do { \

+		per_cpu_ptr(mib[1], get_cpu())->mibs[field]++; \

+		put_cpu(); \

+	} while (0)

 #define SNMP_INC_STATS(mib, field) 	\

 	(per_cpu_ptr(mib[!in_softirq()], raw_smp_processor_id())->mibs[field]++)

 #define SNMP_DEC_STATS(mib, field) 	\

@@ -145,6 +149,9 @@ struct linux_mib {

 #define SNMP_ADD_STATS_BH(mib, field, addend) 	\

 	(per_cpu_ptr(mib[0], raw_smp_processor_id())->mibs[field] += addend)

 #define SNMP_ADD_STATS_USER(mib, field, addend) 	\

-	(per_cpu_ptr(mib[1], raw_smp_processor_id())->mibs[field] += addend)

+	do { \

+		per_cpu_ptr(mib[1], get_cpu())->mibs[field] += addend; \

+		put_cpu(); \

+	} while (0)


 #endif

--
[view original message]
From: David Miller <davem@...>
To: <herbert@...>
Cc: <kuznet@...>, <wangchen@...>, <gerrit@...>, <andi@...>, <netdev@...>, <linux-kernel@...>, <clameter@...>, <mingo@...>
Subject: Re: [PATCH 3/3] [UDP6]: Counter increment on BH mode
Date: Thursday, December 20, 2007 - 8:12 am

From: Herbert Xu <herbert@gondor.apana.org.au>


Applied to net-2.6.25, thanks.

--
[view original message]
From: Wang Chen <wangchen@...>
To: Herbert Xu <herbert@...>
Cc: Alexey Kuznetsov <kuznet@...>, Gerrit Renker <gerrit@...>, <davem@...>, <andi@...>, <netdev@...>, Linux Kernel Mailing List <linux-kernel@...>, Christoph Lameter <clameter@...>, Ingo Molnar <mingo@...>
Subject: Re: [PATCH 3/3] [UDP6]: Counter increment on BH mode
Date: Monday, December 3, 2007 - 11:50 pm

It's no need to include smp.h?


--

WCN


--
[view original message]
From: Herbert Xu <herbert@...>
To: Wang Chen <wangchen@...>
Cc: Alexey Kuznetsov <kuznet@...>, Gerrit Renker <gerrit@...>, <davem@...>, <andi@...>, <netdev@...>, Linux Kernel Mailing List <linux-kernel@...>, Christoph Lameter <clameter@...>, Ingo Molnar <mingo@...>
Subject: Re: [PATCH 3/3] [UDP6]: Counter increment on BH mode
Date: Monday, December 3, 2007 - 11:57 pm

We need it for smp_processor_id.  Well we needed it before too but

there's probably some implicit inclusion which has made it work.

It's better to declare these inclusions explicitly as otherwise

they may break on less common architectures later.


Cheers,

--

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

--
Previous thread: [ANNOUNCE] The Linux Test Project has been Released for NOVEMBER 2007 by Subrata Modak on Monday, December 3, 2007 - 7:29 am. (1 message)

Next thread: hibernation issue with kernel 2.6.23-gentoo-r3 at T41 by Toralf on Monday, December 3, 2007 - 9:19 am. (11 messages)