[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1d6a0c1e-5fa2-4c21-b3c1-7bfb2f9dd669@redhat.com>
Date: Wed, 3 Sep 2025 13:53:41 -0400
From: Waiman Long <llong@...hat.com>
To: André Almeida <andrealmeid@...lia.com>,
Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
linux-kernel@...r.kernel.org
Cc: Darren Hart <dvhart@...radead.org>, Davidlohr Bueso <dave@...olabs.net>,
Ingo Molnar <mingo@...hat.com>, Juri Lelli <juri.lelli@...hat.com>,
Peter Zijlstra <peterz@...radead.org>, Thomas Gleixner <tglx@...utronix.de>,
Valentin Schneider <vschneid@...hat.com>, Borislav Petkov <bp@...en8.de>,
kernel-dev@...lia.com
Subject: Re: [PATCH 1/2] selftest/futex: Make the error check more precise for
futex_numa_mpol
On 9/1/25 4:33 PM, André Almeida wrote:
> Instead of just checking if the syscall failed as expected, check as
> well what is the error code returned, to check if it's match the
> expectation and it's failing in the correct error path inside the
> kernel.
>
> Signed-off-by: André Almeida <andrealmeid@...lia.com>
> ---
> This patch is aimed for 6.18
> ---
> .../futex/functional/futex_numa_mpol.c | 36 +++++++++++--------
> 1 file changed, 21 insertions(+), 15 deletions(-)
>
> diff --git a/tools/testing/selftests/futex/functional/futex_numa_mpol.c b/tools/testing/selftests/futex/functional/futex_numa_mpol.c
> index 802c15c82190..c84441751235 100644
> --- a/tools/testing/selftests/futex/functional/futex_numa_mpol.c
> +++ b/tools/testing/selftests/futex/functional/futex_numa_mpol.c
> @@ -77,7 +77,7 @@ static void join_max_threads(void)
> }
> }
>
> -static void __test_futex(void *futex_ptr, int must_fail, unsigned int futex_flags)
> +static void __test_futex(void *futex_ptr, int err_value, unsigned int futex_flags)
> {
> int to_wake, ret, i, need_exit = 0;
>
> @@ -88,11 +88,17 @@ static void __test_futex(void *futex_ptr, int must_fail, unsigned int futex_flag
>
> do {
> ret = futex2_wake(futex_ptr, to_wake, futex_flags);
> - if (must_fail) {
> - if (ret < 0)
> - break;
> - ksft_exit_fail_msg("futex2_wake(%d, 0x%x) should fail, but didn't\n",
> - to_wake, futex_flags);
> +
> + if (err_value) {
> + if (ret >= 0)
> + ksft_exit_fail_msg("futex2_wake(%d, 0x%x) should fail, but didn't\n",
> + to_wake, futex_flags);
> +
> + if (errno != err_value)
> + ksft_exit_fail_msg("futex2_wake(%d, 0x%x) expected error was %d, but returned %d (%s)\n",
> + to_wake, futex_flags, err_value, errno, strerror(errno));
> +
> + break;
If (ret >= 0), the 2nd (errno != err_value) failure message will likely
be printed too. Should we use "else if" so that only one error message
will be printed?
> }
> if (ret < 0) {
> ksft_exit_fail_msg("Failed futex2_wake(%d, 0x%x): %m\n",
> @@ -106,12 +112,12 @@ static void __test_futex(void *futex_ptr, int must_fail, unsigned int futex_flag
> join_max_threads();
>
> for (i = 0; i < MAX_THREADS; i++) {
> - if (must_fail && thread_args[i].result != -1) {
> + if (err_value && thread_args[i].result != -1) {
> ksft_print_msg("Thread %d should fail but succeeded (%d)\n",
> i, thread_args[i].result);
> need_exit = 1;
> }
> - if (!must_fail && thread_args[i].result != 0) {
> + if (!err_value && thread_args[i].result != 0) {
> ksft_print_msg("Thread %d failed (%d)\n", i, thread_args[i].result);
> need_exit = 1;
> }
> @@ -120,14 +126,14 @@ static void __test_futex(void *futex_ptr, int must_fail, unsigned int futex_flag
> ksft_exit_fail_msg("Aborting due to earlier errors.\n");
> }
>
> -static void test_futex(void *futex_ptr, int must_fail)
> +static void test_futex(void *futex_ptr, int err_value)
> {
> - __test_futex(futex_ptr, must_fail, FUTEX2_SIZE_U32 | FUTEX_PRIVATE_FLAG | FUTEX2_NUMA);
> + __test_futex(futex_ptr, err_value, FUTEX2_SIZE_U32 | FUTEX_PRIVATE_FLAG | FUTEX2_NUMA);
> }
>
> -static void test_futex_mpol(void *futex_ptr, int must_fail)
> +static void test_futex_mpol(void *futex_ptr, int err_value)
> {
> - __test_futex(futex_ptr, must_fail, FUTEX2_SIZE_U32 | FUTEX_PRIVATE_FLAG | FUTEX2_NUMA | FUTEX2_MPOL);
> + __test_futex(futex_ptr, err_value, FUTEX2_SIZE_U32 | FUTEX_PRIVATE_FLAG | FUTEX2_NUMA | FUTEX2_MPOL);
> }
>
> static void usage(char *prog)
> @@ -184,16 +190,16 @@ int main(int argc, char *argv[])
>
> /* FUTEX2_NUMA futex must be 8-byte aligned */
> ksft_print_msg("Mis-aligned futex\n");
> - test_futex(futex_ptr + mem_size - 4, 1);
> + test_futex(futex_ptr + mem_size - 4, 22);
>
> futex_numa->numa = FUTEX_NO_NODE;
> mprotect(futex_ptr, mem_size, PROT_READ);
> ksft_print_msg("Memory, RO\n");
> - test_futex(futex_ptr, 1);
> + test_futex(futex_ptr, 14);
>
> mprotect(futex_ptr, mem_size, PROT_NONE);
> ksft_print_msg("Memory, no access\n");
> - test_futex(futex_ptr, 1);
> + test_futex(futex_ptr, 14);
>
> mprotect(futex_ptr, mem_size, PROT_READ | PROT_WRITE);
> ksft_print_msg("Memory back to RW\n");
I believe it is better to use the error number mnemonic (EINVAL &
EFAULT) instead of 22 and 14 as argument to make the code easier to read.
Cheers,
Longman
Powered by blists - more mailing lists