[PATCH v7 0/2] selftests/futex: fix the failed futex_requeue test issue

[PATCH v7 0/2] selftests/futex: fix the failed futex_requeue test issue

[Yuwen Chen]

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.

An attempt is made to avoid this problem by checking whether the child
thread is in a sleeping state in the main thread.

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.

v3->v4: https://lore.kernel.org/all/tencent_FC5FB35D2545AFDCA6E377AE4DE75C79AF08@qq.com/
    1. Add a new futex_thread.h file.
    2. Add a new function named futex_thread_destroy.
    3. Fix the issue that threads are not reclaimed in futex_requeue.

v4->v5: https://lore.kernel.org/all/tencent_204540DCC2E303AEB6E77679C05F5C5D3808@qq.com/
    1. Split the patch into two.
    2. Modify the thread interface to support returning error codes.
    3. In the test cases, check the return values of the threads.
    4. Modify the macro definitions to more understandable names.
    5. When failing to open the /proc/%d/wchan file, delay for 100 ms.
    6. Modify the parameter description of the interface.

v5->v6: https://lore.kernel.org/all/tencent_35CFB8AE9D4BE7917D95763D15B972638C0A@qq.com/
    1. Add parameters to the futex_wait_for_thread function to support custom timeout periods.
    2. Even when the proc file system is not mounted, the test case can still pass.
    3. Add timeout time comments to the usleep function.
    4. Reduce the time of FUTEX_WAIT_TIMEOUT_SECS to 1 second.
    5. Add the necessary header files to futex_thread.h.

v6->v7: https://lore.kernel.org/all/tencent_D6D5FA3C2F6FE52BA6ABA1A88C1B6E1CC605@qq.com/
    1. Changed `static` to `static inline` for all functions in futex_thread.h.
    2. Added `sleep_time_us` variable with zero-guard in __wait_for_thread() to prevent
       tight loop when timeout_us < WAIT_THREAD_RETRIES.
    3. Removed fallback usleep(100ms) from futex_wait_for_thread(); moved error handling to callers.
    4. Added return value check for pthread_barrier_init() in futex_thread_create().
    5. Added pthread_barrier_destroy() cleanup on pthread_create() failure path.
    6. Unified error checking to `if (ret < 0)` in both requeue_single and requeue_multiple.
    7. Added comment for the break logic in requeue_multiple when /proc is not available.

Yuwen Chen (2):
  selftests/futex: implement the interfaces related to threads
  selftests/futex: fix the failed futex_requeue test issue

 .../selftests/futex/functional/Makefile       |   3 +-
 .../futex/functional/futex_requeue.c          |  46 ++++---
 .../selftests/futex/include/futex_thread.h    | 114 ++++++++++++++++++
 3 files changed, 148 insertions(+), 15 deletions(-)
 create mode 100644 tools/testing/selftests/futex/include/futex_thread.h

-- 
2.34.1

[PATCH v7 1/2] selftests/futex: implement the interfaces related to threads

[Yuwen Chen]

There are timing issues in the use of threads in some unit test cases
for futex. A potentially good solution is to check whether the thread
is in the sleep state after creating it.

A file named futex_thread.h is added, in which several thread-related
functions are implemented to facilitate the solution of this problem.

Signed-off-by: Yuwen Chen <ywen.chen@foxmail.com>
---
v6->v7:
    1. Changed `static` to `static inline` for all functions to prevent
       compiler warnings about unused functions.
    2. Added `sleep_time_us` variable with zero-guard in __wait_for_thread()
       to prevent tight loop when timeout_us < WAIT_THREAD_RETRIES.
    3. Removed fallback usleep(100ms) from futex_wait_for_thread(); the
       caller is now responsible for handling errors.
    4. Added return value check for pthread_barrier_init().
    5. Added pthread_barrier_destroy() cleanup on pthread_create() failure.

 .../selftests/futex/functional/Makefile       |   3 +-
 .../selftests/futex/include/futex_thread.h    | 114 ++++++++++++++++++
 2 files changed, 116 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/futex/include/futex_thread.h

