On Tue, May 26, 2026 at 2:51 AM André Almeida <[email protected]> wrote: > > Em 25/05/2026 06:20, Wake Liu escreveu: > > Migrate futex_wait test to the kselftest harness framework, > > removing mixed legacy ksft_* API usages and ensuring proper thread joining. > > > > Why is this patchset not part of the same series that you just sent? >
The overall scope of migrating all the remaining futex functional tests to the harness is quite large (I still have 2 series planned for requeue tests, just not finish yet lol). To keep the review process manageable and incremental, I prefer splitting them into smaller, logically grouped topic-based series (e.g., wait-related vs. requeue-related). > > Signed-off-by: Wake Liu <[email protected]> > > --- > > .../selftests/futex/functional/futex_wait.c | 125 +++++++++++------- > > 1 file changed, 77 insertions(+), 48 deletions(-) > > > > diff --git a/tools/testing/selftests/futex/functional/futex_wait.c > > b/tools/testing/selftests/futex/functional/futex_wait.c > > index 7b8879409007..ed4b040600b8 100644 > > --- a/tools/testing/selftests/futex/functional/futex_wait.c > > +++ b/tools/testing/selftests/futex/functional/futex_wait.c > > @@ -9,6 +9,7 @@ > > #include <sys/shm.h> > > #include <sys/mman.h> > > #include <fcntl.h> > > +#include <stdlib.h> > > > > #include "futextest.h" > > #include "kselftest_harness.h" > > @@ -19,125 +20,153 @@ > > > > void *futex; > > > > +struct waiter_args { > > + struct __test_metadata *_metadata; > > + unsigned int flags; > > +}; > > + > > static void *waiterfn(void *arg) > > { > > + struct waiter_args *args = (struct waiter_args *)arg; > > + struct __test_metadata *_metadata = args->_metadata; > > As much as I like the benefits of being able to properly use the harness > macros in threads, I fell like we are touching private structs here. Are > they safe to be used concurrently? If a child thread an the main thread > fails, does that counts as two fails? > > Perhaps we should have a look to create a kselftest wrapper for > pthread_create(). > Regarding concurrency safety: _metadata is allocated via MAP_SHARED in the harness, so it is shared across forks. For pthread threads, they share the same address space, so they naturally access the exact same _metadata pointer. While concurrently writing to _metadata->exit_code on failure is technically a data race under the C standard, practically it is a simple write of a constant value (KSFT_FAIL), which is robust enough for test scenarios. Stdio logging (TH_LOG) is also thread-safe in glibc (flockfile is used internally). If a child thread and the main thread both fail, it will still register as a single test failure since they both set _metadata->exit_code = KSFT_FAIL. A kselftest wrapper for pthread_create() would indeed be a great addition to the harness for cleaner thread management in the future! > > struct timespec to; > > - unsigned int flags = 0; > > - > > - if (arg) > > - flags = *((unsigned int *) arg); > > + int res; > > > > to.tv_sec = 0; > > to.tv_nsec = timeout_ns; > > > > - if (futex_wait(futex, 0, &to, flags)) > > - printf("waiter failed errno %d\n", errno); > > + res = futex_wait(futex, 0, &to, args->flags); > > + if (res) { > > + EXPECT_EQ(res, 0) > > I have changed it to ASSERT_EQ(res, -1) just to see what happens when a > child thread fails, but the test still reported PASSED: 3 / 3 I believe you might have hit a logical skip in the conditional check here: if (res) { ASSERT_EQ(res, -1); } Since futex_wait succeeds in the normal path (returning 0), the "if (res)" condition evaluates to false, and the block is skipped entirely. This is why your amended assertion was never evaluated, and the test reported PASSED. If you place the assertion outside the conditional block (e.g., immediately after futex_wait) or force a failure on success (res == 0), it will correctly trigger a SIGABRT via abort() inside the child thread, terminating the entire test process and reporting a FAILURE in the harness. -- Best Regards, Wake Liu
