On Thu, Aug 6, 2020 at 5:27 PM Boqun Feng <boqun.f...@gmail.com> wrote: > > 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?
rseq_cmpeqv_cmpeqv_store is used below to make sure the reference is the one that should be used; if it is no longer "active", the iteration is restarted. > > 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 > >