arm64: percpu: Implement this_cpu operations
Subsystems (2)
PATCH 1/1 arm64: percpu: Implement this_cpu operations
The generic this_cpu operations disable interrupts to ensure that the requested operation is protected from pre-emption. For arm64, this is overkill and can hurt throughput and latency.
This patch provides arm64 specific implementations for the this_cpu operations. Rather than disable interrupts, we use the exclusive monitor or atomic operations as appropriate.
The following operations are implemented: add, add_return, and, or, read, write, xchg. We also wire up a cmpxchg implementation from cmpxchg.h.
Testing was performed using the percpu_test module and hackbench on a Juno board running 3.18-rc3.
---
This patch applies on top of one I sent out earlier:
"arm64: xchg: Implement cmpxchg_double"
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-October/294705.html
My two least favourite things in the kernel are excessive pre-processor use and assembler, so this patch has been fun to write ;-).
I've tried to make the inline assembler clearer with named parameters, it really upset checkpatch, if it upsets people then please shout and I will change it.
Cheers,
Signature · 6 lines
Steve --- arch/arm64/include/asm/cmpxchg.h | 6 +- arch/arm64/include/asm/percpu.h | 231 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 235 insertions(+), 2 deletions(-)
arch/arm64/include/asm/cmpxchg.h +4 -2
@@ -237,8 +237,10 @@ static inline unsigned long __cmpxchg_mb(volatile void *ptr, unsigned long old, __ret; \ }) -#define this_cpu_cmpxchg_8 (ptr, o, n) \ - cmpxchg(raw_cpu_ptr(&(ptr)), o, n); +#define this_cpu_cmpxchg_1 (ptr, o, n) cmpxchg(raw_cpu_ptr(&(ptr)), o, n) +#define this_cpu_cmpxchg_2 (ptr, o, n) cmpxchg(raw_cpu_ptr(&(ptr)), o, n) +#define this_cpu_cmpxchg_4 (ptr, o, n) cmpxchg(raw_cpu_ptr(&(ptr)), o, n) +#define this_cpu_cmpxchg_8 (ptr, o, n) cmpxchg(raw_cpu_ptr(&(ptr)), o, n)#define this_cpu_cmpxchg_double_8 (ptr1, ptr2, o1, o2, n1, n2) \ cmpxchg_double(raw_cpu_ptr(&(ptr1)), raw_cpu_ptr(&(ptr2)), \
arch/arm64/include/asm/percpu.h +231 -0
237 lines not on this page · raw message
Signature · 1 line
1.9.3
PATCH 1/1 arm64: percpu: Implement this_cpu operations
Hi Steve,
Quoted 8 lines · Steve Capper
The generic
this_cpuoperations disable interrupts to ensure that the requested operation is protected from pre-emption. For arm64, this is overkill and can hurt throughput and latency.This patch provides arm64 specific implementations for the
this_cpuoperations. Rather than disable interrupts, we use the exclusive monitor or atomic operations as appropriate.
Quoted 7 lines
The following operations are implemented: add,
add_return, and, or, read, write, xchg. We also wire up a cmpxchg implementation fromcmpxchg.h.Testing was performed using the
percpu_testmodule and hackbench on a Juno board running 3.18-rc3.
Got any numbers?
Quoted 133 lines
Signed-off-by Steve Capper <[email protected]>---
This patch applies on top of one I sent out earlier:
"arm64: xchg: Implementcmpxchg_double"
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-October/294705.htmlMy two least favourite things in the kernel are excessive pre-processor use and assembler, so this patch has been fun to write ;-).
I've tried to make the inline assembler clearer with named parameters, it really upset checkpatch, if it upsets people then please shout and I will change it.
Cheers,
--
Steve--- arch/arm64/include/asm/cmpxchg.h | 6 +- arch/arm64/include/asm/percpu.h | 231 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 235 insertions(+), 2 deletions(-)arch/arm64/include/asm/cmpxchg.h +4
-2@@ -237,8 +237,10 @@ static inline unsigned long __cmpxchg_mb(volatile void *ptr, unsigned long old, __ret; \ }) -#define this_cpu_cmpxchg_8 (ptr, o, n) \ - cmpxchg(raw_cpu_ptr(&(ptr)), o, n); +#define this_cpu_cmpxchg_1 (ptr, o, n) cmpxchg(raw_cpu_ptr(&(ptr)), o, n) +#define this_cpu_cmpxchg_2 (ptr, o, n) cmpxchg(raw_cpu_ptr(&(ptr)), o, n) +#define this_cpu_cmpxchg_4 (ptr, o, n) cmpxchg(raw_cpu_ptr(&(ptr)), o, n) +#define this_cpu_cmpxchg_8 (ptr, o, n) cmpxchg(raw_cpu_ptr(&(ptr)), o, n)#define this_cpu_cmpxchg_double_8 (ptr1, ptr2, o1, o2, n1, n2) \ cmpxchg_double(raw_cpu_ptr(&(ptr1)), raw_cpu_ptr(&(ptr2)), \arch/arm64/include/asm/percpu.h +85
-0@@ -44,6 +44,237 @@ static inline unsigned long __my_cpu_offset(void)#endif /* CONFIG_SMP */ +#define PERCPU_OP (op, asm_op) \ +static inline unsigned long __percpu_##op(void *ptr, \ + unsigned long val, int size) \ +{ \ + unsigned long loop, ret; \ + \ + switch (size) { \ + case 1: \ + do { \ + asm ("//__per_cpu_" #op "_1\n" \ + "ldxrb %w[ret], %[ptr]\n" \ + #asm_op " %w[ret], %w[ret], %w[val]\n" \ + "stxrb %w[loop], %w[ret], %[ptr]\n" \ + : [loop] "=&r" (loop), [ret] "=&r" (ret), \ + [ptr] "+Q"(*(u8 *)ptr) \ + : [val] "Ir" (val)); \ + } while (loop); \ + break; \ + case 2: \ + do { \ + asm ("//__per_cpu_" #op "_2\n" \ + "ldxrh %w[ret], %[ptr]\n" \ + #asm_op " %w[ret], %w[ret], %w[val]\n" \ + "stxrh %w[loop], %w[ret], %[ptr]\n" \ + : [loop] "=&r" (loop), [ret] "=&r" (ret), \ + [ptr] "+Q"(*(u16 *)ptr) \ + : [val] "Ir" (val)); \ + } while (loop); \ + break; \ + case 4: \ + do { \ + asm ("//__per_cpu_" #op "_4\n" \ + "ldxr %w[ret], %[ptr]\n" \ + #asm_op " %w[ret], %w[ret], %w[val]\n" \ + "stxr %w[loop], %w[ret], %[ptr]\n" \ + : [loop] "=&r" (loop), [ret] "=&r" (ret), \ + [ptr] "+Q"(*(u32 *)ptr) \ + : [val] "Ir" (val)); \ + } while (loop); \ + break; \ + case 8: \ + do { \ + asm ("//__per_cpu_" #op "_8\n" \ + "ldxr %[ret], %[ptr]\n" \ + #asm_op " %[ret], %[ret], %[val]\n" \ + "stxr %w[loop], %[ret], %[ptr]\n" \ + : [loop] "=&r" (loop), [ret] "=&r" (ret), \ + [ptr] "+Q"(*(u64 *)ptr) \ + : [val] "Ir" (val)); \ + } while (loop); \ + break; \ + default: \ + BUILD_BUG(); \ + } \ + \ + return ret; \ +} + +PERCPU_OP(add, add) +PERCPU_OP(and, and) +PERCPU_OP(or, orr) +#undef PERCPU_OP + +static inline unsigned long __percpu_read (void *ptr,int size) +{ +unsigned long ret; + +switch (size) { +case 1 : + asm ("//__per_cpu_read_1\n" +"ldrb %w[ret], %[ptr]\n" : + [ret]"=&r" (ret), [ptr]"+Q" (*(u8 *)ptr)); +break ; +case 2 : + asm ("//__per_cpu_read_2\n" +"ldrh %w[ret], %[ptr]\n" : + [ret]"=&r" (ret), [ptr]"+Q" (*(u16 *)ptr)); +break ; +case 4 : + asm ("//__per_cpu_read_4\n" +"ldr %w[ret], %[ptr]\n" : + [ret]"=&r" (ret), [ptr]"+Q" (*(u32 *)ptr)); +break ; +case 8 : + asm ("//__per_cpu_read_8\n"
arch/arm64/include/asm/percpu.h static inline unsigned long __my_cpu_offset(void) · line 141
Quoted 10 lines
arch/arm64/include/asm/percpu.h +10
-0+ "ldr %[ret], %[ptr]\n" : + [ret] "=&r"(ret), [ptr] "+Q"(*(u64 *)ptr)); + break; +default : +BUILD_BUG (); + } + +return ret; +} +
Why are the 'ptr' references '+Q' outputs here rather than 'Q' inputs?
Quoted 18 lines
arch/arm64/include/asm/percpu.h +18
-0+static inline void __percpu_write (void *ptr,unsigned long val,int size) +{ +switch (size) { +case 1 : + asm ("//__per_cpu_write_1\n" +"strb %w[val], %[ptr]\n" : + [ptr]"+Q" (*(u8 *)ptr) : [val]"r" (val)); +break ; +case 2 : + asm ("//__per_cpu_write_2\n" +"strh %w[val], %[ptr]\n" : + [ptr]"+Q" (*(u16 *)ptr) : [val]"r" (val)); +break ; +case 4 : + asm ("//__per_cpu_write_4\n" +"str %w[val], %[ptr]\n" : + [ptr]"+Q" (*(u32 *)ptr) : [val]"r" (val)); +break ;
arch/arm64/include/asm/percpu.h static inline unsigned long __my_cpu_offset(void) · line 169
Quoted 10 lines
arch/arm64/include/asm/percpu.h +10
-0+case 8 : + asm ("//__per_cpu_write_8\n" +"str %[val], %[ptr]\n" : + [ptr]"+Q" (*(u64 *)ptr) : [val]"r" (val)); +break ; +default : +BUILD_BUG (); + } +} +
... and similarly, why are these '+Q' and not just '=Q' ?
Quoted 119 lines
arch/arm64/include/asm/percpu.h +108
-1+static inline unsigned long __percpu_xchg (void *ptr,unsigned long val, +int size) +{ +unsigned long ret, loop; + +switch (size) { +case 1 : +do { + asm ("//__percpu_xchg_1\n" +"ldxrb %w[ret], %[ptr]\n" +"stxrb %w[loop], %w[val], %[ptr]\n" + : [loop]"=&r" (loop), [ret]"=&r" (ret), + [ptr]"+Q" (*(u8 *)ptr) + : [val]"r" (val)); + }while (loop); +break ; +case 2 : +do { + asm ("//__percpu_xchg_2\n" +"ldxrh %w[ret], %[ptr]\n" +"stxrh %w[loop], %w[val], %[ptr]\n" + : [loop]"=&r" (loop), [ret]"=&r" (ret), + [ptr]"+Q" (*(u16 *)ptr) + : [val]"r" (val)); + }while (loop); +break ; +case 4 : +do { + asm ("//__percpu_xchg_4\n" +"ldxr %w[ret], %[ptr]\n" +"stxr %w[loop], %w[val], %[ptr]\n" + : [loop]"=&r" (loop), [ret]"=&r" (ret), + [ptr]"+Q" (*(u32 *)ptr) + : [val]"r" (val)); + }while (loop); +break ; +case 8 : +do { + asm ("//__percpu_xchg_8\n" +"ldxr %[ret], %[ptr]\n" +"stxr %w[loop], %[val], %[ptr]\n" + : [loop]"=&r" (loop), [ret]"=&r" (ret), + [ptr]"+Q" (*(u64 *)ptr) + : [val]"r" (val)); + }while (loop); +break ; +default : +BUILD_BUG (); + } + +return ret; +} + +#define _percpu_add (pcp, val) \ + __percpu_add(raw_cpu_ptr(&(pcp)), val, sizeof(pcp)) + +#define _percpu_add_return (pcp, val) (typeof(pcp)) (_percpu_add(pcp, val)) + +#define _percpu_and (pcp, val) \ + __percpu_and(raw_cpu_ptr(&(pcp)), val, sizeof(pcp)) + +#define _percpu_or (pcp, val) \ + __percpu_or(raw_cpu_ptr(&(pcp)), val, sizeof(pcp)) + +#define _percpu_read (pcp) (typeof(pcp)) \ + (__percpu_read(raw_cpu_ptr(&(pcp)), sizeof(pcp))) + +#define _percpu_write (pcp, val) \ + __percpu_write(raw_cpu_ptr(&(pcp)), (unsigned long)(val), sizeof(pcp)) + +#define _percpu_xchg (pcp, val) (typeof(pcp)) \ + (__percpu_xchg(raw_cpu_ptr(&(pcp)), (unsigned long)(val), sizeof(pcp))) + +#define this_cpu_add_1 (pcp, val) _percpu_add(pcp, val) +#define this_cpu_add_2 (pcp, val) _percpu_add(pcp, val) +#define this_cpu_add_4 (pcp, val) _percpu_add(pcp, val) +#define this_cpu_add_8 (pcp, val) _percpu_add(pcp, val) + +#define this_cpu_add_return_1 (pcp, val) _percpu_add_return(pcp, val) +#define this_cpu_add_return_2 (pcp, val) _percpu_add_return(pcp, val) +#define this_cpu_add_return_4 (pcp, val) _percpu_add_return(pcp, val) +#define this_cpu_add_return_8 (pcp, val) _percpu_add_return(pcp, val) + +#define this_cpu_and_1 (pcp, val) _percpu_and(pcp, val) +#define this_cpu_and_2 (pcp, val) _percpu_and(pcp, val) +#define this_cpu_and_4 (pcp, val) _percpu_and(pcp, val) +#define this_cpu_and_8 (pcp, val) _percpu_and(pcp, val) + +#define this_cpu_or_1 (pcp, val) _percpu_or(pcp, val) +#define this_cpu_or_2 (pcp, val) _percpu_or(pcp, val) +#define this_cpu_or_4 (pcp, val) _percpu_or(pcp, val) +#define this_cpu_or_8 (pcp, val) _percpu_or(pcp, val) + +#define this_cpu_read_1 (pcp) _percpu_read(pcp) +#define this_cpu_read_2 (pcp) _percpu_read(pcp) +#define this_cpu_read_4 (pcp) _percpu_read(pcp) +#define this_cpu_read_8 (pcp) _percpu_read(pcp) + +#define this_cpu_write_1 (pcp, val) _percpu_write(pcp, val) +#define this_cpu_write_2 (pcp, val) _percpu_write(pcp, val) +#define this_cpu_write_4 (pcp, val) _percpu_write(pcp, val) +#define this_cpu_write_8 (pcp, val) _percpu_write(pcp, val) + +#define this_cpu_xchg_1 (pcp, val) _percpu_xchg(pcp, val) +#define this_cpu_xchg_2 (pcp, val) _percpu_xchg(pcp, val) +#define this_cpu_xchg_4 (pcp, val) _percpu_xchg(pcp, val) +#define this_cpu_xchg_8 (pcp, val) _percpu_xchg(pcp, val) +#include <asm-generic/percpu.h> #endif /* __ASM_PERCPU_H */ --1.9.3
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH 1/1 arm64: percpu: Implement this_cpu operations
Quoted 1 line · Ard Biesheuvel
Hi Steve,
Hey Ard,
Quoted 14 lines · Steve Capper
Quoted 12 lines
The generic
this_cpuoperations disable interrupts to ensure that the requested operation is protected from pre-emption. For arm64, this is overkill and can hurt throughput and latency.This patch provides arm64 specific implementations for the
this_cpuoperations. Rather than disable interrupts, we use the exclusive monitor or atomic operations as appropriate.The following operations are implemented: add,
add_return, and, or, read, write, xchg. We also wire up a cmpxchg implementation fromcmpxchg.h.
Quoted 5 lines
Quoted 3 lines
Testing was performed using the
percpu_testmodule and hackbench on a Juno board running 3.18-rc3.Got any numbers?
For `./hackbench 100 process 1000' on a Juno system running all cores I get:
w/o patch | with patch
---------------+------------
mean 47.05 | 44.93
stddev 0.38 | 0.68
---------------+------------
(times in seconds, rounded to 2d.p., six runs performed)
So a just under 5% speed boost.
[...]
Quoted 5 lines
Quoted 5 lines
arch/arm64/include/asm/percpu.h +0
-0@@ -44,6 +44,237 @@ static inline unsigned long __my_cpu_offset(void)
[...]
Quoted 26 lines
Quoted 26 lines
arch/arm64/include/asm/percpu.h +26
-0+static inline unsigned long __percpu_read (void *ptr,int size) +{ +unsigned long ret; + +switch (size) { +case 1 : + asm ("//__per_cpu_read_1\n" +"ldrb %w[ret], %[ptr]\n" : + [ret]"=&r" (ret), [ptr]"+Q" (*(u8 *)ptr)); +break ; +case 2 : + asm ("//__per_cpu_read_2\n" +"ldrh %w[ret], %[ptr]\n" : + [ret]"=&r" (ret), [ptr]"+Q" (*(u16 *)ptr)); +break ; +case 4 : + asm ("//__per_cpu_read_4\n" +"ldr %w[ret], %[ptr]\n" : + [ret]"=&r" (ret), [ptr]"+Q" (*(u32 *)ptr)); +break ; +case 8 : + asm ("//__per_cpu_read_8\n" +"ldr %[ret], %[ptr]\n" : + [ret]"=&r" (ret), [ptr]"+Q" (*(u64 *)ptr)); +break ; +default :
Quoted 9 lines
arch/arm64/include/asm/percpu.h static inline unsigned long __my_cpu_offset(void) · line 141
Quoted 6 lines
arch/arm64/include/asm/percpu.h +6
-0+BUILD_BUG (); + } + +return ret; +} +Why are the 'ptr' references '+Q' outputs here rather than 'Q' inputs?
Because I fudged the constraint :-).
Thanks, you're quite right it should be 'Q' constraint as we're only reading from that address.
Quoted 22 lines
Quoted 22 lines
arch/arm64/include/asm/percpu.h +22
-0+static inline void __percpu_write (void *ptr,unsigned long val,int size) +{ +switch (size) { +case 1 : + asm ("//__per_cpu_write_1\n" +"strb %w[val], %[ptr]\n" : + [ptr]"+Q" (*(u8 *)ptr) : [val]"r" (val)); +break ; +case 2 : + asm ("//__per_cpu_write_2\n" +"strh %w[val], %[ptr]\n" : + [ptr]"+Q" (*(u16 *)ptr) : [val]"r" (val)); +break ; +case 4 : + asm ("//__per_cpu_write_4\n" +"str %w[val], %[ptr]\n" : + [ptr]"+Q" (*(u32 *)ptr) : [val]"r" (val)); +break ; +case 8 : + asm ("//__per_cpu_write_8\n" +"str %[val], %[ptr]\n" : + [ptr]"+Q" (*(u64 *)ptr) : [val]"r" (val));
Quoted 8 lines
arch/arm64/include/asm/percpu.h static inline unsigned long __my_cpu_offset(void) · line 141
Quoted 6 lines
arch/arm64/include/asm/percpu.h +6
-0+break ; +default : +BUILD_BUG (); + } +} +... and similarly, why are these '+Q' and not just '=Q' ?
Another mistake on my part, it should be "=Q" as we're only writing to that address.
Thanks for going through this carefully Ard, I'll send out a corrected V2 once I've sanity checked it.
Cheers,
Signature · 1 line
Steve
PATCH v2 1/1 arm64: percpu: Implement this_cpu operations
The generic this_cpu operations disable interrupts to ensure that the requested operation is protected from pre-emption. For arm64, this is overkill and can hurt throughput and latency.
This patch provides arm64 specific implementations for the this_cpu operations. Rather than disable interrupts, we use the exclusive monitor or atomic operations as appropriate.
The following operations are implemented: add, add_return, and, or, read, write, xchg. We also wire up a cmpxchg implementation from cmpxchg.h.
Testing was performed using the percpu_test module and hackbench on a Juno board running 3.18-rc3.
---
Changed in V2: corrected address constraint for __percpu_read and __percpu_write.
--- arch/arm64/include/asm/cmpxchg.h | 6 +- arch/arm64/include/asm/percpu.h | 231 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 235 insertions(+), 2 deletions(-)
arch/arm64/include/asm/cmpxchg.h +4 -2
@@ -237,8 +237,10 @@ static inline unsigned long __cmpxchg_mb(volatile void *ptr, unsigned long old, __ret; \ }) -#define this_cpu_cmpxchg_8 (ptr, o, n) \ - cmpxchg(raw_cpu_ptr(&(ptr)), o, n); +#define this_cpu_cmpxchg_1 (ptr, o, n) cmpxchg(raw_cpu_ptr(&(ptr)), o, n) +#define this_cpu_cmpxchg_2 (ptr, o, n) cmpxchg(raw_cpu_ptr(&(ptr)), o, n) +#define this_cpu_cmpxchg_4 (ptr, o, n) cmpxchg(raw_cpu_ptr(&(ptr)), o, n) +#define this_cpu_cmpxchg_8 (ptr, o, n) cmpxchg(raw_cpu_ptr(&(ptr)), o, n)#define this_cpu_cmpxchg_double_8 (ptr1, ptr2, o1, o2, n1, n2) \ cmpxchg_double(raw_cpu_ptr(&(ptr1)), raw_cpu_ptr(&(ptr2)), \
arch/arm64/include/asm/percpu.h +231 -0
237 lines not on this page · raw message
Signature · 1 line
1.9.3
PATCH 1/1 arm64: percpu: Implement this_cpu operations
In reply to Steve CapperHi Steve,
Thanks for looking at this!
Quoted 14 lines · Steve Capper
The generic
this_cpuoperations disable interrupts to ensure that the requested operation is protected from pre-emption. For arm64, this is overkill and can hurt throughput and latency.This patch provides arm64 specific implementations for the
this_cpuoperations. Rather than disable interrupts, we use the exclusive monitor or atomic operations as appropriate.The following operations are implemented: add,
add_return, and, or, read, write, xchg. We also wire up a cmpxchg implementation fromcmpxchg.h.Testing was performed using the
percpu_testmodule and hackbench on a Juno board running 3.18-rc3.
[...]
Quoted 5 lines
arch/arm64/include/asm/cmpxchg.h +0
-0
arch/arm64/include/asm/cmpxchg.h static inline unsigned long __cmpxchg_mb(volatile void *ptr, unsigned long old, · line 243
Quoted 9 lines
arch/arm64/include/asm/cmpxchg.h +4
-2@@ -237,8 +237,10 @@ static inline unsigned long __cmpxchg_mb(volatile void *ptr, unsigned long old, __ret; \ }) -#define this_cpu_cmpxchg_8 (ptr, o, n) \ - cmpxchg(raw_cpu_ptr(&(ptr)), o, n); +#define this_cpu_cmpxchg_1 (ptr, o, n) cmpxchg(raw_cpu_ptr(&(ptr)), o, n) +#define this_cpu_cmpxchg_2 (ptr, o, n) cmpxchg(raw_cpu_ptr(&(ptr)), o, n) +#define this_cpu_cmpxchg_4 (ptr, o, n) cmpxchg(raw_cpu_ptr(&(ptr)), o, n) +#define this_cpu_cmpxchg_8 (ptr, o, n) cmpxchg(raw_cpu_ptr(&(ptr)), o, n)
You can use cmpxchg_local here, as we don't require barrier semantics.
Quoted 18 lines
arch/arm64/include/asm/cmpxchg.h +0
-0#define this_cpu_cmpxchg_double_8 (ptr1, ptr2, o1, o2, n1, n2) \ cmpxchg_double(raw_cpu_ptr(&(ptr1)), raw_cpu_ptr(&(ptr2)), \arch/arm64/include/asm/percpu.h +8
-0@@ -44,6 +44,237 @@ static inline unsigned long __my_cpu_offset(void)#endif /* CONFIG_SMP */ +#define PERCPU_OP (op, asm_op) \ +static inline unsigned long __percpu_##op(void *ptr, \ + unsigned long val, int size) \ +{ \ + unsigned long loop, ret; \ + \ + switch (size) { \ + case 1: \
arch/arm64/include/asm/percpu.h static inline unsigned long __my_cpu_offset(void) · line 64
Quoted 10 lines
arch/arm64/include/asm/percpu.h +10
-0+ do { \ + asm ("//__per_cpu_" #op "_1\n" \ + "ldxrb %w[ret], %[ptr]\n" \ + #asm_op " %w[ret], %w[ret], %w[val]\n" \ + "stxrb %w[loop], %w[ret], %[ptr]\n" \ + : [loop] "=&r" (loop), [ret] "=&r" (ret), \ + [ptr] "+Q"(*(u8 *)ptr) \ + : [val] "Ir" (val)); \ + } while (loop); \ + break; \
Curious, but do you see any difference in code generation over an explicit cbnz, like we use in the ATOMIC_OP macro?
Quoted 33 lines
arch/arm64/include/asm/percpu.h +33
-0+ case 2: \ + do { \ + asm ("//__per_cpu_" #op "_2\n" \ + "ldxrh %w[ret], %[ptr]\n" \ + #asm_op " %w[ret], %w[ret], %w[val]\n" \ + "stxrh %w[loop], %w[ret], %[ptr]\n" \ + : [loop] "=&r" (loop), [ret] "=&r" (ret), \ + [ptr] "+Q"(*(u16 *)ptr) \ + : [val] "Ir" (val)); \ + } while (loop); \ + break; \ + case 4: \ + do { \ + asm ("//__per_cpu_" #op "_4\n" \ + "ldxr %w[ret], %[ptr]\n" \ + #asm_op " %w[ret], %w[ret], %w[val]\n" \ + "stxr %w[loop], %w[ret], %[ptr]\n" \ + : [loop] "=&r" (loop), [ret] "=&r" (ret), \ + [ptr] "+Q"(*(u32 *)ptr) \ + : [val] "Ir" (val)); \ + } while (loop); \ + break; \ + case 8: \ + do { \ + asm ("//__per_cpu_" #op "_8\n" \ + "ldxr %[ret], %[ptr]\n" \ + #asm_op " %[ret], %[ret], %[val]\n" \ + "stxr %w[loop], %[ret], %[ptr]\n" \ + : [loop] "=&r" (loop), [ret] "=&r" (ret), \ + [ptr] "+Q"(*(u64 *)ptr) \ + : [val] "Ir" (val)); \ + } while (loop); \ + break; \
arch/arm64/include/asm/percpu.h static inline unsigned long __my_cpu_offset(void) · line 107
Quoted 10 lines
arch/arm64/include/asm/percpu.h +10
-0+default : \ +BUILD_BUG (); \ + } \ + \ +return ret; \ +} + +PERCPU_OP(add, add) +PERCPU_OP(and, and) +PERCPU_OP (or, orr)
Can you use these to generate local_t versions too?
Will
PATCH 1/1 arm64: percpu: Implement this_cpu operations
Quoted 3 lines · Will Deacon
Hi Steve,
Thanks for looking at this!
Hey Will,
No problem, it's quite beneficial for performance.
Quoted 27 lines · Steve Capper
Quoted 14 lines
The generic
this_cpuoperations disable interrupts to ensure that the requested operation is protected from pre-emption. For arm64, this is overkill and can hurt throughput and latency.This patch provides arm64 specific implementations for the
this_cpuoperations. Rather than disable interrupts, we use the exclusive monitor or atomic operations as appropriate.The following operations are implemented: add,
add_return, and, or, read, write, xchg. We also wire up a cmpxchg implementation fromcmpxchg.h.Testing was performed using the
percpu_testmodule and hackbench on a Juno board running 3.18-rc3.[...]
Quoted 8 lines
arch/arm64/include/asm/cmpxchg.h +0
-0@@ -237,8 +237,10 @@ static inline unsigned long __cmpxchg_mb(volatile void *ptr, unsigned long old, __ret; \ })
Quoted 8 lines
arch/arm64/include/asm/cmpxchg.h static inline unsigned long __cmpxchg_mb(volatile void *ptr, unsigned long old, · line 243
Quoted 6 lines
arch/arm64/include/asm/cmpxchg.h +4
-2-#define this_cpu_cmpxchg_8 (ptr, o, n) \ - cmpxchg(raw_cpu_ptr(&(ptr)), o, n); +#define this_cpu_cmpxchg_1 (ptr, o, n) cmpxchg(raw_cpu_ptr(&(ptr)), o, n) +#define this_cpu_cmpxchg_2 (ptr, o, n) cmpxchg(raw_cpu_ptr(&(ptr)), o, n) +#define this_cpu_cmpxchg_4 (ptr, o, n) cmpxchg(raw_cpu_ptr(&(ptr)), o, n) +#define this_cpu_cmpxchg_8 (ptr, o, n) cmpxchg(raw_cpu_ptr(&(ptr)), o, n)You can use
cmpxchg_localhere, as we don't require barrier semantics.
Agreed, thanks, I will update that.
Quoted 24 lines
Quoted 24 lines
arch/arm64/include/asm/cmpxchg.h +0
-0#define this_cpu_cmpxchg_double_8 (ptr1, ptr2, o1, o2, n1, n2) \ cmpxchg_double(raw_cpu_ptr(&(ptr1)), raw_cpu_ptr(&(ptr2)), \arch/arm64/include/asm/percpu.h +13
-0@@ -44,6 +44,237 @@ static inline unsigned long __my_cpu_offset(void)#endif /* CONFIG_SMP */ +#define PERCPU_OP (op, asm_op) \ +static inline unsigned long __percpu_##op(void *ptr, \ + unsigned long val, int size) \ +{ \ + unsigned long loop, ret; \ + \ + switch (size) { \ + case 1: \ + do { \ + asm ("//__per_cpu_" #op "_1\n" \ + "ldxrb %w[ret], %[ptr]\n" \ + #asm_op " %w[ret], %w[ret], %w[val]\n" \ + "stxrb %w[loop], %w[ret], %[ptr]\n" \
Quoted 8 lines
arch/arm64/include/asm/percpu.h static inline unsigned long __my_cpu_offset(void) · line 64
Quoted 5 lines
arch/arm64/include/asm/percpu.h +5
-0+ : [loop] "=&r" (loop), [ret] "=&r" (ret), \ +[ptr] " +Q "(*(u8 *)ptr) \ +: [val] " Ir" (val)); \ +} while (loop); \ +break; \ Curious, but do you see any difference in code generation over an explicit cbnz, like we use in the
ATOMIC_OPmacro?
I've not noticed any substancial difference in the code paths I've inspected. Theoretically, a compiler that is extremely averse to branches could do some unrolling with this.
I decided to give the compiler as much control as possible, so elected to put the minimum amount of assembler in.
Quoted 38 lines
Quoted 38 lines
arch/arm64/include/asm/percpu.h +37
-0+ case 2: \ + do { \ + asm ("//__per_cpu_" #op "_2\n" \ + "ldxrh %w[ret], %[ptr]\n" \ + #asm_op " %w[ret], %w[ret], %w[val]\n" \ + "stxrh %w[loop], %w[ret], %[ptr]\n" \ + : [loop] "=&r" (loop), [ret] "=&r" (ret), \ + [ptr] "+Q"(*(u16 *)ptr) \ + : [val] "Ir" (val)); \ + } while (loop); \ + break; \ + case 4: \ + do { \ + asm ("//__per_cpu_" #op "_4\n" \ + "ldxr %w[ret], %[ptr]\n" \ + #asm_op " %w[ret], %w[ret], %w[val]\n" \ + "stxr %w[loop], %w[ret], %[ptr]\n" \ + : [loop] "=&r" (loop), [ret] "=&r" (ret), \ + [ptr] "+Q"(*(u32 *)ptr) \ + : [val] "Ir" (val)); \ + } while (loop); \ + break; \ + case 8: \ + do { \ + asm ("//__per_cpu_" #op "_8\n" \ + "ldxr %[ret], %[ptr]\n" \ + #asm_op " %[ret], %[ret], %[val]\n" \ + "stxr %w[loop], %[ret], %[ptr]\n" \ + : [loop] "=&r" (loop), [ret] "=&r" (ret), \ + [ptr] "+Q"(*(u64 *)ptr) \ + : [val] "Ir" (val)); \ + } while (loop); \ + break; \ + default: \ + BUILD_BUG(); \ + } \ + \
Quoted 8 lines
arch/arm64/include/asm/percpu.h static inline unsigned long __my_cpu_offset(void) · line 107
Quoted 6 lines
arch/arm64/include/asm/percpu.h +6
-0+return ret; \ +} + +PERCPU_OP(add, add) +PERCPU_OP(and, and) +PERCPU_OP (or, orr)Can you use these to generate
local_tversions too?
Sure, I forgot about them, I will add them to V3.
Cheers,
Signature · 1 line
Steve
PATCH v3 1/1 arm64: percpu: Implement this_cpu operations
The generic this_cpu operations disable interrupts to ensure that the requested operation is protected from pre-emption. For arm64, this is overkill and can hurt throughput and latency.
This patch provides arm64 specific implementations for the this_cpu operations. Rather than disable interrupts, we use the exclusive monitor or atomic operations as appropriate.
The following operations are implemented: add, add_return, and, or, read, write, xchg. We also wire up a cmpxchg implementation from cmpxchg.h.
Testing was performed using the percpu_test module and hackbench on a Juno board running 3.18-rc4.
---
Having thought more about it, I will send out a separate patch to implement local.h for arm64 as it's logically separate.
Changed in V3: use cmpxchg_local rather than cmpxchg for this_cpu_cmpxchg.
Changed in V2: corrected address constraint for __percpu_read and __percpu_write.
--- arch/arm64/include/asm/cmpxchg.h | 6 +- arch/arm64/include/asm/percpu.h | 231 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 235 insertions(+), 2 deletions(-)
arch/arm64/include/asm/cmpxchg.h +4 -2
@@ -237,8 +237,10 @@ static inline unsigned long __cmpxchg_mb(volatile void *ptr, unsigned long old, __ret; \ }) -#define this_cpu_cmpxchg_8 (ptr, o, n) \ - cmpxchg(raw_cpu_ptr(&(ptr)), o, n); +#define this_cpu_cmpxchg_1 (ptr, o, n) cmpxchg_local(raw_cpu_ptr(&(ptr)), o, n) +#define this_cpu_cmpxchg_2 (ptr, o, n) cmpxchg_local(raw_cpu_ptr(&(ptr)), o, n) +#define this_cpu_cmpxchg_4 (ptr, o, n) cmpxchg_local(raw_cpu_ptr(&(ptr)), o, n) +#define this_cpu_cmpxchg_8 (ptr, o, n) cmpxchg_local(raw_cpu_ptr(&(ptr)), o, n)#define this_cpu_cmpxchg_double_8 (ptr1, ptr2, o1, o2, n1, n2) \ cmpxchg_double(raw_cpu_ptr(&(ptr1)), raw_cpu_ptr(&(ptr2)), \
arch/arm64/include/asm/percpu.h +231 -0
237 lines not on this page · raw message
Signature · 1 line
1.9.3
PATCH v3 1/1 arm64: percpu: Implement this_cpu operations
Hi Steve,
Quoted 8 lines · Steve Capper
The generic
this_cpuoperations disable interrupts to ensure that the requested operation is protected from pre-emption. For arm64, this is overkill and can hurt throughput and latency.This patch provides arm64 specific implementations for the
this_cpuoperations. Rather than disable interrupts, we use the exclusive monitor or atomic operations as appropriate.
Quoted 6 lines
The following operations are implemented: add,
add_return, and, or, read, write, xchg. We also wire up a cmpxchg implementation fromcmpxchg.h.Testing was performed using the
percpu_testmodule and hackbench on a Juno board running 3.18-rc4.
What does this patch apply against? I'm struggling to apply it to our for-next branch (perhaps it conflicts with your other cmpxchg patch?)
Anyway, one comment below.
Quoted 49 lines
arch/arm64/include/asm/percpu.h +49
-0+static inline unsigned long __percpu_read (void *ptr,int size) +{ +unsigned long ret; + +switch (size) { +case 1 : + asm ("//__per_cpu_read_1\n" +"ldrb %w[ret], %[ptr]\n" : + [ret]"=&r" (ret) : [ptr]"Q" (*(u8 *)ptr)); +break ; +case 2 : + asm ("//__per_cpu_read_2\n" +"ldrh %w[ret], %[ptr]\n" : + [ret]"=&r" (ret) : [ptr]"Q" (*(u16 *)ptr)); +break ; +case 4 : + asm ("//__per_cpu_read_4\n" +"ldr %w[ret], %[ptr]\n" : + [ret]"=&r" (ret) : [ptr]"Q" (*(u32 *)ptr)); +break ; +case 8 : + asm ("//__per_cpu_read_8\n" +"ldr %[ret], %[ptr]\n" : + [ret]"=&r" (ret) : [ptr]"Q" (*(u64 *)ptr)); +break ; +default : +BUILD_BUG (); + } + +return ret; +} + +static inline void __percpu_write (void *ptr,unsigned long val,int size) +{ +switch (size) { +case 1 : + asm ("//__per_cpu_write_1\n" +"strb %w[val], %[ptr]\n" : + [ptr]"=Q" (*(u8 *)ptr) : [val]"r" (val)); +break ; +case 2 : + asm ("//__per_cpu_write_2\n" +"strh %w[val], %[ptr]\n" : + [ptr]"=Q" (*(u16 *)ptr) : [val]"r" (val)); +break ; +case 4 : + asm ("//__per_cpu_write_4\n" +"str %w[val], %[ptr]\n" : + [ptr]"=Q" (*(u32 *)ptr) : [val]"r" (val));
arch/arm64/include/asm/percpu.h static inline unsigned long __my_cpu_offset(void) · line 168
Quoted 10 lines
arch/arm64/include/asm/percpu.h +10
-0+break ; +case 8 : + asm ("//__per_cpu_write_8\n" +"str %[val], %[ptr]\n" : + [ptr]"=Q" (*(u64 *)ptr) : [val]"r" (val)); +break ; +default : +BUILD_BUG (); + } +}
Can you implement the read/write accessors with ACCESS_ONCE instead? I think we're just after a single-copy atomic access without barrier semantics, so that should work if you get your types right.
Will
PATCH v4 1/1 arm64: percpu: Implement this_cpu operations
The generic this_cpu operations disable interrupts to ensure that the requested operation is protected from pre-emption. For arm64, this is overkill and can hurt throughput and latency.
This patch provides arm64 specific implementations for the this_cpu operations. Rather than disable interrupts, we use the exclusive monitor or atomic operations as appropriate.
The following operations are implemented: add, add_return, and, or, read, write, xchg. We also wire up a cmpxchg implementation from cmpxchg.h.
Testing was performed using the percpu_test module and hackbench on a Juno board running 3.18-rc4.
---
Changed in V4: rebased to allow for merge in Will's for-next branch. (I had erroneously changed my cmpxchg_double patch).
Changed the read/write accessors to use ACCESS_ONCE rather than asm.
Changed in V3: use cmpxchg_local rather than cmpxchg for this_cpu_cmpxchg.
Changed in V2: corrected address constraint for __percpu_read and __percpu_write.
--- arch/arm64/include/asm/cmpxchg.h | 6 +- arch/arm64/include/asm/percpu.h | 215 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 219 insertions(+), 2 deletions(-)
arch/arm64/include/asm/cmpxchg.h +4 -2
@@ -246,8 +246,10 @@ static inline unsigned long __cmpxchg_mb(volatile void *ptr, unsigned long old, __ret; \ }) -#define this_cpu_cmpxchg_8 (ptr, o, n) \ - cmpxchg_local(raw_cpu_ptr(&(ptr)), o, n) +#define this_cpu_cmpxchg_1 (ptr, o, n) cmpxchg_local(raw_cpu_ptr(&(ptr)), o, n) +#define this_cpu_cmpxchg_2 (ptr, o, n) cmpxchg_local(raw_cpu_ptr(&(ptr)), o, n) +#define this_cpu_cmpxchg_4 (ptr, o, n) cmpxchg_local(raw_cpu_ptr(&(ptr)), o, n) +#define this_cpu_cmpxchg_8 (ptr, o, n) cmpxchg_local(raw_cpu_ptr(&(ptr)), o, n)#define this_cpu_cmpxchg_double_8 (ptr1, ptr2, o1, o2, n1, n2) \ cmpxchg_double_local(raw_cpu_ptr(&(ptr1)), raw_cpu_ptr(&(ptr2)), \
arch/arm64/include/asm/percpu.h +215 -0
221 lines not on this page · raw message
Signature · 1 line
1.9.3
PATCH v4 1/1 arm64: percpu: Implement this_cpu operations
Hi Steve,
Quoted 8 lines · Steve Capper
The generic
this_cpuoperations disable interrupts to ensure that the requested operation is protected from pre-emption. For arm64, this is overkill and can hurt throughput and latency.This patch provides arm64 specific implementations for the
this_cpuoperations. Rather than disable interrupts, we use the exclusive monitor or atomic operations as appropriate.
Quoted 6 lines
The following operations are implemented: add,
add_return, and, or, read, write, xchg. We also wire up a cmpxchg implementation fromcmpxchg.h.Testing was performed using the
percpu_testmodule and hackbench on a Juno board running 3.18-rc4.
Looks good. I notice that this change drops the compiler barriers too, which we used to get via local_irq_{enable/disable}. I *think* that's fine (at least, I couldn't find a place that breaks due to that) but it would be nice to know that it was deliberate :)
Quoted 5 lines
arch/arm64/include/asm/percpu.h +5
-0+static inline void __percpu_write (void *ptr,unsigned long val,int size) +{ +switch (size) { +case 1 : +ACCESS_ONCE (*((u8 *)ptr)) = (u8 ) val;
arch/arm64/include/asm/percpu.h static inline unsigned long __my_cpu_offset(void) · line 148
Quoted 10 lines
arch/arm64/include/asm/percpu.h +10
-0+break ; +case 2 : +ACCESS_ONCE (*((u16 *)ptr)) = (u16 ) val; +break ; +case 4 : +ACCESS_ONCE (*((u32 *)ptr)) = (u32 ) val; +break ; +case 8 : +ACCESS_ONCE (*((u64 *)ptr)) = (u64 ) val; +break ;
I think you've gone a bit overboard with brackets and spacing here. Can't you just do something like:
ACCESS_ONCE(*(u64 *)ptr) = (u64)val;
Anyway:
Will
PATCH v4 1/1 arm64: percpu: Implement this_cpu operations
Quoted 1 line · Will Deacon
Hi Steve,
Hey Will,
[sorry for the late reply, was recovering from plague]
Quoted 14 lines · Steve Capper
Quoted 12 lines
The generic
this_cpuoperations disable interrupts to ensure that the requested operation is protected from pre-emption. For arm64, this is overkill and can hurt throughput and latency.This patch provides arm64 specific implementations for the
this_cpuoperations. Rather than disable interrupts, we use the exclusive monitor or atomic operations as appropriate.The following operations are implemented: add,
add_return, and, or, read, write, xchg. We also wire up a cmpxchg implementation fromcmpxchg.h.
Quoted 7 lines
Quoted 2 lines
Testing was performed using the
percpu_testmodule and hackbench on a Juno board running 3.18-rc4.Looks good. I notice that this change drops the compiler barriers too, which we used to get via local_irq_{enable/disable}. I *think* that's fine (at least, I couldn't find a place that breaks due to that) but it would be nice to know that it was deliberate :)
Yes it was deliberate. :-)
My understanding of the this_cpu... set of accessors is that they are there to solely protect the access with respect to pre-emption, not other CPUs, thus we shouldn't need barriers?
Quoted 20 lines
Quoted 6 lines
arch/arm64/include/asm/percpu.h +5
-0+static inline void __percpu_write (void *ptr,unsigned long val,int size) +{ +switch (size) { +case 1 : +ACCESS_ONCE (*((u8 *)ptr)) = (u8 ) val;arch/arm64/include/asm/percpu.h static inline unsigned long __my_cpu_offset(void) · line 148
Quoted 10 lines
arch/arm64/include/asm/percpu.h +10
-0+break ; +case 2 : +ACCESS_ONCE (*((u16 *)ptr)) = (u16 ) val; +break ; +case 4 : +ACCESS_ONCE (*((u32 *)ptr)) = (u32 ) val; +break ; +case 8 : +ACCESS_ONCE (*((u64 *)ptr)) = (u64 ) val; +break ;I think you've gone a bit overboard with brackets and spacing here. Can't you just do something like:
Quoted 1 line
ACCESS_ONCE(*(u64 *)ptr) = (u64)val;
Yeah, I went a little paranthesis crazy there, I'll correct this lest it gets confused with LISP.
Quoted 4 lines
Anyway:
Reviewed-by Will Deacon <[email protected]>
Ta Will,
I'll send out a V5 with the parantheses sanetised.
Cheers,
Signature · 1 line
Steve
PATCH v5 1/1 arm64: percpu: Implement this_cpu operations
The generic this_cpu operations disable interrupts to ensure that the requested operation is protected from pre-emption. For arm64, this is overkill and can hurt throughput and latency.
This patch provides arm64 specific implementations for the this_cpu operations. Rather than disable interrupts, we use the exclusive monitor or atomic operations as appropriate.
The following operations are implemented: add, add_return, and, or, read, write, xchg. We also wire up a cmpxchg implementation from cmpxchg.h.
Testing was performed using the percpu_test module and hackbench on a Juno board running 3.18-rc4.
---
Changed in V5: spacing and parantheses sanetised for read/write accessors.
Changed in V4: rebased to allow for merge in Will's for-next branch. (I had erroneously changed my cmpxchg_double patch).
Changed the read/write accessors to use ACCESS_ONCE rather than asm.
Changed in V3: use cmpxchg_local rather than cmpxchg for this_cpu_cmpxchg.
Changed in V2: corrected address constraint for __percpu_read and __percpu_write.
--- arch/arm64/include/asm/cmpxchg.h | 6 +- arch/arm64/include/asm/percpu.h | 215 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 219 insertions(+), 2 deletions(-)
arch/arm64/include/asm/cmpxchg.h +4 -2
@@ -246,8 +246,10 @@ static inline unsigned long __cmpxchg_mb(volatile void *ptr, unsigned long old, __ret; \ }) -#define this_cpu_cmpxchg_8 (ptr, o, n) \ - cmpxchg_local(raw_cpu_ptr(&(ptr)), o, n) +#define this_cpu_cmpxchg_1 (ptr, o, n) cmpxchg_local(raw_cpu_ptr(&(ptr)), o, n) +#define this_cpu_cmpxchg_2 (ptr, o, n) cmpxchg_local(raw_cpu_ptr(&(ptr)), o, n) +#define this_cpu_cmpxchg_4 (ptr, o, n) cmpxchg_local(raw_cpu_ptr(&(ptr)), o, n) +#define this_cpu_cmpxchg_8 (ptr, o, n) cmpxchg_local(raw_cpu_ptr(&(ptr)), o, n)#define this_cpu_cmpxchg_double_8 (ptr1, ptr2, o1, o2, n1, n2) \ cmpxchg_double_local(raw_cpu_ptr(&(ptr1)), raw_cpu_ptr(&(ptr2)), \
arch/arm64/include/asm/percpu.h +215 -0
221 lines not on this page · raw message
Signature · 1 line
1.9.3