[LTP] [PATCH v2] syscalls/futex_waitv0[23]: replace TST_THREAD_STATE_WAIT with repeated wake

[Andrea Cervesato]

Hi!

On 9/20/22 11:21, Jan Stancek wrote:
> 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).
>
> For futex_waitv03 this replaces while loop with TST_RETRY_FUNC.
>
> Signed-off-by: Jan Stancek <jstancek@redhat.com>
> ---
>   .../kernel/syscalls/futex/futex_waitv02.c     | 21 ++++++-------------
>   .../kernel/syscalls/futex/futex_waitv03.c     | 12 +++--------
>   testcases/kernel/syscalls/futex/futextest.h   | 15 +++++++++++++
>   3 files changed, 24 insertions(+), 24 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/futex/futex_waitv02.c b/testcases/kernel/syscalls/futex/futex_waitv02.c
> index 0a0e2b62075c..ccea5eb5e745 100644
> --- a/testcases/kernel/syscalls/futex/futex_waitv02.c
> +++ b/testcases/kernel/syscalls/futex/futex_waitv02.c
> @@ -50,19 +50,13 @@ 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,
> -			1, FUTEX_PRIVATE_FLAG));
> -	if (TST_RET < 0) {
> -		tst_brk(TBROK | TTERRNO,
> -			"futex_wake private returned: %ld", TST_RET);
> -	}
> +	TST_RETRY_FUNC(futex_wake(tv.fntype,
> +		(void *)(uintptr_t)waitv[numfutex - 1].uaddr,
> +		1, FUTEX_PRIVATE_FLAG), futex_waked_someone);

Correct way of using TST_RETRY_FUNC is the following:

     ret = TST_RETRY_FUNC(futex_wake(tv.fntype, (void 
*)(uintptr_t)waitv[numfutex - 1].uaddr, 1, FUTEX_PRIVATE_FLAG), 
TST_RETVAL_GE0);

     if (ret < 0)
         tst_brk(TBROK | TTERRNO, "futex_wake private returned: %ld", ret);

>   
>   	return NULL;
>   }
> @@ -70,16 +64,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) {
> diff --git a/testcases/kernel/syscalls/futex/futex_waitv03.c b/testcases/kernel/syscalls/futex/futex_waitv03.c
> index ee79728474ee..c674f52d8d4c 100644
> --- a/testcases/kernel/syscalls/futex/futex_waitv03.c
> +++ b/testcases/kernel/syscalls/futex/futex_waitv03.c
> @@ -74,15 +74,9 @@ static void *threaded(LTP_ATTRIBUTE_UNUSED void *arg)
>   {
>   	struct futex_test_variants tv = futex_variant();
>   
> -	do {
> -		TEST(futex_wake(tv.fntype, (void *)(uintptr_t)waitv[numfutex - 1].uaddr,
> -			1, 0));
> -		if (TST_RET < 0) {
> -			tst_brk(TBROK | TTERRNO,
> -				"futex_wake private returned: %ld", TST_RET);
> -		}
> -		usleep(1000);
> -	} while (TST_RET < 1);
> +	TST_RETRY_FUNC(futex_wake(tv.fntype,
> +		(void *)(uintptr_t)waitv[numfutex - 1].uaddr,
> +		1, 0), futex_waked_someone);
>   
>   	return NULL;
>   }
> diff --git a/testcases/kernel/syscalls/futex/futextest.h b/testcases/kernel/syscalls/futex/futextest.h
> index fd10885f3205..515b5102d4fc 100644
> --- a/testcases/kernel/syscalls/futex/futextest.h
> +++ b/testcases/kernel/syscalls/futex/futextest.h
> @@ -277,4 +277,19 @@ futex_set(futex_t *uaddr, u_int32_t newval)
>   	return newval;
>   }
>   
> +/**
> + * futex_waked_someone() - ECHCK func for TST_RETRY_FUNC
> + * @ret:	return value of futex_wake
> + *
> + * Return value drives TST_RETRY_FUNC macro.
> + */
> +static inline int
> +futex_waked_someone(int ret)
> +{
> +	if (ret < 0)
> +		tst_brk(TBROK | TERRNO, "futex_wake returned: %d", ret);
> +
> +	return ret;
> +}
> +
>   #endif /* _FUTEXTEST_H */

--

Regards,
Andrea Cervesato