Re: [RESEND PATCH v3] selftests/futex: fix the failed futex_requeue test issue

[Sebastian Andrzej Siewior <bigeasy@linutronix.de>]

On 2026-04-27 18:18:38 [+0800], Yuwen Chen wrote:
> This test item has extremely high requirements for timing and can only
> pass the test under specific conditions. The following situations will
> lead to test failure:
> 
>     MainThread                  Thread1
>         │
>   pthread_create-------------------┐
>         │                          │
>  futex_cmp_requeue                 │
>         │                     futex_wait
>         │                          │
> 
> If the child thread is not waiting in the futex_wait function when the
> main thread reaches the futex_cmp_requeue function, the test will fail.
> 
> This patch avoids this problem by checking whether the child thread is

This patch.

> in a sleeping state in the main thread.

The pthread_barrier you introduced is not enough?
You also increase the wait-timeout to 3s and parse the wait function.
You could describe what the expectation here is. I would assume that the
barrier on its own should be enough. But the barrier and the state
parsing is a bit much. This makes me curious what is so special on your
side that it does not work as-is. Is this some big-little case where the
big creates the thread and does requeue while the littke is supposed to
do futex_wait but falls behind?

> Fixes: 7cb5dd8e2c8c ("selftests: futex: Add futex compare requeue test")
> Signed-off-by: Yuwen Chen <ywen.chen@foxmail.com>
> Co-developed-by: Edward Liaw <edliaw@google.com>
> Signed-off-by: Edward Liaw <edliaw@google.com>
> ---
> v1->v2:
>     1. Fix the issue of abnormal use of fscanf in the get_thread_state function
>     2. Add timeout logic
> 
> v2->v3: https://lore.kernel.org/all/tencent_B9DBF2ECBE56BAB68BDAB949C6935D01CE09@qq.com/
>     1. Use /proc/[pid]/wchan instead of /proc/[pid]/stat to check if a process has entered the sleep state
>     2. Refactor part of the logic to facilitate code reuse.
> 
>  .../futex/functional/futex_requeue.c          | 91 ++++++++++++++++---
>  1 file changed, 77 insertions(+), 14 deletions(-)
> 
> diff --git a/tools/testing/selftests/futex/functional/futex_requeue.c b/tools/testing/selftests/futex/functional/futex_requeue.c
> index dcf0d5f2f3122..7163e827691e4 100644
> --- a/tools/testing/selftests/futex/functional/futex_requeue.c
> +++ b/tools/testing/selftests/futex/functional/futex_requeue.c
> @@ -11,8 +11,9 @@
>  #include "futextest.h"
>  #include "kselftest_harness.h"
>  
> -#define timeout_ns  30000000
> -#define WAKE_WAIT_US 10000
> +#define FUTEX_WAIT_TIMEOUT_S	3 /* 3s */
> +#define WAIT_THREAD_RETRIES	100
> +#define WAIT_THREAD_DELAY_US	(10000 * 100) /* 1s */
>  
>  volatile futex_t *f1;
>  
> @@ -20,8 +21,8 @@ void *waiterfn(void *arg)
>  {
>  	struct timespec to;
>  
> -	to.tv_sec = 0;
> -	to.tv_nsec = timeout_ns;
> +	to.tv_sec = FUTEX_WAIT_TIMEOUT_S;
> +	to.tv_nsec = 0;
>  
>  	if (futex_wait(f1, *f1, &to, 0))
>  		printf("waiter failed errno %d\n", errno);
> @@ -29,20 +30,83 @@ void *waiterfn(void *arg)
>  	return NULL;
>  }
>  
> +struct futex_thread {
> +	pthread_t thread;
> +	pthread_barrier_t barrier;
> +	pid_t tid;
> +	void *(*threadfn)(void *);
> +	void *arg;
> +};
> +
> +static int wait_for_thread(FILE *fp, int timeout_us)
> +{
> +	char buf[80] = "";
> +
> +	for (int i = 0; i < WAIT_THREAD_RETRIES; i++) {
> +		if (!fgets(buf, sizeof(buf), fp))
> +			return -EIO;
> +		if (!strncmp(buf, "futex", 5))
> +			return 0;
> +		usleep(timeout_us / WAIT_THREAD_RETRIES);
> +		rewind(fp);
> +	}
> +	return -ETIMEDOUT;
> +}
> +
> +int futex_wait_for_thread(struct futex_thread *t, int timeout_us)
> +{
> +	char fname[80];
> +	FILE *fp;
> +	int res;
> +
> +	snprintf(fname, sizeof(fname), "/proc/%d/wchan", t->tid);
> +	fp = fopen(fname, "r");
> +	if (!fp)
> +		return -EIO;
> +	res = wait_for_thread(fp, timeout_us);
> +	fclose(fp);
> +	return res;
> +}
> +
> +static void *futex_thread_fn(void *arg)
> +{
> +	struct futex_thread *t = arg;
> +
> +	t->tid = gettid();
> +	pthread_barrier_wait(&t->barrier);
> +	return t->threadfn(t->arg);
> +}
> +
> +int futex_thread_create(struct futex_thread *t, void *(*threadfn)(void *), void *arg)
> +{
> +	int ret;
> +
> +	pthread_barrier_init(&t->barrier, NULL, 2);
> +	t->tid = 0;
> +	t->threadfn = threadfn;
> +	t->arg = arg;
> +
> +	ret = pthread_create(&t->thread, NULL, futex_thread_fn, t);
> +	if (ret)
> +		return ret;
> +
> +	pthread_barrier_wait(&t->barrier);
> +	return 0;
> +}
> +
>  TEST(requeue_single)
>  {
>  	volatile futex_t _f1 = 0;
>  	volatile futex_t f2 = 0;
> -	pthread_t waiter[10];
> +	struct futex_thread waiter;
>  
>  	f1 = &_f1;
>  
>  	/*
>  	 * Requeue a waiter from f1 to f2, and wake f2.
>  	 */
> -	ASSERT_EQ(0, pthread_create(&waiter[0], NULL, waiterfn, NULL));
> -
> -	usleep(WAKE_WAIT_US);
> +	EXPECT_EQ(0, futex_thread_create(&waiter, waiterfn, NULL));

Why this ASSERT_EQ -> EXPECT_EQ ?

> +	futex_wait_for_thread(&waiter, WAIT_THREAD_DELAY_US);
>  
>  	EXPECT_EQ(1, futex_cmp_requeue(f1, 0, &f2, 0, 1, 0));
>  	EXPECT_EQ(1, futex_wake(&f2, 1, 0));
> @@ -52,8 +116,7 @@ TEST(requeue_multiple)
>  {
>  	volatile futex_t _f1 = 0;
>  	volatile futex_t f2 = 0;
> -	pthread_t waiter[10];
> -	int i;
> +	struct futex_thread waiter[10];

Couldn't that `i# remain where it was and that waiter moved as per Xmas
notation?

>  
>  	f1 = &_f1;
>  
> @@ -61,10 +124,10 @@ TEST(requeue_multiple)
>  	 * Create 10 waiters at f1. At futex_requeue, wake 3 and requeue 7.
>  	 * At futex_wake, wake INT_MAX (should be exactly 7).
>  	 */
> -	for (i = 0; i < 10; i++)
> -		ASSERT_EQ(0, pthread_create(&waiter[i], NULL, waiterfn, NULL));
> -
> -	usleep(WAKE_WAIT_US);
> +	for (int i = 0; i < 10; i++) {
> +		EXPECT_EQ(0, futex_thread_create(&waiter[i], waiterfn, NULL));
> +		futex_wait_for_thread(&waiter[i], WAIT_THREAD_DELAY_US / 10);
> +	}
>  
>  	EXPECT_EQ(10, futex_cmp_requeue(f1, 0, &f2, 3, 7, 0));
>  	EXPECT_EQ(7, futex_wake(&f2, INT_MAX, 0));

Sebastian