From: Waiman Long <llong@redhat.com>
To: "André Almeida" <andrealmeid@igalia.com>,
"Sebastian Andrzej Siewior" <bigeasy@linutronix.de>,
linux-kernel@vger.kernel.org
Cc: Darren Hart <dvhart@infradead.org>,
Davidlohr Bueso <dave@stgolabs.net>,
Ingo Molnar <mingo@redhat.com>,
Juri Lelli <juri.lelli@redhat.com>,
Peter Zijlstra <peterz@infradead.org>,
Thomas Gleixner <tglx@linutronix.de>,
Valentin Schneider <vschneid@redhat.com>,
Borislav Petkov <bp@alien8.de>,
kernel-dev@igalia.com
Subject: Re: [PATCH 1/2] selftest/futex: Make the error check more precise for futex_numa_mpol
Date: Wed, 3 Sep 2025 13:53:41 -0400 [thread overview]
Message-ID: <1d6a0c1e-5fa2-4c21-b3c1-7bfb2f9dd669@redhat.com> (raw)
In-Reply-To: <20250901203327.53887-1-andrealmeid@igalia.com>
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@igalia.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
next prev parent reply other threads:[~2025-09-03 17:53 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-09-01 20:33 André Almeida
2025-09-01 20:33 ` [PATCH 2/2] selftest/futex: Reintroduce "Memory out of range" numa_mpol's subtest André Almeida
2025-09-03 17:57 ` Waiman Long
2025-09-03 17:53 ` Waiman Long [this message]
2025-09-04 15:23 ` [PATCH 1/2] selftest/futex: Make the error check more precise for futex_numa_mpol André Almeida
2025-09-04 15:59 ` Waiman Long
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1d6a0c1e-5fa2-4c21-b3c1-7bfb2f9dd669@redhat.com \
--to=llong@redhat.com \
--cc=andrealmeid@igalia.com \
--cc=bigeasy@linutronix.de \
--cc=bp@alien8.de \
--cc=dave@stgolabs.net \
--cc=dvhart@infradead.org \
--cc=juri.lelli@redhat.com \
--cc=kernel-dev@igalia.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=tglx@linutronix.de \
--cc=vschneid@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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®