lkml.org 
[lkml]   [2010]   [Nov]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [thisops uV2 03/10] percpu: Generic support for this_cpu_add,sub,dec,inc_return
* Christoph Lameter (cl@linux.com) wrote:
> Introduce generic support for this_cpu_add_return etc.
>
> The fallback is to realize these operations with __this_cpu_ops.
>
> Reviewed-by: Tejun Heo <tj@kernel.org>
> Signed-off-by: Christoph Lameter <cl@linux.com>
>
> ---
> include/linux/percpu.h | 70 +++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 70 insertions(+)
>
> Index: linux-2.6/include/linux/percpu.h
> ===================================================================
> --- linux-2.6.orig/include/linux/percpu.h 2010-11-23 17:29:46.000000000 -0600
> +++ linux-2.6/include/linux/percpu.h 2010-11-23 17:31:14.000000000 -0600
> @@ -240,6 +240,20 @@ extern void __bad_size_call_parameter(vo
> pscr_ret__; \
> })
>
> +#define __pcpu_size_call_return2(stem, variable, ...) \
> +({ typeof(variable) pscr_ret__; \

isn't it usual to do ?

( { \
typeof(variable) pscr_ret__; \

instead ?


> + __verify_pcpu_ptr(&(variable)); \
> + switch(sizeof(variable)) { \
> + case 1: pscr_ret__ = stem##1(variable, __VA_ARGS__);break; \

;break; at the EOL seems a bit odd. Maybe moving it to the next line ?

> + case 2: pscr_ret__ = stem##2(variable, __VA_ARGS__);break; \
> + case 4: pscr_ret__ = stem##4(variable, __VA_ARGS__);break; \
> + case 8: pscr_ret__ = stem##8(variable, __VA_ARGS__);break; \
> + default: \
> + __bad_size_call_parameter();break; \
> + } \
> + pscr_ret__; \
> +})
> +
> #define __pcpu_size_call(stem, variable, ...) \
> do { \
> __verify_pcpu_ptr(&(variable)); \
> @@ -529,6 +543,62 @@ 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__; \

add newline after opening ({ \ ?

> + preempt_disable(); \
> + __this_cpu_add((pcp), val); \

Hrm, you are inconsistent between your macros. Here you use "(pcp), " but above:
"(variable, ". I think the extra () are not needed in this case, so you might
want to consider removing these from (pcp).

> + ret__ = __this_cpu_read((pcp)); \

Same here.

> + 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)
Same here.

> +#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)); \

Same for above 2 lines.

> + 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)

Same here.

Other than that:

Reviewed-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

Thanks,

Mathieu

> +#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)
> +
> /*
> * IRQ safe versions of the per cpu RMW operations. Note that these operations
> * are *not* safe against modification of the same variable from another
>

--
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com


\
 
 \ /
  Last update: 2010-11-27 16:01    [W:0.220 / U:0.460 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site