2008-01-08 21:13:51

by Mike Travis

[permalink] [raw]
Subject: [PATCH 10/10] x86: Unify percpu.h

Form a single percpu.h from percpu_32.h and percpu_64.h. Both are now pretty
small so this is simply adding them together.

Cc: Rusty Russell <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Christoph Lameter <[email protected]>
Signed-off-by: Mike Travis <[email protected]>

---
include/asm-x86/percpu.h | 145 ++++++++++++++++++++++++++++++++++++++++++--
include/asm-x86/percpu_32.h | 119 ------------------------------------
include/asm-x86/percpu_64.h | 23 ------
3 files changed, 141 insertions(+), 146 deletions(-)

--- a/include/asm-x86/percpu.h
+++ b/include/asm-x86/percpu.h
@@ -1,5 +1,142 @@
-#ifdef CONFIG_X86_32
-# include "percpu_32.h"
-#else
-# include "percpu_64.h"
+#ifndef _ASM_X86_PERCPU_H_
+#define _ASM_X86_PERCPU_H_
+
+#ifdef CONFIG_X86_64
+#include <linux/compiler.h>
+
+/* Same as asm-generic/percpu.h, except that we store the per cpu offset
+ in the PDA. Longer term the PDA and every per cpu variable
+ should be just put into a single section and referenced directly
+ from %gs */
+
+#ifdef CONFIG_SMP
+#include <asm/pda.h>
+
+#define __per_cpu_offset(cpu) (cpu_pda(cpu)->data_offset)
+#define __my_cpu_offset read_pda(data_offset)
+
+#define per_cpu_offset(x) (__per_cpu_offset(x))
+
#endif
+#include <asm-generic/percpu.h>
+
+DECLARE_PER_CPU(struct x8664_pda, pda);
+
+#else /* CONFIG_X86_64 */
+
+#ifdef __ASSEMBLY__
+
+/*
+ * PER_CPU finds an address of a per-cpu variable.
+ *
+ * Args:
+ * var - variable name
+ * reg - 32bit register
+ *
+ * The resulting address is stored in the "reg" argument.
+ *
+ * Example:
+ * PER_CPU(cpu_gdt_descr, %ebx)
+ */
+#ifdef CONFIG_SMP
+#define PER_CPU(var, reg) \
+ movl %fs:per_cpu__##this_cpu_off, reg; \
+ lea per_cpu__##var(reg), reg
+#define PER_CPU_VAR(var) %fs:per_cpu__##var
+#else /* ! SMP */
+#define PER_CPU(var, reg) \
+ movl $per_cpu__##var, reg
+#define PER_CPU_VAR(var) per_cpu__##var
+#endif /* SMP */
+
+#else /* ...!ASSEMBLY */
+
+/*
+ * PER_CPU finds an address of a per-cpu variable.
+ *
+ * Args:
+ * var - variable name
+ * cpu - 32bit register containing the current CPU number
+ *
+ * The resulting address is stored in the "cpu" argument.
+ *
+ * Example:
+ * PER_CPU(cpu_gdt_descr, %ebx)
+ */
+#ifdef CONFIG_SMP
+
+#define __my_cpu_offset x86_read_percpu(this_cpu_off)
+
+/* fs segment starts at (positive) offset == __per_cpu_offset[cpu] */
+#define __percpu_seg "%%fs:"
+
+#else /* !SMP */
+
+#define __percpu_seg ""
+
+#endif /* SMP */
+
+#include <asm-generic/percpu.h>
+
+/* We can use this directly for local CPU (faster). */
+DECLARE_PER_CPU(unsigned long, this_cpu_off);
+
+/* For arch-specific code, we can use direct single-insn ops (they
+ * don't give an lvalue though). */
+extern void __bad_percpu_size(void);
+
+#define percpu_to_op(op,var,val) \
+ do { \
+ typedef typeof(var) T__; \
+ if (0) { T__ tmp__; tmp__ = (val); } \
+ switch (sizeof(var)) { \
+ case 1: \
+ asm(op "b %1,"__percpu_seg"%0" \
+ : "+m" (var) \
+ :"ri" ((T__)val)); \
+ break; \
+ case 2: \
+ asm(op "w %1,"__percpu_seg"%0" \
+ : "+m" (var) \
+ :"ri" ((T__)val)); \
+ break; \
+ case 4: \
+ asm(op "l %1,"__percpu_seg"%0" \
+ : "+m" (var) \
+ :"ri" ((T__)val)); \
+ break; \
+ default: __bad_percpu_size(); \
+ } \
+ } while (0)
+
+#define percpu_from_op(op,var) \
+ ({ \
+ typeof(var) ret__; \
+ switch (sizeof(var)) { \
+ case 1: \
+ asm(op "b "__percpu_seg"%1,%0" \
+ : "=r" (ret__) \
+ : "m" (var)); \
+ break; \
+ case 2: \
+ asm(op "w "__percpu_seg"%1,%0" \
+ : "=r" (ret__) \
+ : "m" (var)); \
+ break; \
+ case 4: \
+ asm(op "l "__percpu_seg"%1,%0" \
+ : "=r" (ret__) \
+ : "m" (var)); \
+ break; \
+ default: __bad_percpu_size(); \
+ } \
+ ret__; })
+
+#define x86_read_percpu(var) percpu_from_op("mov", per_cpu__##var)
+#define x86_write_percpu(var,val) percpu_to_op("mov", per_cpu__##var, val)
+#define x86_add_percpu(var,val) percpu_to_op("add", per_cpu__##var, val)
+#define x86_sub_percpu(var,val) percpu_to_op("sub", per_cpu__##var, val)
+#define x86_or_percpu(var,val) percpu_to_op("or", per_cpu__##var, val)
+#endif /* !__ASSEMBLY__ */
+#endif /* !CONFIG_X86_64 */
+#endif /* _ASM_X86_PERCPU_H_ */
--- a/include/asm-x86/percpu_32.h
+++ /dev/null
@@ -1,119 +0,0 @@
-#ifndef __ARCH_I386_PERCPU__
-#define __ARCH_I386_PERCPU__
-
-#ifdef __ASSEMBLY__
-
-/*
- * PER_CPU finds an address of a per-cpu variable.
- *
- * Args:
- * var - variable name
- * reg - 32bit register
- *
- * The resulting address is stored in the "reg" argument.
- *
- * Example:
- * PER_CPU(cpu_gdt_descr, %ebx)
- */
-#ifdef CONFIG_SMP
-#define PER_CPU(var, reg) \
- movl %fs:per_cpu__##this_cpu_off, reg; \
- lea per_cpu__##var(reg), reg
-#define PER_CPU_VAR(var) %fs:per_cpu__##var
-#else /* ! SMP */
-#define PER_CPU(var, reg) \
- movl $per_cpu__##var, reg
-#define PER_CPU_VAR(var) per_cpu__##var
-#endif /* SMP */
-
-#else /* ...!ASSEMBLY */
-
-/*
- * PER_CPU finds an address of a per-cpu variable.
- *
- * Args:
- * var - variable name
- * cpu - 32bit register containing the current CPU number
- *
- * The resulting address is stored in the "cpu" argument.
- *
- * Example:
- * PER_CPU(cpu_gdt_descr, %ebx)
- */
-#ifdef CONFIG_SMP
-
-#define __my_cpu_offset x86_read_percpu(this_cpu_off)
-
-/* fs segment starts at (positive) offset == __per_cpu_offset[cpu] */
-#define __percpu_seg "%%fs:"
-
-#else /* !SMP */
-
-#define __percpu_seg ""
-
-#endif /* SMP */
-
-#include <asm-generic/percpu.h>
-
-/* We can use this directly for local CPU (faster). */
-DECLARE_PER_CPU(unsigned long, this_cpu_off);
-
-/* For arch-specific code, we can use direct single-insn ops (they
- * don't give an lvalue though). */
-extern void __bad_percpu_size(void);
-
-#define percpu_to_op(op,var,val) \
- do { \
- typedef typeof(var) T__; \
- if (0) { T__ tmp__; tmp__ = (val); } \
- switch (sizeof(var)) { \
- case 1: \
- asm(op "b %1,"__percpu_seg"%0" \
- : "+m" (var) \
- :"ri" ((T__)val)); \
- break; \
- case 2: \
- asm(op "w %1,"__percpu_seg"%0" \
- : "+m" (var) \
- :"ri" ((T__)val)); \
- break; \
- case 4: \
- asm(op "l %1,"__percpu_seg"%0" \
- : "+m" (var) \
- :"ri" ((T__)val)); \
- break; \
- default: __bad_percpu_size(); \
- } \
- } while (0)
-
-#define percpu_from_op(op,var) \
- ({ \
- typeof(var) ret__; \
- switch (sizeof(var)) { \
- case 1: \
- asm(op "b "__percpu_seg"%1,%0" \
- : "=r" (ret__) \
- : "m" (var)); \
- break; \
- case 2: \
- asm(op "w "__percpu_seg"%1,%0" \
- : "=r" (ret__) \
- : "m" (var)); \
- break; \
- case 4: \
- asm(op "l "__percpu_seg"%1,%0" \
- : "=r" (ret__) \
- : "m" (var)); \
- break; \
- default: __bad_percpu_size(); \
- } \
- ret__; })
-
-#define x86_read_percpu(var) percpu_from_op("mov", per_cpu__##var)
-#define x86_write_percpu(var,val) percpu_to_op("mov", per_cpu__##var, val)
-#define x86_add_percpu(var,val) percpu_to_op("add", per_cpu__##var, val)
-#define x86_sub_percpu(var,val) percpu_to_op("sub", per_cpu__##var, val)
-#define x86_or_percpu(var,val) percpu_to_op("or", per_cpu__##var, val)
-#endif /* !__ASSEMBLY__ */
-
-#endif /* __ARCH_I386_PERCPU__ */
--- a/include/asm-x86/percpu_64.h
+++ /dev/null
@@ -1,23 +0,0 @@
-#ifndef _ASM_X8664_PERCPU_H_
-#define _ASM_X8664_PERCPU_H_
-#include <linux/compiler.h>
-
-/* Same as asm-generic/percpu.h, except that we store the per cpu offset
- in the PDA. Longer term the PDA and every per cpu variable
- should be just put into a single section and referenced directly
- from %gs */
-
-#ifdef CONFIG_SMP
-
-#include <asm/pda.h>
-
-#define __per_cpu_offset(cpu) (cpu_pda(cpu)->data_offset)
-#define __my_cpu_offset read_pda(data_offset)
-
-#define per_cpu_offset(x) (__per_cpu_offset(x))
-
-#endif /* SMP */
-
-#include <asm-generic/percpu.h>
-
-#endif /* _ASM_X8664_PERCPU_H_ */

