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

[Waiman Long <llong@redhat.com>]

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@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?
>>
>>
>
> 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