The enable_event and modify_and_enable_event tests fail when compiled
with -O2:
Expected ctx.signal_count (5) == NUM_THREADS + 1 (6)
The test sequence is:
EXPECT_EQ(ctx.signal_count, NUM_THREADS); /* compiler loads into reg */
ctx.iterate_on = 0; /* triggers SIGTRAP, handler
increments signal_count */
EXPECT_EQ(ctx.signal_count, NUM_THREADS + 1); /* stale cached value */
The signal handler already uses __atomic_fetch_add/__atomic_fetch_sub for
signal_count and tids_want_signal, but the test body reads them as plain
variables. At -O2 the compiler is free to cache the value across the
SIGTRAP-triggering store to iterate_on, so the second EXPECT_EQ sees
a stale register value.
Fix by using __atomic_load_n() for every read of signal_count and
tids_want_signal in test assertions.
Signed-off-by: Eva Kurchatova <[email protected]>
Signed-off-by: Konstantin Khorenko <[email protected]>
---
.../selftests/perf_events/sigtrap_threads.c | 20 +++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/tools/testing/selftests/perf_events/sigtrap_threads.c
b/tools/testing/selftests/perf_events/sigtrap_threads.c
index b5cf8355345d..6b246e0ffde9 100644
--- a/tools/testing/selftests/perf_events/sigtrap_threads.c
+++ b/tools/testing/selftests/perf_events/sigtrap_threads.c
@@ -159,8 +159,8 @@ static void run_test_threads(struct __test_metadata
*_metadata,
TEST_F(sigtrap_threads, remain_disabled)
{
run_test_threads(_metadata, self);
- EXPECT_EQ(ctx.signal_count, 0);
- EXPECT_NE(ctx.tids_want_signal, 0);
+ EXPECT_EQ(__atomic_load_n(&ctx.signal_count, __ATOMIC_RELAXED), 0);
+ EXPECT_NE(__atomic_load_n(&ctx.tids_want_signal, __ATOMIC_RELAXED), 0);
}
TEST_F(sigtrap_threads, enable_event)
@@ -168,15 +168,15 @@ TEST_F(sigtrap_threads, enable_event)
EXPECT_EQ(ioctl(self->fd, PERF_EVENT_IOC_ENABLE, 0), 0);
run_test_threads(_metadata, self);
- EXPECT_EQ(ctx.signal_count, NUM_THREADS);
- EXPECT_EQ(ctx.tids_want_signal, 0);
+ EXPECT_EQ(__atomic_load_n(&ctx.signal_count, __ATOMIC_RELAXED),
NUM_THREADS);
+ EXPECT_EQ(__atomic_load_n(&ctx.tids_want_signal, __ATOMIC_RELAXED), 0);
EXPECT_EQ(ctx.first_siginfo.si_addr, &ctx.iterate_on);
EXPECT_EQ(ctx.first_siginfo.si_perf_type, PERF_TYPE_BREAKPOINT);
EXPECT_EQ(ctx.first_siginfo.si_perf_data,
TEST_SIG_DATA(&ctx.iterate_on, 0));
/* Check enabled for parent. */
ctx.iterate_on = 0;
- EXPECT_EQ(ctx.signal_count, NUM_THREADS + 1);
+ EXPECT_EQ(__atomic_load_n(&ctx.signal_count, __ATOMIC_RELAXED),
NUM_THREADS + 1);
}
/* Test that modification propagates to all inherited events. */
@@ -187,15 +187,15 @@ TEST_F(sigtrap_threads, modify_and_enable_event)
EXPECT_EQ(ioctl(self->fd, PERF_EVENT_IOC_MODIFY_ATTRIBUTES, &new_attr),
0);
run_test_threads(_metadata, self);
- EXPECT_EQ(ctx.signal_count, NUM_THREADS);
- EXPECT_EQ(ctx.tids_want_signal, 0);
+ EXPECT_EQ(__atomic_load_n(&ctx.signal_count, __ATOMIC_RELAXED),
NUM_THREADS);
+ EXPECT_EQ(__atomic_load_n(&ctx.tids_want_signal, __ATOMIC_RELAXED), 0);
EXPECT_EQ(ctx.first_siginfo.si_addr, &ctx.iterate_on);
EXPECT_EQ(ctx.first_siginfo.si_perf_type, PERF_TYPE_BREAKPOINT);
EXPECT_EQ(ctx.first_siginfo.si_perf_data,
TEST_SIG_DATA(&ctx.iterate_on, 42));
/* Check enabled for parent. */
ctx.iterate_on = 0;
- EXPECT_EQ(ctx.signal_count, NUM_THREADS + 1);
+ EXPECT_EQ(__atomic_load_n(&ctx.signal_count, __ATOMIC_RELAXED),
NUM_THREADS + 1);
}
/* Stress test event + signal handling. */
@@ -207,8 +207,8 @@ TEST_F(sigtrap_threads, signal_stress)
run_test_threads(_metadata, self);
EXPECT_EQ(ioctl(self->fd, PERF_EVENT_IOC_DISABLE, 0), 0);
- EXPECT_EQ(ctx.signal_count, NUM_THREADS * ctx.iterate_on);
- EXPECT_EQ(ctx.tids_want_signal, 0);
+ EXPECT_EQ(__atomic_load_n(&ctx.signal_count, __ATOMIC_RELAXED),
NUM_THREADS * ctx.iterate_on);
+ EXPECT_EQ(__atomic_load_n(&ctx.tids_want_signal, __ATOMIC_RELAXED), 0);
EXPECT_EQ(ctx.first_siginfo.si_addr, &ctx.iterate_on);
EXPECT_EQ(ctx.first_siginfo.si_perf_type, PERF_TYPE_BREAKPOINT);
EXPECT_EQ(ctx.first_siginfo.si_perf_data,
TEST_SIG_DATA(&ctx.iterate_on, 0));
--
2.54.0