diff --git a/tools/testing/selftests/futex/functional/Makefile b/tools/testing/selftests/futex/functional/Makefile
index 5c1c824f97400..724e8216e41b1 100644
--- a/tools/testing/selftests/futex/functional/Makefile
+++ b/tools/testing/selftests/futex/functional/Makefile
@@ -11,7 +11,8 @@ endif
 
 LOCAL_HDRS := \
 	../include/futextest.h \
-	../include/atomic.h
+	../include/atomic.h \
+	../include/futex_thread.h
 TEST_GEN_PROGS := \
 	futex_wait_timeout \
 	futex_wait_wouldblock \
diff --git a/tools/testing/selftests/futex/include/futex_thread.h b/tools/testing/selftests/futex/include/futex_thread.h
new file mode 100644
index 0000000000000..49ae687012fe5
--- /dev/null
+++ b/tools/testing/selftests/futex/include/futex_thread.h
@@ -0,0 +1,114 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#ifndef _FUTEX_THREAD_H
+#define _FUTEX_THREAD_H
+#include <stdio.h>
+#include <string.h>
+#include <errno.h>
+#include <pthread.h>
+#include <unistd.h>
+
+#define USEC_PER_SEC 1000000L
+#define WAIT_THREAD_RETRIES 100
+
+struct futex_thread {
+	pthread_t		thread;
+	pthread_barrier_t	barrier;
+	pid_t			tid;
+	int			(*threadfn)(void *arg);
+	void			*arg;
+	int			retval;
+};
+
+static inline int __wait_for_thread(FILE *fp, int timeout_us)
+{
+	char buf[80] = "";
+	int i;
+	int sleep_time_us = timeout_us / WAIT_THREAD_RETRIES;
+
+	if (sleep_time_us <= 0)
+		sleep_time_us = 1;
+
+	for (i = 0; i < WAIT_THREAD_RETRIES; i++) {
+		if (!fgets(buf, sizeof(buf), fp))
+			return -EIO;
+		if (!strncmp(buf, "futex", 5))
+			return 0;
+		usleep(sleep_time_us);
+		rewind(fp);
+	}
+	return -ETIMEDOUT;
+}
+
+static void *__futex_thread_fn(void *arg)
+{
+	struct futex_thread *t = arg;
+
+	t->tid = gettid();
+	pthread_barrier_wait(&t->barrier);
+	t->retval = t->threadfn(t->arg);
+	return NULL;
+}
+
+/**
+ * futex_wait_for_thread - Wait for the child thread to sleep in the futex context
+ * @t:          Thread handle.
+ * @timeout_us: The timeout for waiting for the thread to enter the sleep state.
+ */
+static inline 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;
+}
+
+/**
+ * futex_thread_create - Create a new thread for testing.
+ * @t:        The handle of the newly created thread.
+ * @threadfn: The new thread starts execution by invoking threadfn
+ * @arg:      The parameters passed to threadfn.
+ */
+static inline int futex_thread_create(struct futex_thread *t, int (*threadfn)(void *), void *arg)
+{
+	int ret;
+
+	ret = pthread_barrier_init(&t->barrier, NULL, 2);
+	if (ret)
+		return ret;
+
+	t->tid = 0;
+	t->threadfn = threadfn;
+	t->arg = arg;
+
+	ret = pthread_create(&t->thread, NULL, __futex_thread_fn, t);
+	if (ret) {
+		pthread_barrier_destroy(&t->barrier);
+		return ret;
+	}
+
+	pthread_barrier_wait(&t->barrier);
+	return 0;
+}
+
+/**
+ * futex_thread_destroy - Wait for and reclaim the resources of the thread.
+ * @t:      Thread handle.
+ */
+static inline int futex_thread_destroy(struct futex_thread *t)
+{
+	pthread_join(t->thread, NULL);
+	pthread_barrier_destroy(&t->barrier);
+	return t->retval;
+}
+
+#endif
-- 
2.34.1

[PATCH v7 2/2] selftests/futex: fix the failed futex_requeue test issue

[Yuwen Chen]

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.

