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

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

[Zi Yan]

When userfaultfd is not compiled into kernel, userfaultfd() returns -1,
causing uffd tests in madv_guard fail. Skip the tests instead.

Signed-off-by: Zi Yan <ziy@nvidia.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");
+	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

[PATCH 2/2] selftests/mm: skip hugevm test if kernel config file is not present.

[Zi Yan]

When running hugevm tests in a machine without kernel config present, e.g.,
a VM running a kernel without CONFIG_IKCONFIG_PROC nor /boot/config-*,
skip hugevm tests, which reads kernel config to get page table level
information.

Signed-off-by: Zi Yan <ziy@nvidia.com>
---
 .../selftests/mm/va_high_addr_switch.sh       | 26 +++++++------------
 1 file changed, 9 insertions(+), 17 deletions(-)

diff --git a/tools/testing/selftests/mm/va_high_addr_switch.sh b/tools/testing/selftests/mm/va_high_addr_switch.sh
index 1f92e8caceac..325de53966b6 100755
--- a/tools/testing/selftests/mm/va_high_addr_switch.sh
+++ b/tools/testing/selftests/mm/va_high_addr_switch.sh
@@ -7,23 +7,20 @@
 # real test to check that the kernel is configured to support at least 5
 # pagetable levels.
 
-# 1 means the test failed
-exitcode=1
-
 # Kselftest framework requirement - SKIP code is 4.
 ksft_skip=4
 
-fail()
+skip()
 {
 	echo "$1"
-	exit $exitcode
+	exit $ksft_skip
 }
 
 check_supported_x86_64()
 {
 	local config="/proc/config.gz"
 	[[ -f "${config}" ]] || config="/boot/config-$(uname -r)"
-	[[ -f "${config}" ]] || fail "Cannot find kernel config in /proc or /boot"
+	[[ -f "${config}" ]] || skip "Cannot find kernel config in /proc or /boot"
 
 	# gzip -dcfq automatically handles both compressed and plaintext input.
 	# See man 1 gzip under '-f'.
@@ -33,11 +30,9 @@ check_supported_x86_64()
 		else {print 1}; exit}' /proc/cpuinfo 2>/dev/null)
 
 	if [[ "${pg_table_levels}" -lt 5 ]]; then
-		echo "$0: PGTABLE_LEVELS=${pg_table_levels}, must be >= 5 to run this test"
-		exit $ksft_skip
+		skip "$0: PGTABLE_LEVELS=${pg_table_levels}, must be >= 5 to run this test"
 	elif [[ "${cpu_supports_pl5}" -ne 0 ]]; then
-		echo "$0: CPU does not have the necessary la57 flag to support page table level 5"
-		exit $ksft_skip
+		skip "$0: CPU does not have the necessary la57 flag to support page table level 5"
 	fi
 }
 
@@ -45,24 +40,21 @@ check_supported_ppc64()
 {
 	local config="/proc/config.gz"
 	[[ -f "${config}" ]] || config="/boot/config-$(uname -r)"
-	[[ -f "${config}" ]] || fail "Cannot find kernel config in /proc or /boot"
+	[[ -f "${config}" ]] || skip "Cannot find kernel config in /proc or /boot"
 
 	local pg_table_levels=$(gzip -dcfq "${config}" | grep PGTABLE_LEVELS | cut -d'=' -f 2)
 	if [[ "${pg_table_levels}" -lt 5 ]]; then
-		echo "$0: PGTABLE_LEVELS=${pg_table_levels}, must be >= 5 to run this test"
-		exit $ksft_skip
+		skip "$0: PGTABLE_LEVELS=${pg_table_levels}, must be >= 5 to run this test"
 	fi
 
 	local mmu_support=$(grep -m1 "mmu" /proc/cpuinfo | awk '{print $3}')
 	if [[ "$mmu_support" != "radix" ]]; then
-		echo "$0: System does not use Radix MMU, required for 5-level paging"
-		exit $ksft_skip
+		skip "$0: System does not use Radix MMU, required for 5-level paging"
 	fi
 
 	local hugepages_total=$(awk '/HugePages_Total/ {print $2}' /proc/meminfo)
 	if [[ "${hugepages_total}" -eq 0 ]]; then
-		echo "$0: HugePages are not enabled, required for some tests"
-		exit $ksft_skip
+		skip "$0: HugePages are not enabled, required for some tests"
 	fi
 }
 
-- 
2.47.2

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

[Lorenzo Stoakes]

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.

>
> Signed-off-by: Zi Yan <ziy@nvidia.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!

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.

> +	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!

Re: [PATCH 2/2] selftests/mm: skip hugevm test if kernel config file is not present.

[Lorenzo Stoakes]

On Thu, May 15, 2025 at 02:23:33PM -0400, Zi Yan wrote:
> When running hugevm tests in a machine without kernel config present, e.g.,
> a VM running a kernel without CONFIG_IKCONFIG_PROC nor /boot/config-*,
> skip hugevm tests, which reads kernel config to get page table level
> information.
>
> Signed-off-by: Zi Yan <ziy@nvidia.com>

Looks generally reasonable to me, but I'm not so familiar with this so,

Acked-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

