mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/3] selftests/futex: fix the issue of abnormal test results caused by thread timing
@ 2025-12-11  8:36 Yuwen Chen
  2025-12-11  8:38 ` [PATCH 1/3] selftests/futex: reduce array declarations in the requeue_single Yuwen Chen
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Yuwen Chen @ 2025-12-11  8:36 UTC (permalink / raw)
  To: andrealmeid, akpm
  Cc: edliaw, dave, dbueso, dvhart, hpa, justinstitt, kernel-team,
	linux-kernel, linux-kselftest, llvm, luto, mingo, morbo, nathan,
	ndesaulniers, peterz, shuah, tglx, usama.anjum

On the Android arm32 platform, when performing the futex_requeue test, it will
most likely return a failure. The specific reason is detailed in a commit[1]
previously submitted by Edward Liaw. However, this commit cannot perfectly
solve the problem. This is because using a barrier does not guarantee that
the child thread will wait on futex_wait.

This series of patches attempts to solve this problem by checking whether
the child thread is in a sleeping state. This is because when the child thread
goes to sleep, it indicates that it is waiting for the futex lock.

Link: https://lore.kernel.org/all/20240918231102.234253-1-edliaw@google.com/


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/3] selftests/futex: reduce array declarations in the requeue_single
  2025-12-11  8:36 [PATCH 0/3] selftests/futex: fix the issue of abnormal test results caused by thread timing Yuwen Chen
@ 2025-12-11  8:38 ` Yuwen Chen
  2025-12-11  8:38 ` [PATCH 2/3] selftests/futex: use pthread_join to reclaim thread resources Yuwen Chen
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Yuwen Chen @ 2025-12-11  8:38 UTC (permalink / raw)
  To: ywen.chen
  Cc: akpm, andrealmeid, dave, dbueso, dvhart, edliaw, hpa,
	justinstitt, kernel-team, linux-kernel, linux-kselftest, llvm,
	luto, mingo, morbo, nathan, ndesaulniers, peterz, shuah, tglx,
	usama.anjum

In the requeue_single function, the variable "waits" only uses
one element. There is no need to use an array.

Signed-off-by: Yuwen Chen <ywen.chen@foxmail.com>
---
 tools/testing/selftests/futex/functional/futex_requeue.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/futex/functional/futex_requeue.c b/tools/testing/selftests/futex/functional/futex_requeue.c
index 69e2555b60399..1807465de2144 100644
--- a/tools/testing/selftests/futex/functional/futex_requeue.c
+++ b/tools/testing/selftests/futex/functional/futex_requeue.c
@@ -33,7 +33,7 @@ TEST(requeue_single)
 {
 	volatile futex_t _f1 = 0;
 	volatile futex_t f2 = 0;
-	pthread_t waiter[10];
+	pthread_t waiter;
 	int res;
 
 	f1 = &_f1;
@@ -41,7 +41,7 @@ TEST(requeue_single)
 	/*
 	 * Requeue a waiter from f1 to f2, and wake f2.
 	 */
-	if (pthread_create(&waiter[0], NULL, waiterfn, NULL))
+	if (pthread_create(&waiter, NULL, waiterfn, NULL))
 		ksft_exit_fail_msg("pthread_create failed\n");
 
 	usleep(WAKE_WAIT_US);
-- 
2.34.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/3] selftests/futex: use pthread_join to reclaim thread resources
  2025-12-11  8:36 [PATCH 0/3] selftests/futex: fix the issue of abnormal test results caused by thread timing Yuwen Chen
  2025-12-11  8:38 ` [PATCH 1/3] selftests/futex: reduce array declarations in the requeue_single Yuwen Chen
