Re: [PATCH 1/2] selftest/futex: Make the error check more precise for futex_numa_mpol

[Waiman Long]

On 9/4/25 11:23 AM, André Almeida wrote:
> Hi Waiman,
>
> Thanks for the feedback!
>
> Em 03/09/2025 14:53, Waiman Long escreveu:
> > 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@xxxxxxxxxx>
> > > ---
> > > 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?
>
> ksft_exit_fail_msg() calls exit(), so the code will exit before 
> executing the second failure message.
>
> If this was a  ksft_test_result_error() call, then the message would 
> be printed twice.

I didn't realize that. Thanks for letting me know. In that case, the 
code should be OK.

Cheers,
Longman