An attempt is made to avoid this problem by checking whether the child
thread is in a sleeping state in the main thread.

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>
---
v6->v7:
    1. Unified error checking to `if (ret < 0)` in both requeue_single
       and requeue_multiple test cases.
    2. Added comment explaining the break logic in requeue_multiple when
       /proc is not available.

 .../futex/functional/futex_requeue.c          | 46 +++++++++++++------
 1 file changed, 32 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..5c849ccf16e26 100644
--- a/tools/testing/selftests/futex/functional/futex_requeue.c
+++ b/tools/testing/selftests/futex/functional/futex_requeue.c
@@ -9,51 +9,59 @@
 #include <limits.h>
 
 #include "futextest.h"
+#include "futex_thread.h"
 #include "kselftest_harness.h"
 
-#define timeout_ns  30000000
-#define WAKE_WAIT_US 10000
+#define FUTEX_WAIT_TIMEOUT_SECS 1
+#define WAIT_THREAD_CREATE_TIMEOUT_USECS (USEC_PER_SEC / 2) /* 500ms */
 
 volatile futex_t *f1;
 
-void *waiterfn(void *arg)
+static int waiterfn(void *arg)
 {
 	struct timespec to;
 
-	to.tv_sec = 0;
-	to.tv_nsec = timeout_ns;
+	to.tv_sec = FUTEX_WAIT_TIMEOUT_SECS;
+	to.tv_nsec = 0;
 
-	if (futex_wait(f1, *f1, &to, 0))
+	if (futex_wait(f1, *f1, &to, 0)) {
 		printf("waiter failed errno %d\n", errno);
+		return -errno;
+	}
 
-	return NULL;
+	return 0;
 }
 
 TEST(requeue_single)
 {
+	struct futex_thread waiter;
 	volatile futex_t _f1 = 0;
 	volatile futex_t f2 = 0;
-	pthread_t waiter[10];
+	int ret;
 
 	f1 = &_f1;
 
 	/*
 	 * Requeue a waiter from f1 to f2, and wake f2.
 	 */
-	ASSERT_EQ(0, pthread_create(&waiter[0], NULL, waiterfn, NULL));
+	ASSERT_EQ(0, futex_thread_create(&waiter, waiterfn, NULL));
 
-	usleep(WAKE_WAIT_US);
+	ret = futex_wait_for_thread(&waiter, WAIT_THREAD_CREATE_TIMEOUT_USECS);
+	if (ret < 0)
+		usleep(WAIT_THREAD_CREATE_TIMEOUT_USECS);
 
 	EXPECT_EQ(1, futex_cmp_requeue(f1, 0, &f2, 0, 1, 0));
 	EXPECT_EQ(1, futex_wake(&f2, 1, 0));
+
+	EXPECT_EQ(0, futex_thread_destroy(&waiter));
 }
 
 TEST(requeue_multiple)
 {
+	struct futex_thread waiter[10];
 	volatile futex_t _f1 = 0;
 	volatile futex_t f2 = 0;
-	pthread_t waiter[10];
-	int i;
+	int i, ret;
 
 	f1 = &_f1;
 
@@ -62,12 +70,22 @@ TEST(requeue_multiple)
 	 * 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));
+		ASSERT_EQ(0, futex_thread_create(&waiter[i], waiterfn, NULL));
 
-	usleep(WAKE_WAIT_US);
+	for (i = 0; i < 10; i++) {
+		ret = futex_wait_for_thread(&waiter[i], WAIT_THREAD_CREATE_TIMEOUT_USECS / 10);
+		if (ret < 0) {
+			/* /proc not available, give all threads time to enter futex wait */
+			usleep(WAIT_THREAD_CREATE_TIMEOUT_USECS);
+			break;
+		}
+	}
 
 	EXPECT_EQ(10, futex_cmp_requeue(f1, 0, &f2, 3, 7, 0));
 	EXPECT_EQ(7, futex_wake(&f2, INT_MAX, 0));
+
+	for (i = 0; i < 10; i++)
+		EXPECT_EQ(0, futex_thread_destroy(&waiter[i]));
 }
 
 TEST_HARNESS_MAIN
-- 
2.34.1