Re: [PATCH] slub: Disable the lockless allocator

From: Linus Torvalds
Date: Sat Mar 26 2011 - 18:19:36 EST


On Sat, Mar 26, 2011 at 12:49 PM, Christoph Lameter <cl@xxxxxxxxx> wrote:
> On Sat, 26 Mar 2011, Christoph Lameter wrote:
>
>> Tejun: Whats going on there? I should be getting offsets into the per cpu
>> area and not kernel addresses.
>
> Its a UP kernel running on dual Athlon. So its okay ... Argh.... The
> following patch fixes it by using the fallback code for cmpxchg_double:

Hmm.

Looking closer, I think there are more bugs in that cmpxchg_double thing.

In particular, it doesn't mark memory as changed, so gcc might
miscompile it even on SMP. Also, I think we'd be better off using the
'cmpxchg16b' instruction even on UP, so it's sad to disable it
entirely there.

Wouldn't something like the attached be better?

NOTE! TOTALLY UNTESTED!

Linus
arch/x86/include/asm/percpu.h | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/percpu.h b/arch/x86/include/asm/percpu.h
index a09e1f0..d475b43 100644
--- a/arch/x86/include/asm/percpu.h
+++ b/arch/x86/include/asm/percpu.h
@@ -45,7 +45,7 @@
#include <linux/stringify.h>

#ifdef CONFIG_SMP
-#define __percpu_arg(x) "%%"__stringify(__percpu_seg)":%P" #x
+#define __percpu_prefix "%%"__stringify(__percpu_seg)":"
#define __my_cpu_offset percpu_read(this_cpu_off)

/*
@@ -62,9 +62,11 @@
(typeof(*(ptr)) __kernel __force *)tcp_ptr__; \
})
#else
-#define __percpu_arg(x) "%P" #x
+#define __percpu_prefix ""
#endif

+#define __percpu_arg(x) __percpu_prefix "%P" #x
+
/*
* Initialized pointers to per-cpu variables needed for the boot
* processor need to use these macros to get the proper address
@@ -516,11 +518,11 @@ do { \
typeof(o2) __n2 = n2; \
typeof(o2) __dummy; \
alternative_io("call this_cpu_cmpxchg16b_emu\n\t" P6_NOP4, \
- "cmpxchg16b %%gs:(%%rsi)\n\tsetz %0\n\t", \
+ "cmpxchg16b " __percpu_prefix "(%%rsi)\n\tsetz %0\n\t", \
X86_FEATURE_CX16, \
ASM_OUTPUT2("=a"(__ret), "=d"(__dummy)), \
"S" (&pcp1), "b"(__n1), "c"(__n2), \
- "a"(__o1), "d"(__o2)); \
+ "a"(__o1), "d"(__o2) : "memory"); \
__ret; \
})