| Message ID | 6bac7035adc2cfc8ab3800fe1d2d03223ec57ff5.1663662348.git.jstancek@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | syscalls/futex_waitv02: replace TST_THREAD_STATE_WAIT with repeated wake | expand |
Hi! On 9/20/22 10:28, Jan Stancek wrote: > Same patch as for futex_waitv03. TST_THREAD_STATE_WAIT isn't reliable to > tell that it's safe to call futex_wake(). futex_wake() can be called > prematurely and return 0, which leaves other thread timing out. > > Replace it with repeated futex_wake() until it fails or wakes at least 1 waiter. > Also extend timeout to 5 seconds to avoid false positives from systems with > high steal time (e.g. overloaded s390x host). > > Signed-off-by: Jan Stancek <jstancek@redhat.com> > --- > .../kernel/syscalls/futex/futex_waitv02.c | 26 +++++++++---------- > 1 file changed, 12 insertions(+), 14 deletions(-) > > diff --git a/testcases/kernel/syscalls/futex/futex_waitv02.c b/testcases/kernel/syscalls/futex/futex_waitv02.c > index 0a0e2b62075c..00cf0960808a 100644 > --- a/testcases/kernel/syscalls/futex/futex_waitv02.c > +++ b/testcases/kernel/syscalls/futex/futex_waitv02.c > @@ -50,19 +50,20 @@ static void setup(void) > } > } > > -static void *threaded(void *arg) > +static void *threaded(LTP_ATTRIBUTE_UNUSED void *arg) > { > struct futex_test_variants tv = futex_variant(); > - int tid = *(int *)arg; > > - TST_THREAD_STATE_WAIT(tid, 'S', 0); > - > - TEST(futex_wake(tv.fntype, (void *)(uintptr_t)waitv[numfutex - 1].uaddr, > + do { > + TEST(futex_wake(tv.fntype, > + (void *)(uintptr_t)waitv[numfutex - 1].uaddr, > 1, FUTEX_PRIVATE_FLAG)); > - if (TST_RET < 0) { > - tst_brk(TBROK | TTERRNO, > - "futex_wake private returned: %ld", TST_RET); > - } > + if (TST_RET < 0) { > + tst_brk(TBROK | TTERRNO, > + "futex_wake private returned: %ld", TST_RET); > + } > + usleep(1000); > + } while (TST_RET < 1); > Why not using TST_RETRY_FUNC macro instead? TST_RETRY_FUNC(futex_wake(tv.fntype, (void *)(uintptr_t)waitv[numfutex - 1].uaddr, 1, FUTEX_PRIVATE_FLAG), TST_RETVAL_GE0); > return NULL; > } > @@ -70,16 +71,13 @@ static void *threaded(void *arg) > static void run(void) > { > struct timespec to; > - int tid; > pthread_t t; > > - tid = tst_syscall(__NR_gettid); > - > - SAFE_PTHREAD_CREATE(&t, NULL, threaded, (void *)&tid); > + SAFE_PTHREAD_CREATE(&t, NULL, threaded, NULL); > > /* setting absolute timeout for futex2 */ > SAFE_CLOCK_GETTIME(CLOCK_MONOTONIC, &to); > - to.tv_sec++; > + to.tv_sec += 5; > > TEST(futex_waitv(waitv, numfutex, 0, &to, CLOCK_MONOTONIC)); > if (TST_RET < 0) { -- Regards, Andrea Cervesato
diff --git a/testcases/kernel/syscalls/futex/futex_waitv02.c b/testcases/kernel/syscalls/futex/futex_waitv02.c index 0a0e2b62075c..00cf0960808a 100644 --- a/testcases/kernel/syscalls/futex/futex_waitv02.c +++ b/testcases/kernel/syscalls/futex/futex_waitv02.c @@ -50,19 +50,20 @@ static void setup(void) } } -static void *threaded(void *arg) +static void *threaded(LTP_ATTRIBUTE_UNUSED void *arg) { struct futex_test_variants tv = futex_variant(); - int tid = *(int *)arg; - TST_THREAD_STATE_WAIT(tid, 'S', 0); - - TEST(futex_wake(tv.fntype, (void *)(uintptr_t)waitv[numfutex - 1].uaddr, + do { + TEST(futex_wake(tv.fntype, + (void *)(uintptr_t)waitv[numfutex - 1].uaddr, 1, FUTEX_PRIVATE_FLAG)); - if (TST_RET < 0) { - tst_brk(TBROK | TTERRNO, - "futex_wake private returned: %ld", TST_RET); - } + if (TST_RET < 0) { + tst_brk(TBROK | TTERRNO, + "futex_wake private returned: %ld", TST_RET); + } + usleep(1000); + } while (TST_RET < 1); return NULL; } @@ -70,16 +71,13 @@ static void *threaded(void *arg) static void run(void) { struct timespec to; - int tid; pthread_t t; - tid = tst_syscall(__NR_gettid); - - SAFE_PTHREAD_CREATE(&t, NULL, threaded, (void *)&tid); + SAFE_PTHREAD_CREATE(&t, NULL, threaded, NULL); /* setting absolute timeout for futex2 */ SAFE_CLOCK_GETTIME(CLOCK_MONOTONIC, &to); - to.tv_sec++; + to.tv_sec += 5; TEST(futex_waitv(waitv, numfutex, 0, &to, CLOCK_MONOTONIC)); if (TST_RET < 0) {
Same patch as for futex_waitv03. TST_THREAD_STATE_WAIT isn't reliable to tell that it's safe to call futex_wake(). futex_wake() can be called prematurely and return 0, which leaves other thread timing out. Replace it with repeated futex_wake() until it fails or wakes at least 1 waiter. Also extend timeout to 5 seconds to avoid false positives from systems with high steal time (e.g. overloaded s390x host). Signed-off-by: Jan Stancek <jstancek@redhat.com> --- .../kernel/syscalls/futex/futex_waitv02.c | 26 +++++++++---------- 1 file changed, 12 insertions(+), 14 deletions(-)