--


2008-01-09 19:29:25

by Dave Hansen

[permalink] [raw]
Subject: Re: [PATCH 10/10] x86: Unify percpu.h

On Tue, 2008-01-08 at 13:10 -0800, [email protected] wrote:
> Form a single percpu.h from percpu_32.h and percpu_64.h. Both are now pretty
> small so this is simply adding them together.

I guess I just don't really see the point of moving the code around like
this. Before, it would have been easier to tell at a glance before
whether you were looking at 32 or 64-bit code because of which file you
are in. But, now, you need to look for #ifdef context. I'm not sure
that's a win.

This only saves 5 net lines of code, and those are probably from:

-#ifndef __ARCH_I386_PERCPU__
-#define __ARCH_I386_PERCPU__

right?

The rest of the set looks brilliant, though.

-- Dave

2008-01-09 19:31:52

by Christoph Lameter

[permalink] [raw]
Subject: Re: [PATCH 10/10] x86: Unify percpu.h

On Wed, 9 Jan 2008, Dave Hansen wrote:

> On Tue, 2008-01-08 at 13:10 -0800, [email protected] wrote:
> > Form a single percpu.h from percpu_32.h and percpu_64.h. Both are now pretty
> > small so this is simply adding them together.
>
> I guess I just don't really see the point of moving the code around like
> this. Before, it would have been easier to tell at a glance before
> whether you were looking at 32 or 64-bit code because of which file you
> are in. But, now, you need to look for #ifdef context. I'm not sure
> that's a win.
>
> This only saves 5 net lines of code, and those are probably from:
>
> -#ifndef __ARCH_I386_PERCPU__
> -#define __ARCH_I386_PERCPU__
>
> right?
>
> The rest of the set looks brilliant, though.