@ 2025-12-11  8:38 ` Yuwen Chen
  2025-12-11  8:38 ` [PATCH 3/3] selftests/futex: fix the issue of abnormal test results caused by thread timing Yuwen Chen
  2025-12-12  4:30 ` Licay
  3 siblings, 0 replies; 6+ messages in thread
From: Yuwen Chen @ 2025-12-11  8:38 UTC (permalink / raw)
  To: ywen.chen
  Cc: akpm, andrealmeid, dave, dbueso, dvhart, edliaw, hpa,
	justinstitt, kernel-team, linux-kernel, linux-kselftest, llvm,
	luto, mingo, morbo, nathan, ndesaulniers, peterz, shuah, tglx,
	usama.anjum

When creating a thread using pthread_create, you should use
pthread_join to free its resources.

Signed-off-by: Yuwen Chen <ywen.chen@foxmail.com>
---
 tools/testing/selftests/futex/functional/futex_requeue.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/testing/selftests/futex/functional/futex_requeue.c b/tools/testing/selftests/futex/functional/futex_requeue.c
index 1807465de2144..7a22458c7fc96 100644
--- a/tools/testing/selftests/futex/functional/futex_requeue.c
+++ b/tools/testing/selftests/futex/functional/futex_requeue.c
@@ -62,6 +62,8 @@ TEST(requeue_single)
 	} else {
 		ksft_test_result_pass("futex_requeue simple succeeds\n");
 	}
+
+	pthread_join(waiter, NULL);
 }
 
 TEST(requeue_multiple)
@@ -101,6 +103,9 @@ TEST(requeue_multiple)
 	} else {
 		ksft_test_result_pass("futex_requeue many succeeds\n");
 	}
+
+	for (i = 0; i < 10; i++)
+		pthread_join(waiter[i], NULL);
 }
 
 TEST_HARNESS_MAIN
-- 
2.34.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 3/3] selftests/futex: fix the issue of abnormal test results caused by thread timing
  2025-12-11  8:36 [PATCH 0/3] selftests/futex: fix the issue of abnormal test results caused by thread timing Yuwen Chen
  2025-12-11  8:38 ` [PATCH 1/3] selftests/futex: reduce array declarations in the requeue_single Yuwen Chen
  2025-12-11  8:38 ` [PATCH 2/3] selftests/futex: use pthread_join to reclaim thread resources Yuwen Chen
@ 2025-12-11  8:38 ` Yuwen Chen
  2025-12-12  4:30 ` Licay
  3 siblings, 0 replies; 6+ messages in thread
From: Yuwen Chen @ 2025-12-11  8:38 UTC (permalink / raw)
  To: ywen.chen
  Cc: akpm, andrealmeid, dave, dbueso, dvhart, edliaw, hpa,
	justinstitt, kernel-team, linux-kernel, linux-kselftest, llvm,
	luto, mingo, morbo, nathan, ndesaulniers, peterz, shuah, tglx,
	usama.anjum

Fixes a race between parent and child threads in futex_requeue.

Similar to commit 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

This issue can be resolved by checking whether the child thread is in a
sleeping state. This is because when the child thread goes to sleep, it
indicates that it is waiting for the futex lock.

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>
---
 .../selftests/futex/functional/Makefile       |  2 +-
 .../futex/functional/futex_requeue.c          | 58 ++++++++++++++++---
 2 files changed, 52 insertions(+), 8 deletions(-)

diff --git a/tools/testing/selftests/futex/functional/Makefile b/tools/testing/selftests/futex/functional/Makefile
index 490ace1f017e8..8589917a4b126 100644
--- a/tools/testing/selftests/futex/functional/Makefile
+++ b/tools/testing/selftests/futex/functional/Makefile
@@ -2,7 +2,7 @@
 PKG_CONFIG ?= pkg-config
 LIBNUMA_TEST = $(shell sh -c "$(PKG_CONFIG) numa --atleast-version 2.0.16 > /dev/null 2>&1 && echo SUFFICIENT || echo NO")
 
-INCLUDES := -I../include -I../../ $(KHDR_INCLUDES)
+INCLUDES := -I../include -I../../ $(KHDR_INCLUDES) -I../../../../include
 CFLAGS := $(CFLAGS) -g -O2 -Wall -pthread -D_FILE_OFFSET_BITS=64 -D_TIME_BITS=64 $(INCLUDES) $(KHDR_INCLUDES) -DLIBNUMA_VER_$(LIBNUMA_TEST)=1
 LDLIBS := -lpthread -lrt -lnuma
 
diff --git a/tools/testing/selftests/futex/functional/futex_requeue.c b/tools/testing/selftests/futex/functional/futex_requeue.c
index 7a22458c7fc96..994295fac6972 100644
--- a/tools/testing/selftests/futex/functional/futex_requeue.c
+++ b/tools/testing/selftests/futex/functional/futex_requeue.c
@@ -7,6 +7,7 @@
 
 #include <pthread.h>
 #include <limits.h>
+#include <linux/compiler.h>
 
 #include "futextest.h"
 #include "../../kselftest_harness.h"
@@ -15,6 +16,7 @@
 #define WAKE_WAIT_US 10000
 
 volatile futex_t *f1;
+static pthread_barrier_t barrier;
 
 void *waiterfn(void *arg)
 {
@@ -23,28 +25,59 @@ void *waiterfn(void *arg)
 	to.tv_sec = 0;
 	to.tv_nsec = timeout_ns;
 
+	WRITE_ONCE(*((pid_t *)arg), gettid());
+	pthread_barrier_wait(&barrier);
+
 	if (futex_wait(f1, *f1, &to, 0))
 		printf("waiter failed errno %d\n", errno);
 
 	return NULL;
 }
 
+static int get_thread_state(pid_t pid)
+{
+	FILE *fp;
+	char buf[80], tag[80];
+	char val = 0;
+
+	snprintf(buf, sizeof(buf), "/proc/%d/status", pid);
+	fp = fopen(buf, "r");
+	if (!fp)
+		return -1;
+
+	while (fgets(buf, sizeof(buf), fp))
+		if (fscanf(fp, "%s %c\n", tag, &val) == 2 && !strcmp(tag, "State:"))
+			break;
+
+	fclose(fp);
+	return val;
+}
+
 TEST(requeue_single)
 {
 	volatile futex_t _f1 = 0;
 	volatile futex_t f2 = 0;
 	pthread_t waiter;
-	int res;
+	pid_t tids;
+	int res, state;
 
 	f1 = &_f1;
+	pthread_barrier_init(&barrier, NULL, 2);
 
 	/*
 	 * Requeue a waiter from f1 to f2, and wake f2.
 	 */
-	if (pthread_create(&waiter, NULL, waiterfn, NULL))
+	if (pthread_create(&waiter, NULL, waiterfn, &tids))
 		ksft_exit_fail_msg("pthread_create failed\n");
 
-	usleep(WAKE_WAIT_US);
+	pthread_barrier_wait(&barrier);
+	pthread_barrier_destroy(&barrier);
+	while ((state = get_thread_state(READ_ONCE(tids))) != 'S') {
+		usleep(WAKE_WAIT_US);
+
+		if (state < 0)
+			break;
+	}
 
 	ksft_print_dbg_msg("Requeuing 1 futex from f1 to f2\n");
 	res = futex_cmp_requeue(f1, 0, &f2, 0, 1, 0);
@@ -71,7 +104,8 @@ TEST(requeue_multiple)
 	volatile futex_t _f1 = 0;
 	volatile futex_t f2 = 0;
 	pthread_t waiter[10];
-	int res, i;
+	pid_t tids[10];
+	int res, i, state;
 
 	f1 = &_f1;
 
@@ -80,11 +114,21 @@ TEST(requeue_multiple)
 	 * At futex_wake, wake INT_MAX (should be exactly 7).
 	 */
 	for (i = 0; i < 10; i++) {
-		if (pthread_create(&waiter[i], NULL, waiterfn, NULL))
+		pthread_barrier_init(&barrier, NULL, 2);
+
+		if (pthread_create(&waiter[i], NULL, waiterfn, &tids[i]))
 			ksft_exit_fail_msg("pthread_create failed\n");
-	}
 
-	usleep(WAKE_WAIT_US);
+		pthread_barrier_wait(&barrier);
+		pthread_barrier_destroy(&barrier);
+
+		while ((state = get_thread_state(READ_ONCE(tids[i]))) != 'S') {
+			usleep(WAKE_WAIT_US);
+
+			if (state < 0)
+				break;
+		}
+	}
 
 	ksft_print_dbg_msg("Waking 3 futexes at f1 and requeuing 7 futexes from f1 to f2\n");
 	res = futex_cmp_requeue(f1, 0, &f2, 3, 7, 0);
-- 
2.34.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 3/3] selftests/futex: fix the issue of abnormal test results caused by thread timing
  2025-12-11  8:36 [PATCH 0/3] selftests/futex: fix the issue of abnormal test results caused by thread timing Yuwen Chen
                   ` (2 preceding siblings ...)
  2025-12-11  8:38 ` [PATCH 3/3] selftests/futex: fix the issue of abnormal test results caused by thread timing Yuwen Chen
@ 2025-12-12  4:30 ` Licay
  2025-12-15  7:50   ` Yuwen Chen
  3 siblings, 1 reply; 6+ messages in thread
From: Licay @ 2025-12-12  4:30 UTC (permalink / raw)
  To: ywen.chen
  Cc: akpm, andrealmeid, dave, dbueso, dvhart, edliaw, hpa,
	justinstitt, kernel-team, linux-kernel, linux-kselftest, llvm,
	luto, mingo, morbo, nathan, ndesaulniers, peterz, shuah, tglx,
	usama.anjum, Licay

From: Licay <licayy@outlook.com>

Hi Ywen,

Thanks for the patch! I have a few suggestions to improve it:

1. Avoid Parsing /proc
   The current approach uses get_thread_state() to read /proc/$pid/status, which isn't very reliable.
   A better way would be to have waiterfn directly signal when it's ready using atomic operations.

2. Use Atomic Counting Instead of Polling Thread State
   Before entering futex_wait, waiterfn can atomically increment a counter. The parent thread then just waits for this counter to reach the expected value.
   This is much simpler and avoids the overhead of checking /proc repeatedly.

3. Use Standard Atomic Types
   Replace the custom READ_ONCE/WRITE_ONCE macros with standard <stdatomic.h> types like atomic_int.
   It's cleaner and more portable across different platforms.

Here's the basic idea:
- Add a global atomic_int ready_count variable
- In waiterfn: atomic_fetch_add(&ready_count, 1) right before futex_wait()
- Parent thread: spin-wait until atomic_load(&ready_count) reaches the expected value

This approach is much cleaner - no /proc dependency, simpler logic, and better performance.

Best regards,
Licay
Signed-off-by: Licay <licayy@outlook.com>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 3/3] selftests/futex: fix the issue of abnormal test results caused by thread timing
  2025-12-12  4:30 ` Licay
