Re: [PATCH 3/3] rseq/selftests: Add support for arm64
From: Will Deacon
Date: Mon Jul 02 2018 - 12:49:05 EST
On Thu, Jun 28, 2018 at 04:50:40PM -0400, Mathieu Desnoyers wrote:
> ----- On Jun 28, 2018, at 12:47 PM, Will Deacon will.deacon@xxxxxxx wrote:
> > On Tue, Jun 26, 2018 at 12:11:52PM -0400, Mathieu Desnoyers wrote:
> >> ----- On Jun 26, 2018, at 11:14 AM, Will Deacon will.deacon@xxxxxxx wrote:
> >> > On Mon, Jun 25, 2018 at 02:10:10PM -0400, Mathieu Desnoyers wrote:
> >> >> I notice you are using the instructions
> >> >>
> >> >> adrp
> >> >> add
> >> >> str
> >> >>
> >> >> to implement RSEQ_ASM_STORE_RSEQ_CS(). Did you compare
> >> >> performance-wise with an approach using a literal pool
> >> >> near the instruction pointer like I did on arm32 ?
> >> >
> >> > I didn't, no. Do you have a benchmark to hand so I can give this a go?
> >>
> >> see tools/testing/selftests/rseq/param_test_benchmark --help
> >>
> >> It's a stripped-down version of param_test, without all the code for
> >> delay loops and testing checks.
> >>
> >> Example use for counter increment with 4 threads, doing 5G counter
> >> increments per thread:
> >>
> >> time ./param_test_benchmark -T i -t 4 -r 5000000000
> >
> > Thanks. I ran that on a few arm64 systems I have access to, with three
> > configurations of the selftest:
> >
> > 1. As I posted
> > 2. With the abort signature and branch in-lined, so as to avoid the CBNZ
> > address limitations in large codebases
> > 3. With both the abort handler and the table inlined (i.e. the same thing
> > as 32-bit).
> >
> > There isn't a reliably measurable difference between (1) and (2), but I take
> > between 12% and 27% hit between (2) and (3).
>
> Those results puzzle me. Do you have the actual code snippets of each
> implementation nearby ?
Sure, I've included the diffs for (2) and (3) below. They both apply on top
of my branch at:
git://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git rseq
Will
--->8
diff --git a/tools/testing/selftests/rseq/rseq-arm64.h b/tools/testing/selftests/rseq/rseq-arm64.h
index 599788f74137..954f34671ca6 100644
--- a/tools/testing/selftests/rseq/rseq-arm64.h
+++ b/tools/testing/selftests/rseq/rseq-arm64.h
@@ -104,11 +104,11 @@ do { \
__rseq_str(label) ":\n"
#define RSEQ_ASM_DEFINE_ABORT(label, abort_label) \
- " .pushsection __rseq_failure, \"ax\"\n" \
- " .long " __rseq_str(RSEQ_SIG) "\n" \
+ " b 222f\n" \
+ " .inst " __rseq_str(RSEQ_SIG) "\n" \
__rseq_str(label) ":\n" \
" b %l[" __rseq_str(abort_label) "]\n" \
- " .popsection\n"
+ "222:\n"
#define RSEQ_ASM_OP_STORE(value, var) \
" str %[" __rseq_str(value) "], %[" __rseq_str(var) "]\n"
--->8
diff --git a/tools/testing/selftests/rseq/rseq-arm64.h b/tools/testing/selftests/rseq/rseq-arm64.h
index 599788f74137..2554aa17acf3 100644
--- a/tools/testing/selftests/rseq/rseq-arm64.h
+++ b/tools/testing/selftests/rseq/rseq-arm64.h
@@ -80,35 +80,37 @@ do { \
#define RSEQ_ASM_TMP_REG "x15"
#define RSEQ_ASM_TMP_REG_2 "x14"
-#define __RSEQ_ASM_DEFINE_TABLE(label, version, flags, start_ip, \
+#define __RSEQ_ASM_DEFINE_TABLE(version, flags, start_ip, \
post_commit_offset, abort_ip) \
- " .pushsection __rseq_table, \"aw\"\n" \
- " .balign 32\n" \
- __rseq_str(label) ":\n" \
" .long " __rseq_str(version) ", " __rseq_str(flags) "\n" \
" .quad " __rseq_str(start_ip) ", " \
__rseq_str(post_commit_offset) ", " \
- __rseq_str(abort_ip) "\n" \
- " .popsection\n"
+ __rseq_str(abort_ip) "\n"
-#define RSEQ_ASM_DEFINE_TABLE(label, start_ip, post_commit_ip, abort_ip) \
- __RSEQ_ASM_DEFINE_TABLE(label, 0x0, 0x0, start_ip, \
- (post_commit_ip - start_ip), abort_ip)
+#define RSEQ_ASM_DEFINE_TABLE(start_ip, post_commit_ip, abort_ip) \
+ " .pushsection __rseq_table, \"aw\"\n" \
+ " .balign 32\n" \
+ __RSEQ_ASM_DEFINE_TABLE(0x0, 0x0, start_ip, \
+ (post_commit_ip - start_ip), abort_ip) \
+ " .popsection\n"
-#define RSEQ_ASM_STORE_RSEQ_CS(label, cs_label, rseq_cs) \
+#define RSEQ_ASM_STORE_RSEQ_CS(label, table_label, rseq_cs) \
RSEQ_INJECT_ASM(1) \
- " adrp " RSEQ_ASM_TMP_REG ", " __rseq_str(cs_label) "\n" \
- " add " RSEQ_ASM_TMP_REG ", " RSEQ_ASM_TMP_REG \
- ", :lo12:" __rseq_str(cs_label) "\n" \
+ " adr " RSEQ_ASM_TMP_REG ", " __rseq_str(table_label) "\n" \
" str " RSEQ_ASM_TMP_REG ", %[" __rseq_str(rseq_cs) "]\n" \
__rseq_str(label) ":\n"
-#define RSEQ_ASM_DEFINE_ABORT(label, abort_label) \
- " .pushsection __rseq_failure, \"ax\"\n" \
- " .long " __rseq_str(RSEQ_SIG) "\n" \
+#define RSEQ_ASM_DEFINE_ABORT(table_label, start_ip, post_commit_ip, label, \
+ abort_label) \
+ " b 222f\n" \
+ " .balign 32\n" \
+ __rseq_str(table_label) ":\n" \
+ __RSEQ_ASM_DEFINE_TABLE(0x0, 0x0, start_ip, \
+ (post_commit_ip - start_ip), label ## f) \
+ " .inst " __rseq_str(RSEQ_SIG) "\n" \
__rseq_str(label) ":\n" \
" b %l[" __rseq_str(abort_label) "]\n" \
- " .popsection\n"
+ "222:\n"
#define RSEQ_ASM_OP_STORE(value, var) \
" str %[" __rseq_str(value) "], %[" __rseq_str(var) "]\n"
@@ -181,8 +183,8 @@ int rseq_cmpeqv_storev(intptr_t *v, intptr_t expect, intptr_t newv, int cpu)
RSEQ_INJECT_C(9)
__asm__ __volatile__ goto (
- RSEQ_ASM_DEFINE_TABLE(1, 2f, 3f, 4f)
- RSEQ_ASM_STORE_RSEQ_CS(2, 1b, rseq_cs)
+ RSEQ_ASM_DEFINE_TABLE(1f, 2f, 4f)
+ RSEQ_ASM_STORE_RSEQ_CS(1, 3f, rseq_cs)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
RSEQ_INJECT_ASM(3)
RSEQ_ASM_OP_CMPEQ(v, expect, %l[cmpfail])
@@ -191,9 +193,9 @@ int rseq_cmpeqv_storev(intptr_t *v, intptr_t expect, intptr_t newv, int cpu)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, %l[error1])
RSEQ_ASM_OP_CMPEQ(v, expect, %l[error2])
#endif
- RSEQ_ASM_OP_FINAL_STORE(newv, v, 3)
+ RSEQ_ASM_OP_FINAL_STORE(newv, v, 2)
RSEQ_INJECT_ASM(5)
- RSEQ_ASM_DEFINE_ABORT(4, abort)
+ RSEQ_ASM_DEFINE_ABORT(3, 1b, 2b, 4, abort)
: /* gcc asm goto does not allow outputs */
: [cpu_id] "r" (cpu),
[current_cpu_id] "Qo" (__rseq_abi.cpu_id),
@@ -230,8 +232,8 @@ int rseq_cmpnev_storeoffp_load(intptr_t *v, intptr_t expectnot,
RSEQ_INJECT_C(9)
__asm__ __volatile__ goto (
- RSEQ_ASM_DEFINE_TABLE(1, 2f, 3f, 4f)
- RSEQ_ASM_STORE_RSEQ_CS(2, 1b, rseq_cs)
+ RSEQ_ASM_DEFINE_TABLE(1f, 2f, 4f)
+ RSEQ_ASM_STORE_RSEQ_CS(1, 3f, rseq_cs)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
RSEQ_INJECT_ASM(3)
RSEQ_ASM_OP_CMPNE(v, expectnot, %l[cmpfail])
@@ -243,9 +245,9 @@ int rseq_cmpnev_storeoffp_load(intptr_t *v, intptr_t expectnot,
RSEQ_ASM_OP_R_LOAD(v)
RSEQ_ASM_OP_R_STORE(load)
RSEQ_ASM_OP_R_LOAD_OFF(voffp)
- RSEQ_ASM_OP_R_FINAL_STORE(v, 3)
+ RSEQ_ASM_OP_R_FINAL_STORE(v, 2)
RSEQ_INJECT_ASM(5)
- RSEQ_ASM_DEFINE_ABORT(4, abort)
+ RSEQ_ASM_DEFINE_ABORT(3, 1b, 2b, 4, abort)
: /* gcc asm goto does not allow outputs */
: [cpu_id] "r" (cpu),
[current_cpu_id] "Qo" (__rseq_abi.cpu_id),
@@ -281,8 +283,8 @@ int rseq_addv(intptr_t *v, intptr_t count, int cpu)
RSEQ_INJECT_C(9)
__asm__ __volatile__ goto (
- RSEQ_ASM_DEFINE_TABLE(1, 2f, 3f, 4f)
- RSEQ_ASM_STORE_RSEQ_CS(2, 1b, rseq_cs)
+ RSEQ_ASM_DEFINE_TABLE(1f, 2f, 4f)
+ RSEQ_ASM_STORE_RSEQ_CS(1, 3f, rseq_cs)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
RSEQ_INJECT_ASM(3)
#ifdef RSEQ_COMPARE_TWICE
@@ -290,9 +292,9 @@ int rseq_addv(intptr_t *v, intptr_t count, int cpu)
#endif
RSEQ_ASM_OP_R_LOAD(v)
RSEQ_ASM_OP_R_ADD(count)
- RSEQ_ASM_OP_R_FINAL_STORE(v, 3)
+ RSEQ_ASM_OP_R_FINAL_STORE(v, 2)
RSEQ_INJECT_ASM(4)
- RSEQ_ASM_DEFINE_ABORT(4, abort)
+ RSEQ_ASM_DEFINE_ABORT(3, 1b, 2b, 4, abort)
: /* gcc asm goto does not allow outputs */
: [cpu_id] "r" (cpu),
[current_cpu_id] "Qo" (__rseq_abi.cpu_id),
@@ -324,8 +326,8 @@ int rseq_cmpeqv_trystorev_storev(intptr_t *v, intptr_t expect,
RSEQ_INJECT_C(9)
__asm__ __volatile__ goto (
- RSEQ_ASM_DEFINE_TABLE(1, 2f, 3f, 4f)
- RSEQ_ASM_STORE_RSEQ_CS(2, 1b, rseq_cs)
+ RSEQ_ASM_DEFINE_TABLE(1f, 2f, 4f)
+ RSEQ_ASM_STORE_RSEQ_CS(1, 3f, rseq_cs)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
RSEQ_INJECT_ASM(3)
RSEQ_ASM_OP_CMPEQ(v, expect, %l[cmpfail])
@@ -336,9 +338,9 @@ int rseq_cmpeqv_trystorev_storev(intptr_t *v, intptr_t expect,
#endif
RSEQ_ASM_OP_STORE(newv2, v2)
RSEQ_INJECT_ASM(5)
- RSEQ_ASM_OP_FINAL_STORE(newv, v, 3)
+ RSEQ_ASM_OP_FINAL_STORE(newv, v, 2)
RSEQ_INJECT_ASM(6)
- RSEQ_ASM_DEFINE_ABORT(4, abort)
+ RSEQ_ASM_DEFINE_ABORT(3, 1b, 2b, 4, abort)
: /* gcc asm goto does not allow outputs */
: [cpu_id] "r" (cpu),
[current_cpu_id] "Qo" (__rseq_abi.cpu_id),
@@ -378,8 +380,8 @@ int rseq_cmpeqv_trystorev_storev_release(intptr_t *v, intptr_t expect,
RSEQ_INJECT_C(9)
__asm__ __volatile__ goto (
- RSEQ_ASM_DEFINE_TABLE(1, 2f, 3f, 4f)
- RSEQ_ASM_STORE_RSEQ_CS(2, 1b, rseq_cs)
+ RSEQ_ASM_DEFINE_TABLE(1f, 2f, 4f)
+ RSEQ_ASM_STORE_RSEQ_CS(1, 3f, rseq_cs)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
RSEQ_INJECT_ASM(3)
RSEQ_ASM_OP_CMPEQ(v, expect, %l[cmpfail])
@@ -390,9 +392,9 @@ int rseq_cmpeqv_trystorev_storev_release(intptr_t *v, intptr_t expect,
#endif
RSEQ_ASM_OP_STORE(newv2, v2)
RSEQ_INJECT_ASM(5)
- RSEQ_ASM_OP_FINAL_STORE_RELEASE(newv, v, 3)
+ RSEQ_ASM_OP_FINAL_STORE_RELEASE(newv, v, 2)
RSEQ_INJECT_ASM(6)
- RSEQ_ASM_DEFINE_ABORT(4, abort)
+ RSEQ_ASM_DEFINE_ABORT(3, 1b, 2b, 4, abort)
: /* gcc asm goto does not allow outputs */
: [cpu_id] "r" (cpu),
[current_cpu_id] "Qo" (__rseq_abi.cpu_id),
@@ -432,8 +434,8 @@ int rseq_cmpeqv_cmpeqv_storev(intptr_t *v, intptr_t expect,
RSEQ_INJECT_C(9)
__asm__ __volatile__ goto (
- RSEQ_ASM_DEFINE_TABLE(1, 2f, 3f, 4f)
- RSEQ_ASM_STORE_RSEQ_CS(2, 1b, rseq_cs)
+ RSEQ_ASM_DEFINE_TABLE(1f, 2f, 4f)
+ RSEQ_ASM_STORE_RSEQ_CS(1, 3f, rseq_cs)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
RSEQ_INJECT_ASM(3)
RSEQ_ASM_OP_CMPEQ(v, expect, %l[cmpfail])
@@ -445,9 +447,9 @@ int rseq_cmpeqv_cmpeqv_storev(intptr_t *v, intptr_t expect,
RSEQ_ASM_OP_CMPEQ(v, expect, %l[error2])
RSEQ_ASM_OP_CMPEQ(v2, expect2, %l[error3])
#endif
- RSEQ_ASM_OP_FINAL_STORE(newv, v, 3)
+ RSEQ_ASM_OP_FINAL_STORE(newv, v, 2)
RSEQ_INJECT_ASM(6)
- RSEQ_ASM_DEFINE_ABORT(4, abort)
+ RSEQ_ASM_DEFINE_ABORT(3, 1b, 2b, 4, abort)
: /* gcc asm goto does not allow outputs */
: [cpu_id] "r" (cpu),
[current_cpu_id] "Qo" (__rseq_abi.cpu_id),
@@ -489,8 +491,8 @@ int rseq_cmpeqv_trymemcpy_storev(intptr_t *v, intptr_t expect,
RSEQ_INJECT_C(9)
__asm__ __volatile__ goto (
- RSEQ_ASM_DEFINE_TABLE(1, 2f, 3f, 4f)
- RSEQ_ASM_STORE_RSEQ_CS(2, 1b, rseq_cs)
+ RSEQ_ASM_DEFINE_TABLE(1f, 2f, 4f)
+ RSEQ_ASM_STORE_RSEQ_CS(1, 3f, rseq_cs)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
RSEQ_INJECT_ASM(3)
RSEQ_ASM_OP_CMPEQ(v, expect, %l[cmpfail])
@@ -501,9 +503,9 @@ int rseq_cmpeqv_trymemcpy_storev(intptr_t *v, intptr_t expect,
#endif
RSEQ_ASM_OP_R_BAD_MEMCPY(dst, src, len)
RSEQ_INJECT_ASM(5)
- RSEQ_ASM_OP_FINAL_STORE(newv, v, 3)
+ RSEQ_ASM_OP_FINAL_STORE(newv, v, 2)
RSEQ_INJECT_ASM(6)
- RSEQ_ASM_DEFINE_ABORT(4, abort)
+ RSEQ_ASM_DEFINE_ABORT(3, 1b, 2b, 4, abort)
: /* gcc asm goto does not allow outputs */
: [cpu_id] "r" (cpu),
[current_cpu_id] "Qo" (__rseq_abi.cpu_id),
@@ -544,8 +546,8 @@ int rseq_cmpeqv_trymemcpy_storev_release(intptr_t *v, intptr_t expect,
RSEQ_INJECT_C(9)
__asm__ __volatile__ goto (
- RSEQ_ASM_DEFINE_TABLE(1, 2f, 3f, 4f)
- RSEQ_ASM_STORE_RSEQ_CS(2, 1b, rseq_cs)
+ RSEQ_ASM_DEFINE_TABLE(1f, 2f, 4f)
+ RSEQ_ASM_STORE_RSEQ_CS(1, 3f, rseq_cs)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
RSEQ_INJECT_ASM(3)
RSEQ_ASM_OP_CMPEQ(v, expect, %l[cmpfail])
@@ -556,9 +558,9 @@ int rseq_cmpeqv_trymemcpy_storev_release(intptr_t *v, intptr_t expect,
#endif
RSEQ_ASM_OP_R_BAD_MEMCPY(dst, src, len)
RSEQ_INJECT_ASM(5)
- RSEQ_ASM_OP_FINAL_STORE_RELEASE(newv, v, 3)
+ RSEQ_ASM_OP_FINAL_STORE_RELEASE(newv, v, 2)
RSEQ_INJECT_ASM(6)
- RSEQ_ASM_DEFINE_ABORT(4, abort)
+ RSEQ_ASM_DEFINE_ABORT(3, 1b, 2b, 4, abort)
: /* gcc asm goto does not allow outputs */
: [cpu_id] "r" (cpu),
[current_cpu_id] "Qo" (__rseq_abi.cpu_id),