> ---
>  .../selftests/mm/va_high_addr_switch.sh       | 26 +++++++------------
>  1 file changed, 9 insertions(+), 17 deletions(-)
>
> diff --git a/tools/testing/selftests/mm/va_high_addr_switch.sh b/tools/testing/selftests/mm/va_high_addr_switch.sh
> index 1f92e8caceac..325de53966b6 100755
> --- a/tools/testing/selftests/mm/va_high_addr_switch.sh
> +++ b/tools/testing/selftests/mm/va_high_addr_switch.sh
> @@ -7,23 +7,20 @@
>  # real test to check that the kernel is configured to support at least 5
>  # pagetable levels.
>
> -# 1 means the test failed
> -exitcode=1
> -
>  # Kselftest framework requirement - SKIP code is 4.
>  ksft_skip=4
>
> -fail()
> +skip()
>  {
>  	echo "$1"
> -	exit $exitcode
> +	exit $ksft_skip
>  }
>
>  check_supported_x86_64()
>  {
>  	local config="/proc/config.gz"
>  	[[ -f "${config}" ]] || config="/boot/config-$(uname -r)"
> -	[[ -f "${config}" ]] || fail "Cannot find kernel config in /proc or /boot"
> +	[[ -f "${config}" ]] || skip "Cannot find kernel config in /proc or /boot"
>
>  	# gzip -dcfq automatically handles both compressed and plaintext input.
>  	# See man 1 gzip under '-f'.
> @@ -33,11 +30,9 @@ check_supported_x86_64()
>  		else {print 1}; exit}' /proc/cpuinfo 2>/dev/null)
>
>  	if [[ "${pg_table_levels}" -lt 5 ]]; then
> -		echo "$0: PGTABLE_LEVELS=${pg_table_levels}, must be >= 5 to run this test"
> -		exit $ksft_skip
> +		skip "$0: PGTABLE_LEVELS=${pg_table_levels}, must be >= 5 to run this test"
>  	elif [[ "${cpu_supports_pl5}" -ne 0 ]]; then
> -		echo "$0: CPU does not have the necessary la57 flag to support page table level 5"
> -		exit $ksft_skip
> +		skip "$0: CPU does not have the necessary la57 flag to support page table level 5"
>  	fi
>  }
>
> @@ -45,24 +40,21 @@ check_supported_ppc64()
>  {
>  	local config="/proc/config.gz"
>  	[[ -f "${config}" ]] || config="/boot/config-$(uname -r)"
> -	[[ -f "${config}" ]] || fail "Cannot find kernel config in /proc or /boot"
> +	[[ -f "${config}" ]] || skip "Cannot find kernel config in /proc or /boot"
>
>  	local pg_table_levels=$(gzip -dcfq "${config}" | grep PGTABLE_LEVELS | cut -d'=' -f 2)
>  	if [[ "${pg_table_levels}" -lt 5 ]]; then
> -		echo "$0: PGTABLE_LEVELS=${pg_table_levels}, must be >= 5 to run this test"
> -		exit $ksft_skip
> +		skip "$0: PGTABLE_LEVELS=${pg_table_levels}, must be >= 5 to run this test"
>  	fi
>
>  	local mmu_support=$(grep -m1 "mmu" /proc/cpuinfo | awk '{print $3}')
>  	if [[ "$mmu_support" != "radix" ]]; then
> -		echo "$0: System does not use Radix MMU, required for 5-level paging"
> -		exit $ksft_skip
> +		skip "$0: System does not use Radix MMU, required for 5-level paging"
>  	fi
>
>  	local hugepages_total=$(awk '/HugePages_Total/ {print $2}' /proc/meminfo)
>  	if [[ "${hugepages_total}" -eq 0 ]]; then
> -		echo "$0: HugePages are not enabled, required for some tests"
> -		exit $ksft_skip
> +		skip "$0: HugePages are not enabled, required for some tests"
>  	fi
>  }
>
> --
> 2.47.2
>

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

[Zi Yan]

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>
>> ---
>>  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.

>
> 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.

>
>> +	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

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

[Lorenzo Stoakes]

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 :)

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.

>
> >
> > 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!

>
> >
> >> +	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

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

[Zi Yan]

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

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

[Pedro Falcato]

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.
> 
> Signed-off-by: Zi Yan <ziy@nvidia.com>

Reviewed-by: Pedro Falcato <pfalcato@suse.de>

Thanks!

-- 
Pedro

Re: [PATCH 2/2] selftests/mm: skip hugevm test if kernel config file is not present.

[Pedro Falcato]

On Thu, May 15, 2025 at 07:43:40PM +0100, Lorenzo Stoakes wrote:
> On Thu, May 15, 2025 at 02:23:33PM -0400, Zi Yan wrote:
> > When running hugevm tests in a machine without kernel config present, e.g.,
> > a VM running a kernel without CONFIG_IKCONFIG_PROC nor /boot/config-*,
> > skip hugevm tests, which reads kernel config to get page table level
> > information.
> >
> > Signed-off-by: Zi Yan <ziy@nvidia.com>
> 
> Looks generally reasonable to me, but I'm not so familiar with this so,
> 
> Acked-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

Acked-by: Pedro Falcato <pfalcato@suse.de>

Same here. Although, despite this being worth patching, I do think we should
document somewhere what the expectations are for mm selftests (in terms of
kernel config, environment, libc, possibly even utilities present).

-- 
Pedro