| Message ID | 20240619002204.2492673-1-edliaw@google.com |
|---|---|
| State | New |
| Headers | show |
| Series | selftests/futex: Order calls in futex_requeue | expand |
On 6/19/24 5:22 AM, Edward Liaw wrote: > Like fbf4dec70277 ("selftests/futex: Order calls to futex_lock_pi"), > which fixed a flake in futex_lock_pi due to racing between the parent > and child threads. > > The same issue can occur in the futex_requeue test, because it expects > waiterfn to make progress to futex_wait before the parent starts to > requeue. This is mitigated by the parent sleeping for WAKE_WAIT_US, but > it still fails occasionally. This can be reproduced by adding a sleep in > the waiterfn before futex_wait: > > TAP version 13 > 1..2 > not ok 1 futex_requeue simple returned: 0 > not ok 2 futex_requeue simple returned: 0 > not ok 3 futex_requeue many returned: 0 > not ok 4 futex_requeue many returned: 0 > > Instead, replace the sleep with barriers to make the sequencing > explicit. > > Fixes: 7cb5dd8e2c8c ("selftests: futex: Add futex compare requeue test") > Signed-off-by: Edward Liaw <edliaw@google.com> Reviewed-by: Muhammad Usama Anjum <usama.anjum@collabora.com> > --- > .../selftests/futex/functional/futex_requeue.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/tools/testing/selftests/futex/functional/futex_requeue.c b/tools/testing/selftests/futex/functional/futex_requeue.c > index 51485be6eb2f..8f7d3e8bf32a 100644 > --- a/tools/testing/selftests/futex/functional/futex_requeue.c > +++ b/tools/testing/selftests/futex/functional/futex_requeue.c > @@ -12,9 +12,9 @@ > > #define TEST_NAME "futex-requeue" > #define timeout_ns 30000000 > -#define WAKE_WAIT_US 10000 > > volatile futex_t *f1; > +static pthread_barrier_t barrier; > > void usage(char *prog) > { > @@ -32,6 +32,8 @@ void *waiterfn(void *arg) > to.tv_sec = 0; > to.tv_nsec = timeout_ns; > > + pthread_barrier_wait(&barrier); > + > if (futex_wait(f1, *f1, &to, 0)) > printf("waiter failed errno %d\n", errno); > > @@ -70,13 +72,15 @@ int main(int argc, char *argv[]) > ksft_print_msg("%s: Test futex_requeue\n", > basename(argv[0])); > > + pthread_barrier_init(&barrier, NULL, 2); > /* > * Requeue a waiter from f1 to f2, and wake f2. > */ > if (pthread_create(&waiter[0], NULL, waiterfn, NULL)) > error("pthread_create failed\n", errno); > > - usleep(WAKE_WAIT_US); > + pthread_barrier_wait(&barrier); > + pthread_barrier_destroy(&barrier); > > info("Requeuing 1 futex from f1 to f2\n"); > res = futex_cmp_requeue(f1, 0, &f2, 0, 1, 0); > @@ -99,6 +103,7 @@ int main(int argc, char *argv[]) > ksft_test_result_pass("futex_requeue simple succeeds\n"); > } > > + pthread_barrier_init(&barrier, NULL, 11); > > /* > * Create 10 waiters at f1. At futex_requeue, wake 3 and requeue 7. > @@ -109,7 +114,8 @@ int main(int argc, char *argv[]) > error("pthread_create failed\n", errno); > } > > - usleep(WAKE_WAIT_US); > + pthread_barrier_wait(&barrier); > + pthread_barrier_destroy(&barrier); > > info("Waking 3 futexes at f1 and requeuing 7 futexes from f1 to f2\n"); > res = futex_cmp_requeue(f1, 0, &f2, 3, 7, 0);
diff --git a/tools/testing/selftests/futex/functional/futex_requeue.c b/tools/testing/selftests/futex/functional/futex_requeue.c index 51485be6eb2f..8f7d3e8bf32a 100644 --- a/tools/testing/selftests/futex/functional/futex_requeue.c +++ b/tools/testing/selftests/futex/functional/futex_requeue.c @@ -12,9 +12,9 @@ #define TEST_NAME "futex-requeue" #define timeout_ns 30000000 -#define WAKE_WAIT_US 10000 volatile futex_t *f1; +static pthread_barrier_t barrier; void usage(char *prog) { @@ -32,6 +32,8 @@ void *waiterfn(void *arg) to.tv_sec = 0; to.tv_nsec = timeout_ns; + pthread_barrier_wait(&barrier); + if (futex_wait(f1, *f1, &to, 0)) printf("waiter failed errno %d\n", errno); @@ -70,13 +72,15 @@ int main(int argc, char *argv[]) ksft_print_msg("%s: Test futex_requeue\n", basename(argv[0])); + pthread_barrier_init(&barrier, NULL, 2); /* * Requeue a waiter from f1 to f2, and wake f2. */ if (pthread_create(&waiter[0], NULL, waiterfn, NULL)) error("pthread_create failed\n", errno); - usleep(WAKE_WAIT_US); + pthread_barrier_wait(&barrier); + pthread_barrier_destroy(&barrier); info("Requeuing 1 futex from f1 to f2\n"); res = futex_cmp_requeue(f1, 0, &f2, 0, 1, 0); @@ -99,6 +103,7 @@ int main(int argc, char *argv[]) ksft_test_result_pass("futex_requeue simple succeeds\n"); } + pthread_barrier_init(&barrier, NULL, 11); /* * Create 10 waiters at f1. At futex_requeue, wake 3 and requeue 7. @@ -109,7 +114,8 @@ int main(int argc, char *argv[]) error("pthread_create failed\n", errno); } - usleep(WAKE_WAIT_US); + pthread_barrier_wait(&barrier); + pthread_barrier_destroy(&barrier); info("Waking 3 futexes at f1 and requeuing 7 futexes from f1 to f2\n"); res = futex_cmp_requeue(f1, 0, &f2, 3, 7, 0);
Like fbf4dec70277 ("selftests/futex: Order calls to futex_lock_pi"), which fixed a flake in futex_lock_pi due to racing between the parent and child threads. The same issue can occur in the futex_requeue test, because it expects waiterfn to make progress to futex_wait before the parent starts to requeue. This is mitigated by the parent sleeping for WAKE_WAIT_US, but it still fails occasionally. This can be reproduced by adding a sleep in the waiterfn before futex_wait: TAP version 13 1..2 not ok 1 futex_requeue simple returned: 0 not ok 2 futex_requeue simple returned: 0 not ok 3 futex_requeue many returned: 0 not ok 4 futex_requeue many returned: 0 Instead, replace the sleep with barriers to make the sequencing explicit. Fixes: 7cb5dd8e2c8c ("selftests: futex: Add futex compare requeue test") Signed-off-by: Edward Liaw <edliaw@google.com> --- .../selftests/futex/functional/futex_requeue.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)