Well this is only the first patchset. The next one will unify this even
more (and make percpu functions work consistent between the two arches)
but it requires changes to the way the %gs register is used in
x86_64. So we only do the simplest thing here to have one file to patch
against later.

2008-01-09 19:54:51

by Dave Hansen

[permalink] [raw]
Subject: Re: [PATCH 10/10] x86: Unify percpu.h

On Wed, 2008-01-09 at 11:31 -0800, Christoph Lameter wrote:
> Well this is only the first patchset. The next one will unify this even
> more (and make percpu functions work consistent between the two arches)
> but it requires changes to the way the %gs register is used in
> x86_64. So we only do the simplest thing here to have one file to patch
> against later.

Then I really think this particular patch belongs in that other patch
set. Here, it makes very little sense, and it's on the end anyway.

-- Dave

2008-01-09 20:11:25

by Christoph Lameter

[permalink] [raw]
Subject: Re: [PATCH 10/10] x86: Unify percpu.h

On Wed, 9 Jan 2008, Dave Hansen wrote:

> Then I really think this particular patch belongs in that other patch
> set. Here, it makes very little sense, and it's on the end anyway.

It makes sense in that both percpu_32/64 are very small as a result of
earlier patches and so its justifiable to put them together to simplify
the next patchset.

2008-01-10 13:49:31

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 10/10] x86: Unify percpu.h


* Christoph Lameter <[email protected]> wrote:

> On Wed, 9 Jan 2008, Dave Hansen wrote:
>
> > Then I really think this particular patch belongs in that other
> > patch set. Here, it makes very little sense, and it's on the end
> > anyway.
>
> It makes sense in that both percpu_32/64 are very small as a result of
> earlier patches and so its justifiable to put them together to
> simplify the next patchset.

i'd agree with this - lets just keep the existing flow of patches
intact. It's not like the percpu code is in any danger of becoming
unclean or quirky - it's one of the best-maintained pieces of kernel
code :)

Ingo