Re: [PATCH] Code clean up for percpu_xxx() functions

[Christoph Lameter]

On Mon, 10 Oct 2011, Alex,Shi wrote:

> index 3470c9d..d1de56c 100644
> --- a/arch/x86/include/asm/percpu.h
> +++ b/arch/x86/include/asm/percpu.h
> @@ -46,7 +46,7 @@
>
>  #ifdef CONFIG_SMP
>  #define __percpu_prefix		"%%"__stringify(__percpu_seg)":"
> -#define __my_cpu_offset		percpu_read(this_cpu_off)
> +#define __my_cpu_offset		this_cpu_read(this_cpu_off)

Should this not be __this_cpu_read() instead?

>   * per-thread variables implemented as per-cpu variables and thus
>   * stable for the duration of the respective task.
>   */
> -#define percpu_read(var)		percpu_from_op("mov", var, "m" (var))
> -#define percpu_read_stable(var)		percpu_from_op("mov", var, "p" (&(var)))
> -#define percpu_write(var, val)		percpu_to_op("mov", var, val)
> -#define percpu_add(var, val)		percpu_add_op(var, val)
> -#define percpu_sub(var, val)		percpu_add_op(var, -(val))
> -#define percpu_and(var, val)		percpu_to_op("and", var, val)
> -#define percpu_or(var, val)		percpu_to_op("or", var, val)
> -#define percpu_xor(var, val)		percpu_to_op("xor", var, val)
> -#define percpu_inc(var)		percpu_unary_op("inc", var)
> +#define this_cpu_read(var)		percpu_from_op("mov", var, "m" (var))
> +#define this_cpu_read_stable(var)	percpu_from_op("mov", var, "p" (&(var)))
> +#define this_cpu_write(var, val)	percpu_to_op("mov", var, val)
> +#define this_cpu_add(var, val)		percpu_add_op(var, val)
> +#define this_cpu_sub(var, val)		percpu_add_op(var, -(val))
> +#define this_cpu_and(var, val)		percpu_to_op("and", var, val)
> +#define this_cpu_or(var, val)		percpu_to_op("or", var, val)
> +#define this_cpu_xor(var, val)		percpu_to_op("xor", var, val)
> +#define this_cpu_inc(var)		percpu_unary_op("inc", var)

Why define these operations? They are already defined in the form of
this_cpu_add_1/2/4/8 and include/linux/percpu.h branches out to them.

> @@ -522,7 +522,7 @@ void sock_release(struct socket *sock)
>  	if (rcu_dereference_protected(sock->wq, 1)->fasync_list)
>  		printk(KERN_ERR "sock_release: fasync list not empty!\n");
>
> -	percpu_sub(sockets_in_use, 1);
> +	this_cpu_sub(sockets_in_use, 1);
>  	if (!sock->file) {
>  		iput(SOCK_INODE(sock));
>  		return;

In general __this_cpu_xx operations can be used instead of this_cpu_xx
operations if the context is already preempt safe. There is no change of
the code generated for x86 in those cases. But the fallback code on other
platforms can generate more efficient code.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/