vmstat: Optimize zone counter modifications through the use of this cpu operations

vmstat: Optimize zone counter modifications through the use of this cpu operations

[Christoph Lameter]

this cpu operations can be used to slightly optimize the functions. The
changes will avoid some address calculations and replace them with the
use of the percpu segment register.

If one would have this_cpu_inc_return and this_cpu_dec_return then it
would be possible to optimize inc_zone_page_state and dec_zone_page_state even
more.

before:

linux-2.6$ size mm/vmstat.o
   text	   data	    bss	    dec	    hex	filename
   8914	    606	    264	   9784	   2638	mm/vmstat.o

after:

linux-2.6$ size mm/vmstat.o
   text	   data	    bss	    dec	    hex	filename
   8904	    606	    264	   9774	   262e	mm/vmstat.o

Signed-off-by: Christoph Lameter <cl@linux.com>

---
 mm/vmstat.c |   56 ++++++++++++++++++++++++++++++++------------------------
 1 file changed, 32 insertions(+), 24 deletions(-)

Index: linux-2.6/mm/vmstat.c
===================================================================
--- linux-2.6.orig/mm/vmstat.c	2010-11-09 11:18:22.000000000 -0600
+++ linux-2.6/mm/vmstat.c	2010-11-09 11:19:06.000000000 -0600
@@ -167,18 +167,20 @@ static void refresh_zone_stat_thresholds
 void __mod_zone_page_state(struct zone *zone, enum zone_stat_item item,
 				int delta)
 {
-	struct per_cpu_pageset *pcp = this_cpu_ptr(zone->pageset);
-
-	s8 *p = pcp->vm_stat_diff + item;
+	struct per_cpu_pageset * __percpu pcp = zone->pageset;
+	s8 * __percpu p = pcp->vm_stat_diff + item;
 	long x;
+	long t;
+
+	x = delta + __this_cpu_read(*p);

-	x = delta + *p;
+	t = __this_cpu_read(pcp->stat_threshold);

-	if (unlikely(x > pcp->stat_threshold || x < -pcp->stat_threshold)) {
+	if (unlikely(x > t || x < -t)) {
 		zone_page_state_add(x, zone, item);
 		x = 0;
 	}
-	*p = x;
+	__this_cpu_write(*p, x);
 }
 EXPORT_SYMBOL(__mod_zone_page_state);

@@ -221,16 +223,19 @@ EXPORT_SYMBOL(mod_zone_page_state);
  */
 void __inc_zone_state(struct zone *zone, enum zone_stat_item item)
 {
-	struct per_cpu_pageset *pcp = this_cpu_ptr(zone->pageset);
-	s8 *p = pcp->vm_stat_diff + item;
-
-	(*p)++;
+	struct per_cpu_pageset * __percpu pcp = zone->pageset;
+	s8 * __percpu p = pcp->vm_stat_diff + item;
+	int v, t;
+
+	__this_cpu_inc(*p);
+
+	v = __this_cpu_read(*p);
+	t = __this_cpu_read(pcp->stat_threshold);
+	if (unlikely(v > t)) {
+		int overstep = t / 2;

-	if (unlikely(*p > pcp->stat_threshold)) {
-		int overstep = pcp->stat_threshold / 2;
-
-		zone_page_state_add(*p + overstep, zone, item);
-		*p = -overstep;
+		zone_page_state_add(v + overstep, zone, item);
+		__this_cpu_write(*p, overstep);
 	}
 }

@@ -242,16 +247,19 @@ EXPORT_SYMBOL(__inc_zone_page_state);

 void __dec_zone_state(struct zone *zone, enum zone_stat_item item)
 {
-	struct per_cpu_pageset *pcp = this_cpu_ptr(zone->pageset);
-	s8 *p = pcp->vm_stat_diff + item;
-
-	(*p)--;
-
-	if (unlikely(*p < - pcp->stat_threshold)) {
-		int overstep = pcp->stat_threshold / 2;
+	struct per_cpu_pageset * __percpu pcp = zone->pageset;
+	s8 * __percpu p = pcp->vm_stat_diff + item;
+	int v, t;
+
+	__this_cpu_dec(*p);
+
+	v = __this_cpu_read(*p);
+	t = __this_cpu_read(pcp->stat_threshold);
+	if (unlikely(v < - t)) {
+		int overstep = t / 2;

-		zone_page_state_add(*p - overstep, zone, item);
-		*p = overstep;
+		zone_page_state_add(v - overstep, zone, item);
+		__this_cpu_write(*p, overstep);
 	}
 }

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

percpu: Implement this_cpu_add,sub,dec,inc_return

[Christoph Lameter]

Tried it. This is the result.


Implement this_cpu_add_return and friends and supply an optimized
implementation for x86.

Use this_cpu_add_return for vmstats and nmi processing.

There is no win in terms of code size (stays the same because xadd is a
longer instruction thaninc and requires loading a constant in a register first)
but we eliminate one memory access.

Plus we introduce a more flexible way of per cpu atomic operations.

Signed-off-by: Christoph Lameter <cl@linux.com>

---
 arch/x86/include/asm/percpu.h |   50 +++++++++++++++++++++++++++
 arch/x86/kernel/apic/nmi.c    |    3 -
 include/linux/percpu.h        |   77 ++++++++++++++++++++++++++++++++++++++++++
 mm/vmstat.c                   |    8 +---
 4 files changed, 130 insertions(+), 8 deletions(-)

Index: linux-2.6/arch/x86/kernel/apic/nmi.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/apic/nmi.c	2010-11-10 09:30:42.000000000 -0600
+++ linux-2.6/arch/x86/kernel/apic/nmi.c	2010-11-10 09:33:18.000000000 -0600
@@ -432,8 +432,7 @@ nmi_watchdog_tick(struct pt_regs *regs,
 		 * Ayiee, looks like this CPU is stuck ...
 		 * wait a few IRQs (5 seconds) before doing the oops ...
 		 */
-		__this_cpu_inc(alert_counter);
-		if (__this_cpu_read(alert_counter) == 5 * nmi_hz)
+		if (__this_cpu_inc_return(alert_counter) == 5 * nmi_hz)
 			/*
 			 * die_nmi will return ONLY if NOTIFY_STOP happens..
 			 */
Index: linux-2.6/include/linux/percpu.h
===================================================================
--- linux-2.6.orig/include/linux/percpu.h	2010-11-10 09:30:42.000000000 -0600
+++ linux-2.6/include/linux/percpu.h	2010-11-10 09:37:58.000000000 -0600
@@ -240,6 +240,20 @@ extern void __bad_size_call_parameter(vo
 	pscr_ret__;							\
 })

+#define __pcpu_size_call_return2(stem, variable, val)			\
+({	typeof(variable) pscr_ret__;					\
+	__verify_pcpu_ptr(&(variable));					\
+	switch(sizeof(variable)) {					\
+	case 1: pscr_ret__ = stem##1(variable, val);break;		\
+	case 2: pscr_ret__ = stem##2(variable, val);break;		\
+	case 4: pscr_ret__ = stem##4(variable, val);break;		\
+	case 8: pscr_ret__ = stem##8(variable, val);break;		\
+	default:							\
+		__bad_size_call_parameter();break;			\
+	}								\
+	pscr_ret__;							\
+})
+
 #define __pcpu_size_call(stem, variable, ...)				\
 do {									\
 	__verify_pcpu_ptr(&(variable));					\
@@ -529,6 +543,69 @@ do {									\
 # define __this_cpu_xor(pcp, val)	__pcpu_size_call(__this_cpu_xor_, (pcp), (val))
 #endif

+#define _this_cpu_generic_add_return(pcp, val)				\
+({	typeof(pcp) ret__;						\
+	preempt_disable();						\
+	__this_cpu_add((pcp), val);					\
+	ret__ = __this_cpu_read((pcp));					\
+	preempt_enable();						\
+	ret__;								\
+})
+
+#ifndef this_cpu_add_return
+# ifndef this_cpu_add_return_1
+#  define this_cpu_add_return_1(pcp, val)	_this_cpu_generic_add_return(pcp, val)
+# endif
+# ifndef this_cpu_add_return_2
+#  define this_cpu_add_return_2(pcp, val)	_this_cpu_generic_add_return(pcp, val)
+# endif
+# ifndef this_cpu_add_return_4
+#  define this_cpu_add_return_4(pcp, val)	_this_cpu_generic_add_return(pcp, val)
+# endif
+# ifndef this_cpu_add_return_8
+#  define this_cpu_add_return_8(pcp, val)	_this_cpu_generic_add_return(pcp, val)
+# endif
+# define this_cpu_add_return(pcp, val)	__pcpu_size_call_return2(this_cpu_add_return_, (pcp), val)
+#endif
+
+#define this_cpu_sub_return(pcp, val)	this_cpu_add_return(pcp, -(val))
+#define this_cpu_inc_return(pcp)	this_cpu_add_return(pcp, 1)
+#define this_cpu_dec_return(pcp)	this_cpu_add_return(pcp, -1)
+
+#define __this_cpu_generic_add_return(pcp, val)				\
+({	typeof(pcp) ret__;						\
+	__this_cpu_add((pcp), val);					\
+	ret__ = __this_cpu_read((pcp));					\
+	ret__;								\
+})
+
+#ifndef __this_cpu_add_return
+# ifndef __this_cpu_add_return_1
+#  define __this_cpu_add_return_1(pcp, val)	__this_cpu_generic_add_return(pcp, val)
+# endif
+# ifndef __this_cpu_add_return_2
+#  define __this_cpu_add_return_2(pcp, val)	__this_cpu_generic_add_return(pcp, val)
+# endif
+# ifndef __this_cpu_add_return_4
+#  define __this_cpu_add_return_4(pcp, val)	__this_cpu_generic_add_return(pcp, val)
+# endif
+# ifndef __this_cpu_add_return_8
+#  define __this_cpu_add_return_8(pcp, val)	__this_cpu_generic_add_return(pcp, val)
+# endif
+# define __this_cpu_add_return(pcp, val)	__pcpu_size_call_return2(this_cpu_add_return_, (pcp), val)
+#endif
+
+#define __this_cpu_sub_return(pcp, val)	this_cpu_add_return(pcp, -(val))
+#define __this_cpu_inc_return(pcp)	this_cpu_add_return(pcp, 1)
+#define __this_cpu_dec_return(pcp)	this_cpu_add_return(pcp, -1)
+
+#define _this_cpu_generic_to_op(pcp, val, op)				\
+do {									\
+	preempt_disable();						\
+	*__this_cpu_ptr(&(pcp)) op val;					\
+	preempt_enable();						\
+} while (0)
+
 /*
  * IRQ safe versions of the per cpu RMW operations. Note that these operations
  * are *not* safe against modification of the same variable from another
Index: linux-2.6/mm/vmstat.c
===================================================================
--- linux-2.6.orig/mm/vmstat.c	2010-11-10 09:30:42.000000000 -0600
+++ linux-2.6/mm/vmstat.c	2010-11-10 09:33:18.000000000 -0600
@@ -227,9 +227,7 @@ void __inc_zone_state(struct zone *zone,
 	s8 * __percpu p = pcp->vm_stat_diff + item;
 	int v, t;

-	__this_cpu_inc(*p);
-
-	v = __this_cpu_read(*p);
+	v = __this_cpu_inc_return(*p);
 	t = __this_cpu_read(pcp->stat_threshold);
 	if (unlikely(v > t)) {
 		int overstep = t / 2;
@@ -251,9 +249,7 @@ void __dec_zone_state(struct zone *zone,
 	s8 * __percpu p = pcp->vm_stat_diff + item;
 	int v, t;

-	__this_cpu_dec(*p);
-
-	v = __this_cpu_read(*p);
+	v = __this_cpu_dec_return(*p);
 	t = __this_cpu_read(pcp->stat_threshold);
 	if (unlikely(v < - t)) {
 		int overstep = t / 2;
Index: linux-2.6/arch/x86/include/asm/percpu.h
===================================================================
--- linux-2.6.orig/arch/x86/include/asm/percpu.h	2010-11-10 09:30:42.000000000 -0600
+++ linux-2.6/arch/x86/include/asm/percpu.h	2010-11-10 09:33:18.000000000 -0600
@@ -177,6 +177,45 @@ do {									\
 	}								\
 } while (0)

+
+/*
+ * Add return operation
+ */
+#define percpu_add_return_op(var, val)					\
+({									\
+	typedef typeof(var) pao_T__;					\
+	typeof(var) pfo_ret__ = val;					\
+	if (0) {							\
+		pao_T__ pao_tmp__;					\
+		pao_tmp__ = (val);					\
+		(void)pao_tmp__;					\
+	}								\
+	switch (sizeof(var)) {						\
+	case 1:								\
+		asm("xaddb %0, "__percpu_arg(1)				\
+			    : "+q" (pfo_ret__), "+m" (var)		\
+			    : : "memory");				\
+		break;							\
+	case 2:								\
+		asm("xaddw %0, "__percpu_arg(1)				\
+			    : "+r" (pfo_ret__), "+m" (var)		\
+			    : : "memory");				\
+		break;							\
+	case 4:								\
+		asm("xaddl %0, "__percpu_arg(1)				\
+			    : "+r"(pfo_ret__), "+m" (var)		\
+			    : : "memory");				\
+		break;							\
+	case 8:								\
+		asm("xaddq %0, "__percpu_arg(1)				\
+			    : "+re" (pfo_ret__),  "+m" (var)		\
+			    : : "memory");				\
+		break;							\
+	default: __bad_percpu_size();					\
+	}								\
+	pfo_ret__;							\
+})
+
 #define percpu_from_op(op, var, constraint)		\
 ({							\
 	typeof(var) pfo_ret__;				\
@@ -300,6 +339,14 @@ do {									\
 #define irqsafe_cpu_xor_2(pcp, val)	percpu_to_op("xor", (pcp), val)
 #define irqsafe_cpu_xor_4(pcp, val)	percpu_to_op("xor", (pcp), val)

+#ifndef CONFIG_M386
+#define __this_cpu_add_return_1(pcp, val)	percpu_add_return_op((pcp), val)
+#define __this_cpu_add_return_2(pcp, val)	percpu_add_return_op((pcp), val)
+#define __this_cpu_add_return_4(pcp, val)	percpu_add_return_op((pcp), val)
+#define this_cpu_add_return_1(pcp, val)		percpu_add_return_op((pcp), val)
+#define this_cpu_add_return_2(pcp, val)		percpu_add_return_op((pcp), val)
+#define this_cpu_add_return_4(pcp, val)		percpu_add_return_op((pcp), val)
+#endif
 /*
  * Per cpu atomic 64 bit operations are only available under 64 bit.
  * 32 bit must fall back to generic operations.
@@ -324,6 +371,9 @@ do {									\
 #define irqsafe_cpu_or_8(pcp, val)	percpu_to_op("or", (pcp), val)
 #define irqsafe_cpu_xor_8(pcp, val)	percpu_to_op("xor", (pcp), val)

+#define __this_cpu_add_return_8(pcp, val)	percpu_add_return_op((pcp), val)
+#define this_cpu_add_return_8(pcp, val)	percpu_add_return_op((pcp), val)
+
 #endif

 /* This is not atomic against other CPUs -- CPU preemption needs to be off */

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: percpu: Implement this_cpu_add,sub,dec,inc_return

[Eric Dumazet]

Le mercredi 10 novembre 2010 A  09:40 -0600, Christoph Lameter a A(C)crit :
> Tried it. This is the result.
> 
> 
> Implement this_cpu_add_return and friends and supply an optimized
> implementation for x86.
> 
> Use this_cpu_add_return for vmstats and nmi processing.
> 
> There is no win in terms of code size (stays the same because xadd is a
> longer instruction thaninc and requires loading a constant in a register first)
> but we eliminate one memory access.
> 
> Plus we introduce a more flexible way of per cpu atomic operations.
> 
> Signed-off-by: Christoph Lameter <cl@linux.com>
> 
> ---

I believe this new xx_return stuff would be useful on x86_32 :
#if defined(CONFIG_HIGHMEM) || defined(CONFIG_X86_32)

kmap_atomic_idx_push() / kmap_atomic_idx_pop() are a bit expensive
because :

c102a652:       0f 01 3b                invlpg (%ebx)
	int idx = --__get_cpu_var(__kmap_atomic_idx);
c102a655:       64 03 3d 90 40 5f c1    add    %fs:0xc15f4090,%edi
c102a65c:       8b 07                   mov    (%edi),%eax
c102a65e:       83 e8 01                sub    $0x1,%eax
c102a661:       85 c0                   test   %eax,%eax
c102a663:       89 07                   mov    %eax,(%edi)





diff --git a/include/linux/highmem.h b/include/linux/highmem.h
index b676c58..bb5db26 100644
--- a/include/linux/highmem.h
+++ b/include/linux/highmem.h
@@ -91,7 +91,7 @@ static inline int kmap_atomic_idx_push(void)
 
 static inline int kmap_atomic_idx(void)
 {
-	return __get_cpu_var(__kmap_atomic_idx) - 1;
+	return __this_cpu_read(__kmap_atomic_idx) - 1;
 }
 
 static inline int kmap_atomic_idx_pop(void)


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: percpu: Implement this_cpu_add,sub,dec,inc_return

[Christoph Lameter]

On Wed, 17 Nov 2010, Eric Dumazet wrote:

> diff --git a/include/linux/highmem.h b/include/linux/highmem.h
> index b676c58..bb5db26 100644
> --- a/include/linux/highmem.h
> +++ b/include/linux/highmem.h
> @@ -91,7 +91,7 @@ static inline int kmap_atomic_idx_push(void)
>
>  static inline int kmap_atomic_idx(void)
>  {
> -	return __get_cpu_var(__kmap_atomic_idx) - 1;
> +	return __this_cpu_read(__kmap_atomic_idx) - 1;
>  }
>
>  static inline int kmap_atomic_idx_pop(void)

This isnt a use case for this_cpu_dec right? Seems that your message was
cut off?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: percpu: Implement this_cpu_add,sub,dec,inc_return

[Eric Dumazet]

Le vendredi 19 novembre 2010 A  09:42 -0600, Christoph Lameter a A(C)crit :
> On Wed, 17 Nov 2010, Eric Dumazet wrote:
> 
> > diff --git a/include/linux/highmem.h b/include/linux/highmem.h
> > index b676c58..bb5db26 100644
> > --- a/include/linux/highmem.h
> > +++ b/include/linux/highmem.h
> > @@ -91,7 +91,7 @@ static inline int kmap_atomic_idx_push(void)
> >
> >  static inline int kmap_atomic_idx(void)
> >  {
> > -	return __get_cpu_var(__kmap_atomic_idx) - 1;
> > +	return __this_cpu_read(__kmap_atomic_idx) - 1;
> >  }
> >
> >  static inline int kmap_atomic_idx_pop(void)
> 
> This isnt a use case for this_cpu_dec right? Seems that your message was
> cut off?
> 


I wanted to show you the file were it was possible to use this_cpu_{dec|
inc}_return()


My patch on kmap_atomic_idx() doesnt need your new functions ;)

Thanks


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: percpu: Implement this_cpu_add,sub,dec,inc_return

[Christoph Lameter]

On Fri, 19 Nov 2010, Eric Dumazet wrote:

> > This isnt a use case for this_cpu_dec right? Seems that your message was
> > cut off?
> I wanted to show you the file were it was possible to use this_cpu_{dec|
> inc}_return()
>
> My patch on kmap_atomic_idx() doesnt need your new functions ;)

Oh ok you mean this:

---
 include/linux/highmem.h |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Index: linux-2.6/include/linux/highmem.h
===================================================================
--- linux-2.6.orig/include/linux/highmem.h	2010-11-19 09:55:24.000000000 -0600
+++ linux-2.6/include/linux/highmem.h	2010-11-19 09:57:54.000000000 -0600
@@ -81,7 +81,9 @@ DECLARE_PER_CPU(int, __kmap_atomic_idx);

 static inline int kmap_atomic_idx_push(void)
 {
-	int idx = __get_cpu_var(__kmap_atomic_idx)++;
+	int idx = __this_cpu_read(__kmap_atomic_idx);
+
+	__this_cpu_inc(__kmap_atomic_idx);
 #ifdef CONFIG_DEBUG_HIGHMEM
 	WARN_ON_ONCE(in_irq() && !irqs_disabled());
 	BUG_ON(idx > KM_TYPE_NR);
@@ -91,12 +93,12 @@ static inline int kmap_atomic_idx_push(v

 static inline int kmap_atomic_idx(void)
 {
-	return __get_cpu_var(__kmap_atomic_idx) - 1;
+	return __this_cpu_read(__kmap_atomic_idx) - 1;
 }

 static inline int kmap_atomic_idx_pop(void)
 {
-	int idx = --__get_cpu_var(__kmap_atomic_idx);
+	int idx = __this_cpu_dec_return(__kmap_atomic_idx);
 #ifdef CONFIG_DEBUG_HIGHMEM
 	BUG_ON(idx < 0);
 #endif

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: percpu: Implement this_cpu_add,sub,dec,inc_return

[Eric Dumazet]

Le vendredi 19 novembre 2010 A  09:59 -0600, Christoph Lameter a A(C)crit :
> On Fri, 19 Nov 2010, Eric Dumazet wrote:
> 
> > > This isnt a use case for this_cpu_dec right? Seems that your message was
> > > cut off?
> > I wanted to show you the file were it was possible to use this_cpu_{dec|
> > inc}_return()
> >
> > My patch on kmap_atomic_idx() doesnt need your new functions ;)
> 
> Oh ok you mean this:
> 
> ---
>  include/linux/highmem.h |    8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> Index: linux-2.6/include/linux/highmem.h
> ===================================================================
> --- linux-2.6.orig/include/linux/highmem.h	2010-11-19 09:55:24.000000000 -0600
> +++ linux-2.6/include/linux/highmem.h	2010-11-19 09:57:54.000000000 -0600
> @@ -81,7 +81,9 @@ DECLARE_PER_CPU(int, __kmap_atomic_idx);
> 
>  static inline int kmap_atomic_idx_push(void)
>  {
> -	int idx = __get_cpu_var(__kmap_atomic_idx)++;
> +	int idx = __this_cpu_read(__kmap_atomic_idx);
> +
> +	__this_cpu_inc(__kmap_atomic_idx);
>  #ifdef CONFIG_DEBUG_HIGHMEM
>  	WARN_ON_ONCE(in_irq() && !irqs_disabled());
>  	BUG_ON(idx > KM_TYPE_NR);
> @@ -91,12 +93,12 @@ static inline int kmap_atomic_idx_push(v
> 
>  static inline int kmap_atomic_idx(void)
>  {
> -	return __get_cpu_var(__kmap_atomic_idx) - 1;
> +	return __this_cpu_read(__kmap_atomic_idx) - 1;
>  }
> 
>  static inline int kmap_atomic_idx_pop(void)
>  {
> -	int idx = --__get_cpu_var(__kmap_atomic_idx);
> +	int idx = __this_cpu_dec_return(__kmap_atomic_idx);
>  #ifdef CONFIG_DEBUG_HIGHMEM
>  	BUG_ON(idx < 0);
>  #endif


Yes, absolutely.

By the way, is your patch really ok ?

xadd %0,foo   returns in %0 the previous value of the memory, not the
value _after_ the operation.

This is why we do in arch/x86/include/asm/atomic.h :

static inline int atomic_add_return(int i, atomic_t *v)
...
       
        __i = i;
        asm volatile(LOCK_PREFIX "xaddl %0, %1"
                     : "+r" (i), "+m" (v->counter)
                     : : "memory");
        return i + __i;
...





--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: percpu: Implement this_cpu_add,sub,dec,inc_return

[Christoph Lameter]

On Fri, 19 Nov 2010, Eric Dumazet wrote:

> By the way, is your patch really ok ?
>
> xadd %0,foo   returns in %0 the previous value of the memory, not the
> value _after_ the operation.
>
> This is why we do in arch/x86/include/asm/atomic.h :
>
> static inline int atomic_add_return(int i, atomic_t *v)
> ...
>
>         __i = i;
>         asm volatile(LOCK_PREFIX "xaddl %0, %1"
>                      : "+r" (i), "+m" (v->counter)
>                      : : "memory");
>         return i + __i;
> ...

Ok so rename the macros to this_cpu_return_inc/dec/add/sub?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: percpu: Implement this_cpu_add,sub,dec,inc_return

[Christoph Lameter]

On Fri, 19 Nov 2010, Christoph Lameter wrote:

> Ok so rename the macros to this_cpu_return_inc/dec/add/sub?

Actually this is fetchadd. So call I will call this this_cpu_fetch_add/inc/dec/sub.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: percpu: Implement this_cpu_add,sub,dec,inc_return

[Eric Dumazet]

Le vendredi 19 novembre 2010 A  11:59 -0600, Christoph Lameter a A(C)crit :
> On Fri, 19 Nov 2010, Christoph Lameter wrote:
> 
> > Ok so rename the macros to this_cpu_return_inc/dec/add/sub?
> 
> Actually this is fetchadd. So call I will call this this_cpu_fetch_add/inc/dec/sub.
> 

It doesnt really matter, because the final "res += added_value" can be
optimized out by compiler if needed, since its C code.

For example in  net/core/neighbour.c, function neigh_alloc() we do

entries = atomic_inc_return(&tbl->entries) - 1;
if (entries >= tbl->gc_thresh3 ||

This generates this optimal code :

mov        $0x1,%eax
lock xadd  %eax,0x1d0(%rdi)
cmp        0xdc(%rdi),%eax

Yes, if we had atomic_fetch_inc() this could be written :

entries = atomic_fetch_inc(&tbl->entries);
if (entries >= tbl->gc_thresh3 ||

Not sure its clearer in this form.
Is it worth adding another seldom used API ?

Everybody understand the xxxx_inc_return() idiom


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>