On Thu, Aug 06, 2020 at 10:05:44AM -0700, Peter Oskolkov wrote:
> Based on Google-internal RSEQ work done by
> Paul Turner and Andrew Hunter.
>
> This patch adds a selftest for MEMBARRIER_CMD_PRIVATE_RESTART_RSEQ_ON_CPU.
> The test quite often fails without the previous patch in this patchset,
> but consistently passes with it.
>
> Signed-off-by: Peter Oskolkov <p...@google.com>
> ---
> .../selftests/rseq/basic_percpu_ops_test.c | 181 ++++++++++++++++++
> 1 file changed, 181 insertions(+)
>
> diff --git a/tools/testing/selftests/rseq/basic_percpu_ops_test.c
> b/tools/testing/selftests/rseq/basic_percpu_ops_test.c
> index eb3f6db36d36..147c80deac19 100644
> --- a/tools/testing/selftests/rseq/basic_percpu_ops_test.c
> +++ b/tools/testing/selftests/rseq/basic_percpu_ops_test.c
> @@ -3,16 +3,21 @@
> #include <assert.h>
> #include <pthread.h>
> #include <sched.h>
> +#include <stdatomic.h>
> #include <stdint.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> #include <stddef.h>
> +#include <syscall.h>
> +#include <unistd.h>
>
> #include "rseq.h"
>
> #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
>
> +#define MEMBARRIER_CMD_PRIVATE_RESTART_RSEQ_ON_CPU (1<<7)
> +
> struct percpu_lock_entry {
> intptr_t v;
> } __attribute__((aligned(128)));
> @@ -289,6 +294,180 @@ void test_percpu_list(void)
> assert(sum == expected_sum);
> }
>
> +struct test_membarrier_thread_args {
> + int stop;
> + intptr_t percpu_list_ptr;
> +};
> +
> +/* Worker threads modify data in their "active" percpu lists. */
> +void *test_membarrier_worker_thread(void *arg)
> +{
> + struct test_membarrier_thread_args *args =
> + (struct test_membarrier_thread_args *)arg;
> + const int iters = 10 * 1000 * 1000;
> + int i;
> +
> + if (rseq_register_current_thread()) {
> + fprintf(stderr, "Error: rseq_register_current_thread(...)
> failed(%d): %s\n",
> + errno, strerror(errno));
> + abort();
> + }
> +
> + for (i = 0; i < iters; ++i) {
> + while (true) {
> + int cpu, ret;
> + struct percpu_list *list_ptr = (struct percpu_list *)
> + atomic_load(&args->percpu_list_ptr);
> +
What if the manager thread update ->percpu_list_ptr and call
membarrier() here? I.e.
CPU0 CPU1
list_ptr = atomic_load(&args->percpu_list_ptr);
// read list_b
atomic_store(&args->percpu_list_ptr, list_a);
sys_membarrier(MEMBARRIER_CMD_PRIVATE_RESTART_RSEQ_ON_CPU, 1); // send
ipi to restart rseq.cs on CPU1
<got IPI, but not in a rseq.cs, so nothing to
do>
cpu = rseq_cpu_start(); // start a rseq.cs and
accessing list_b!
The thing is, atomic_load() is an reference to ->percpu_list_ptr, which
is outside the rseq.cs, simply restarting rseq doesn't kill this
reference.
Am I missing something subtle?
Regards,
Boqun
> + if (!list_ptr)
> + continue; /* Not yet initialized. */
> +
> + cpu = rseq_cpu_start();
> + struct percpu_list_node *node = list_ptr->c[cpu].head;
> + const intptr_t prev = node->data;
> +
> + ret = rseq_cmpeqv_cmpeqv_storev(&node->data, prev,
> + &args->percpu_list_ptr,
> + (intptr_t)list_ptr, prev + 1, cpu);
> + if (!ret)
> + break; /* Success. */
> + }
> + }
> +
> + if (rseq_unregister_current_thread()) {
> + fprintf(stderr, "Error: rseq_unregister_current_thread(...)
> failed(%d): %s\n",
> + errno, strerror(errno));
> + abort();
> + }
> + return NULL;
> +}
> +
> +void test_membarrier_init_percpu_list(struct percpu_list *list)
> +{
> + int i;
> +
> + memset(list, 0, sizeof(*list));
> + for (i = 0; i < CPU_SETSIZE; i++) {
> + struct percpu_list_node *node;
> +
> + node = malloc(sizeof(*node));
> + assert(node);
> + node->data = 0;
> + node->next = NULL;
> + list->c[i].head = node;
> + }
> +}
> +
> +void test_membarrier_free_percpu_list(struct percpu_list *list)
> +{
> + int i;
> +
> + for (i = 0; i < CPU_SETSIZE; i++)
> + free(list->c[i].head);
> +}
> +
> +static int sys_membarrier(int cmd, int flags)
> +{
> + return syscall(__NR_membarrier, cmd, flags);
> +}
> +
> +/*
> + * The manager thread swaps per-cpu lists that worker threads see,
> + * and validates that there are no unexpected modifications.
> + */
> +void *test_membarrier_manager_thread(void *arg)
> +{
> + struct test_membarrier_thread_args *args =
> + (struct test_membarrier_thread_args *)arg;
> + struct percpu_list list_a, list_b;
> + intptr_t expect_a = 0, expect_b = 0;
> + int cpu_a = 0, cpu_b = 0;
> +
> + if (rseq_register_current_thread()) {
> + fprintf(stderr, "Error: rseq_register_current_thread(...)
> failed(%d): %s\n",
> + errno, strerror(errno));
> + abort();
> + }
> +
> + /* Init lists. */
> + test_membarrier_init_percpu_list(&list_a);
> + test_membarrier_init_percpu_list(&list_b);
> +
> + atomic_store(&args->percpu_list_ptr, (intptr_t)&list_a);
> +
> + while (!atomic_load(&args->stop)) {
> + /* list_a is "active". */
> + cpu_a = rand() % CPU_SETSIZE;
> + /*
> + * As list_b is "inactive", we should never see changes
> + * to list_b.
> + */
> + if (expect_b != atomic_load(&list_b.c[cpu_b].head->data)) {
> + fprintf(stderr, "Membarrier test failed\n");
> + abort();
> + }
> +
> + /* Make list_b "active". */
> + atomic_store(&args->percpu_list_ptr, (intptr_t)&list_b);
> + sys_membarrier(MEMBARRIER_CMD_PRIVATE_RESTART_RSEQ_ON_CPU,
> cpu_a);
> + /*
> + * Cpu A should now only modify list_b, so we values
> + * in list_a should be stable.
> + */
> + expect_a = atomic_load(&list_a.c[cpu_a].head->data);
> +
> + cpu_b = rand() % CPU_SETSIZE;
> + /*
> + * As list_a is "inactive", we should never see changes
> + * to list_a.
> + */
> + if (expect_a != atomic_load(&list_a.c[cpu_a].head->data)) {
> + fprintf(stderr, "Membarrier test failed\n");
> + abort();
> + }
> +
> + /* Make list_a "active". */
> + atomic_store(&args->percpu_list_ptr, (intptr_t)&list_a);
> + sys_membarrier(MEMBARRIER_CMD_PRIVATE_RESTART_RSEQ_ON_CPU,
> cpu_b);
> + /* Remember a value from list_b. */
> + expect_b = atomic_load(&list_b.c[cpu_b].head->data);
> + }
> +
> + test_membarrier_free_percpu_list(&list_a);
> + test_membarrier_free_percpu_list(&list_b);
> +
> + if (rseq_unregister_current_thread()) {
> + fprintf(stderr, "Error: rseq_unregister_current_thread(...)
> failed(%d): %s\n",
> + errno, strerror(errno));
> + abort();
> + }
> + return NULL;
> +}
> +
> +/* Test MEMBARRIER_CMD_PRIVATE_RESTART_RSEQ_ON_CPU membarrier command. */
> +void test_membarrier(void)
> +{
> + struct test_membarrier_thread_args thread_args;
> + pthread_t worker_threads[CPU_SETSIZE];
> + pthread_t manager_thread;
> + int i;
> +
> + thread_args.stop = 0;
> + thread_args.percpu_list_ptr = 0;
> + pthread_create(&manager_thread, NULL,
> + test_membarrier_manager_thread, &thread_args);
> +
> + for (i = 0; i < CPU_SETSIZE; i++)
> + pthread_create(&worker_threads[i], NULL,
> + test_membarrier_worker_thread, &thread_args);
> +
> + for (i = 0; i < CPU_SETSIZE; i++)
> + pthread_join(worker_threads[i], NULL);
> +
> + atomic_store(&thread_args.stop, 1);
> + pthread_join(manager_thread, NULL);
> +}
> +
> int main(int argc, char **argv)
> {
> if (rseq_register_current_thread()) {
> @@ -300,6 +479,8 @@ int main(int argc, char **argv)
> test_percpu_spinlock();
> printf("percpu_list\n");
> test_percpu_list();
> + printf("membarrier\n");
> + test_membarrier();
> if (rseq_unregister_current_thread()) {
> fprintf(stderr, "Error: rseq_unregister_current_thread(...)
> failed(%d): %s\n",
> errno, strerror(errno));
> --
> 2.28.0.163.g6104cc2f0b6-goog
>
signature.asc
Description: PGP signature
