Re: [PATCH 1/2] selftests/mm: skip uffd tests in madv_guard if uffd is not present.

[Zi Yan <ziy@nvidia.com>]

On 15 May 2025, at 14:49, Lorenzo Stoakes wrote:

> On Thu, May 15, 2025 at 02:46:41PM -0400, Zi Yan wrote:
>> On 15 May 2025, at 14:41, Lorenzo Stoakes wrote:
>>
>>> Ah you got to this first :) thanks!
>>>
>>> Could you do this with a cover letter though? It's really weird to have 2/2
>>> reply to 1/2, I know sometimes people do that, but it's just odd, and it'd be
>>> good to have an overview, thanks!
>>>
>>> On Thu, May 15, 2025 at 02:23:32PM -0400, Zi Yan wrote:
>>>> When userfaultfd is not compiled into kernel, userfaultfd() returns -1,
>>>> causing uffd tests in madv_guard fail. Skip the tests instead.
>>>
>>> 'madv_guard'? I'd just say the guard_regions.uffd test to fail.
>>
>> Sure. Will change it.
>>>
>>>>
>>>> Signed-off-by: Zi Yan <ziy@nvidia.com>
>
> Given I was being an idiot below, now the patch is fine as-is, just resend
> with the nitty commit message change and cover letter as a v2 and we should
> be good :)

Sure. I am also waiting for Adam's feedback on patch2 and will resend later.

>
> Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
>
>>>> ---
>>>>  tools/testing/selftests/mm/guard-regions.c | 17 +++++++++++++++--
>>>>  1 file changed, 15 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/tools/testing/selftests/mm/guard-regions.c b/tools/testing/selftests/mm/guard-regions.c
>>>> index 0cd9d236649d..93af3d3760f9 100644
>>>> --- a/tools/testing/selftests/mm/guard-regions.c
>>>> +++ b/tools/testing/selftests/mm/guard-regions.c
>>>> @@ -1453,8 +1453,21 @@ TEST_F(guard_regions, uffd)
>>>>
>>>>  	/* Set up uffd. */
>>>>  	uffd = userfaultfd(0);
>>>> -	if (uffd == -1 && errno == EPERM)
>>>> -		ksft_exit_skip("No userfaultfd permissions, try running as root.\n");
>>>
>>> Let's just make this all part of the same switch please!
>>
>> What do you mean? EPERM is handled in the switch-case below.
>
> Oh man, I'm the biggest idiot on Earth haha!
>
> For some reason I read these '-'s as '+'s :))))
>
> Yes please ignore the above, I therefore - like your approach - and am good
> with it.
>

Yeah, I kinda figured when I read your message, but wanted to double check
with you.

>>
>>>
>>> And while I originally used ksft_exit_skip(), I think we can just use the
>>> SKIP(return, ...) form here just fine to keep it consistent.
>>
>> Right. I am using SKIP below, since when I ran it, ksft_exit_skip()
>> makes the whole test message inconsistent.
>
> Yes, your confusion is warranted, I apparently can't read... :>) and
> indeed, agreed.
>
> Thanks for doing this!
>

Thank you for the review. :)

>>
>>>
>>>> +	if (uffd == -1) {
>>>> +		switch (errno) {
>>>> +		case EPERM:
>>>> +			SKIP(return, "No userfaultfd permissions, try running as root.");
>>>> +			break;
>>>> +		case ENOSYS:
>>>> +			SKIP(return, "userfaultfd is not supported/not enabled.");
>>>> +			break;
>>>> +		default:
>>>> +			ksft_exit_fail_msg("userfaultfd failed with %s\n",
>>>> +					   strerror(errno));
>>>> +			break;
>>>> +		}
>>>> +	}
>>>> +
>>>>  	ASSERT_NE(uffd, -1);
>>>>
>>>>  	ASSERT_EQ(ioctl(uffd, UFFDIO_API, &api), 0);
>>>> --
>>>> 2.47.2
>>>>
>>>
>>> Thanks!
>>
>>
>> --
>> Best Regards,
>> Yan, Zi


--
Best Regards,
Yan, Zi