[PATCH v2 2/4] selftests/futex: Retry the wake in futex_wait instead of sleeping

[Mathieu Desnoyers]

Each futex_wait test starts a waiter thread, which calls futex_wait()
with a 30ms timeout. The main thread sleeps 10ms, then expects
futex_wake() to wake one waiter. So the test assumes that the waiter
reaches futex_wait() within 10ms, and that the main thread runs again
within 30ms.

A busy machine breaks both. The built-in locktorture at times holds a
spinlock for 100ms with preemption off. With it, private_futex failed in
about 0.5% of the runs in 8-CPU VMs, on v7.4-rc1. Timestamps showed two
cases. The waiter started up to 75ms late, after the wake. Or the main
thread woke up to 105ms late, after the waiter had timed out.

Fix this without relying on timing. The waiter sets a flag once
futex_wait() returns. The main thread retries futex_wake() every
millisecond until it sees that flag, and expects one waiter woken in
all. The waiter waits without a timeout. If a wake is lost, the timeout
of the kselftest harness ends the test. Check what pthread_join()
returns.

With this patch, private_futex passed 3000 runs of 3000 in the same VMs,
and the three tests 1000 runs of 1000.

Fixes: c3d128581f64 ("selftests: futex: Add futex wait test")
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx>
---
 .../selftests/futex/functional/futex_wait.c   | 57 ++++++++++++-------
 1 file changed, 38 insertions(+), 19 deletions(-)

diff --git a/tools/testing/selftests/futex/functional/futex_wait.c b/tools/testing/selftests/futex/functional/futex_wait.c
index 5292f5d2c3b5..4bf7d8ee238b 100644
--- a/tools/testing/selftests/futex/functional/futex_wait.c
+++ b/tools/testing/selftests/futex/functional/futex_wait.c
@@ -14,11 +14,11 @@
 #include "futextest.h"
 #include "kselftest_harness.h"
 
-#define timeout_ns  30000000
-#define WAKE_WAIT_US 10000
+#define WAKE_RETRY_US 1000
 #define SHM_PATH "futex_shm_file"
 
 void *futex;
+static int waiter_done;
 
 struct waiter_args {
 	struct __test_metadata	*_metadata;
@@ -29,13 +29,10 @@ static void *waiterfn(void *arg)
 {
 	struct waiter_args *args = (struct waiter_args *)arg;
 	struct __test_metadata *_metadata = args->_metadata;
-	struct timespec to;
 	int res;
 
-	to.tv_sec = 0;
-	to.tv_nsec = timeout_ns;
-
-	res = futex_wait(futex, 0, &to, args->flags);
+	res = futex_wait(futex, 0, NULL, args->flags);
+	__atomic_store_n(&waiter_done, 1, __ATOMIC_RELEASE);
 	if (res) {
 		EXPECT_EQ(res, 0)
 			TH_LOG("waiter failed errno %d: %s", errno, strerror(errno));
@@ -45,6 +42,25 @@ static void *waiterfn(void *arg)
 	return NULL;
 }
 
+/*
+ * Wake the waiter of @uaddr until it has returned from futex_wait(). The
+ * waiter may not have reached futex_wait() yet, and a wake then finds no
+ * waiter. Return how many waiters futex_wake() woke, or -1 on error.
+ */
+static int wake_waiter(void *uaddr, unsigned int flags)
+{
+	int res, woken = 0;
+
+	while (!__atomic_load_n(&waiter_done, __ATOMIC_ACQUIRE)) {
+		res = futex_wake(uaddr, 1, flags);
+		if (res < 0)
+			return res;
+		woken += res;
+		usleep(WAKE_RETRY_US);
+	}
+	return woken;
+}
+
 TEST(private_futex)
 {
 	struct waiter_args *args = malloc(sizeof(*args));
@@ -58,17 +74,18 @@ TEST(private_futex)
 
 	/* Testing a private futex */
 	TH_LOG("Calling private futex_wait on futex: %p", futex);
+	waiter_done = 0;
 	ASSERT_EQ(pthread_create(&waiter, NULL, waiterfn, args), 0)
 		TH_LOG("pthread_create failed");
 
-	usleep(WAKE_WAIT_US);
-
 	TH_LOG("Calling private futex_wake on futex: %p", futex);
-	res = futex_wake(futex, 1, FUTEX_PRIVATE_FLAG);
+	res = wake_waiter(futex, FUTEX_PRIVATE_FLAG);
 	EXPECT_EQ(res, 1)
 		TH_LOG("futex_wake private returned: %d %s", res, res < 0 ? strerror(errno) : "");
 
-	pthread_join(waiter, NULL);
+	res = pthread_join(waiter, NULL);
+	EXPECT_EQ(res, 0)
+		TH_LOG("pthread_join failed: %s", strerror(res));
 }
 
 TEST(anon_page)
@@ -103,19 +120,20 @@ TEST(anon_page)
 	futex = shared_data;
 
 	TH_LOG("Calling shared (page anon) futex_wait on futex: %p", futex);
+	waiter_done = 0;
 	ASSERT_EQ(pthread_create(&waiter, NULL, waiterfn, args), 0)
 		TH_LOG("pthread_create failed");
 
-	usleep(WAKE_WAIT_US);
-
 	TH_LOG("Calling shared (page anon) futex_wake on futex: %p", futex);
-	res = futex_wake(futex, 1, 0);
+	res = wake_waiter(futex, 0);
 	EXPECT_EQ(res, 1) {
 		TH_LOG("futex_wake shared (page anon) returned: %d %s",
 		       res, res < 0 ? strerror(errno) : "");
 	}
 
-	pthread_join(waiter, NULL);
+	res = pthread_join(waiter, NULL);
+	EXPECT_EQ(res, 0)
+		TH_LOG("pthread_join failed: %s", strerror(res));
 	shmdt(shared_data);
 }
 
@@ -158,19 +176,20 @@ TEST(file_backed)
 	futex = shm;
 
 	TH_LOG("Calling shared (file backed) futex_wait on futex: %p", futex);
+	waiter_done = 0;
 	ASSERT_EQ(pthread_create(&waiter, NULL, waiterfn, args), 0)
 		TH_LOG("pthread_create failed");
 
-	usleep(WAKE_WAIT_US);
-
 	TH_LOG("Calling shared (file backed) futex_wake on futex: %p", futex);
-	res = futex_wake(shm, 1, 0);
+	res = wake_waiter(shm, 0);
 	EXPECT_EQ(res, 1) {
 		TH_LOG("futex_wake shared (file backed) returned: %d %s",
 		       res, res < 0 ? strerror(errno) : "");
 	}
 
-	pthread_join(waiter, NULL);
+	res = pthread_join(waiter, NULL);
+	EXPECT_EQ(res, 0)
+		TH_LOG("pthread_join failed: %s", strerror(res));
 	munmap(shm, sizeof(f_private));
 	remove(SHM_PATH);
 	close(fd);
-- 
2.47.3