@ 2025-12-15  7:50   ` Yuwen Chen
  0 siblings, 0 replies; 6+ messages in thread
From: Yuwen Chen @ 2025-12-15  7:50 UTC (permalink / raw)
  To: licayy
  Cc: akpm, andrealmeid, dave, dbueso, dvhart, edliaw, hpa,
	justinstitt, kernel-team, licayy, linux-kernel, linux-kselftest,
	llvm, luto, mingo, morbo, nathan, ndesaulniers, peterz, shuah,
	tglx, usama.anjum, ywen.chen

on Fri, 12 Dec 2025 12:30:20 +0800, Licay wrote:
> 1. Avoid Parsing /proc
>    The current approach uses get_thread_state() to read /proc/$pid/status, which isn't very reliable.
>    A better way would be to have waiterfn directly signal when it's ready using atomic operations.

I haven't found a better way to check whether a process has entered the
sleep state without using the proc interface for the time being. The kernel
probably doesn't provide a system call to obtain whether a process has
entered the sleep state.

> 2. Use Atomic Counting Instead of Polling Thread State
>    Before entering futex_wait, waiterfn can atomically increment a counter. The parent thread then just waits for this counter to reach the expected value.
>    This is much simpler and avoids the overhead of checking /proc repeatedly.

Using atomic instructions does not guarantee that the process is waiting in futex_wait.

> 3. Use Standard Atomic Types
>    Replace the custom READ_ONCE/WRITE_ONCE macros with standard <stdatomic.h> types like atomic_int.
>    It's cleaner and more portable across different platforms.

Only two operations, atomic_set and atomic_read, are used here. Using
WRITE_ONCE and READ_ONCE can avoid introducing too many dependencies.

> Here's the basic idea:
> - Add a global atomic_int ready_count variable
> - In waiterfn: atomic_fetch_add(&ready_count, 1) right before futex_wait()
> - Parent thread: spin-wait until atomic_load(&ready_count) reaches the expected value

Similarly, using atomic_inc before futex_wait does not guarantee that
the thread has reached the futex_wait execution point.

Thank you very much for your reply.


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-12-15  7:50 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-12-11  8:36 [PATCH 0/3] selftests/futex: fix the issue of abnormal test results caused by thread timing Yuwen Chen
2025-12-11  8:38 ` [PATCH 1/3] selftests/futex: reduce array declarations in the requeue_single Yuwen Chen
2025-12-11  8:38 ` [PATCH 2/3] selftests/futex: use pthread_join to reclaim thread resources Yuwen Chen
2025-12-11  8:38 ` [PATCH 3/3] selftests/futex: fix the issue of abnormal test results caused by thread timing Yuwen Chen
2025-12-12  4:30 ` Licay
2025-12-15  7:50   ` Yuwen Chen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®