[PATCH v3 0/5] selftests/mm: Improve run_vmtests.sh

[PATCH v3 0/5] selftests/mm: Improve run_vmtests.sh

[Muhammad Usama Anjum]

In this series, I'm trying to add 3 missing tests to vm_runtests.sh
which is used to run all the tests in mm suite. These tests weren't
running by CIs. While enabling them and through review feedback, I've
fixed some problems in tests as well. I've found more flakiness in more
tests which I'll be fixing with future patches.

hugetlb-read-hwpoison test is being added where it can only run with
newly added "-d" (destructive) flag only. Not sure why it is failing
again. So once it become stable, we can think of moving it to default
set of tests if it doesn't have any side-effect to them.

Cc: Ryan Roberts <ryan.roberts@arm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
Changes in v3:
- Add cover letter
- Fix flakiness in tests found during enablement
- Move additional tests down in the file
- Add "-d" option which poisons the pages and aren't being useable after
  the test

v2: https://lore.kernel.org/all/20240123073615.920324-1-usama.anjum@collabora.com

Muhammad Usama Anjum (5):
  selftests/mm: hugetlb_reparenting_test: do not unmount
  selftests/mm: run_vmtests: remove sudo and conform to tap
  selftests/mm: save and restore nr_hugepages value
  selftests/mm: protection_keys: save/restore nr_hugepages settings
  selftests/mm: run_vmtests.sh: add missing tests

 tools/testing/selftests/mm/Makefile           |  5 +++
 .../selftests/mm/charge_reserved_hugetlb.sh   |  4 +++
 .../selftests/mm/hugetlb_reparenting_test.sh  |  9 +++--
 tools/testing/selftests/mm/on-fault-limit.c   | 36 +++++++++----------
 tools/testing/selftests/mm/protection_keys.c  | 34 ++++++++++++++++++
 tools/testing/selftests/mm/run_vmtests.sh     | 10 +++++-
 6 files changed, 76 insertions(+), 22 deletions(-)

-- 
2.42.0

[PATCH v3 1/5] selftests/mm: hugetlb_reparenting_test: do not unmount

[Muhammad Usama Anjum]

Do not unmount the cgroup if it wasn't mounted by the test. The earlier
patch had fixed this for charge_reserved_hugetlb, but not for this test.
I'm adding fixes tag to that earlier patch.

Fixes: 209376ed2a84 ("selftests/vm: make charge_reserved_hugetlb.sh work with existing cgroup setting")
Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
 tools/testing/selftests/mm/hugetlb_reparenting_test.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/mm/hugetlb_reparenting_test.sh b/tools/testing/selftests/mm/hugetlb_reparenting_test.sh
index 14d26075c8635..615c4d766c909 100755
--- a/tools/testing/selftests/mm/hugetlb_reparenting_test.sh
+++ b/tools/testing/selftests/mm/hugetlb_reparenting_test.sh
@@ -248,5 +248,7 @@ cleanup
 
 echo ALL PASS
 
-umount $CGROUP_ROOT
-rm -rf $CGROUP_ROOT
+if [[ $do_umount ]]; then
+  umount $CGROUP_ROOT
+  rm -rf $CGROUP_ROOT
+fi
-- 
2.42.0

[PATCH v3 2/5] selftests/mm: run_vmtests: remove sudo and conform to tap

[Muhammad Usama Anjum]

Remove sudo as some test running environments may not have sudo
available. Instead skip the test if root privileges aren't available in
the test.

Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
Changes since v1:
- Added this patch in v2

We are allocating 2*RLIMIT_MEMLOCK.rlim_max memory and mmap() isn't
failing. This seems like true bug in the kernel. Even the root user
shouldn't be able to allocate more memory than allowed MEMLOCKed memory.
Any ideas?
---
 tools/testing/selftests/mm/on-fault-limit.c | 36 ++++++++++-----------
 tools/testing/selftests/mm/run_vmtests.sh   |  2 +-
 2 files changed, 18 insertions(+), 20 deletions(-)

diff --git a/tools/testing/selftests/mm/on-fault-limit.c b/tools/testing/selftests/mm/on-fault-limit.c
index b5888d613f34e..0ea98ffab3589 100644
--- a/tools/testing/selftests/mm/on-fault-limit.c
+++ b/tools/testing/selftests/mm/on-fault-limit.c
@@ -5,40 +5,38 @@
 #include <string.h>
 #include <sys/time.h>
 #include <sys/resource.h>
+#include "../kselftest.h"
 
-static int test_limit(void)
+static void test_limit(void)
 {
-	int ret = 1;
 	struct rlimit lims;
 	void *map;
 
-	if (getrlimit(RLIMIT_MEMLOCK, &lims)) {
-		perror("getrlimit");
-		return ret;
-	}
+	if (getrlimit(RLIMIT_MEMLOCK, &lims))
+		ksft_exit_fail_msg("getrlimit: %s\n", strerror(errno));
 
-	if (mlockall(MCL_ONFAULT | MCL_FUTURE)) {
-		perror("mlockall");
-		return ret;
-	}
+	if (mlockall(MCL_ONFAULT | MCL_FUTURE))
+		ksft_exit_fail_msg("mlockall: %s\n", strerror(errno));
 
 	map = mmap(NULL, 2 * lims.rlim_max, PROT_READ | PROT_WRITE,
 		   MAP_PRIVATE | MAP_ANONYMOUS | MAP_POPULATE, -1, 0);
+
+	ksft_test_result(map == MAP_FAILED, "Failed mmap\n");
+
 	if (map != MAP_FAILED)
-		printf("mmap should have failed, but didn't\n");
-	else {
-		ret = 0;
 		munmap(map, 2 * lims.rlim_max);
-	}
-
 	munlockall();
-	return ret;
 }
 
 int main(int argc, char **argv)
 {
-	int ret = 0;
+	ksft_print_header();
+	ksft_set_plan(1);
+
+	if (getuid())
+		ksft_test_result_skip("Require root privileges to run\n");
+	else
+		test_limit();
 
-	ret += test_limit();
-	return ret;
+	ksft_finished();
 }
diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh
index 246d53a5d7f28..e373d592dbf5c 100755
--- a/tools/testing/selftests/mm/run_vmtests.sh
+++ b/tools/testing/selftests/mm/run_vmtests.sh
@@ -291,7 +291,7 @@ echo "$nr_hugepgs" > /proc/sys/vm/nr_hugepages
 
 CATEGORY="compaction" run_test ./compaction_test
 
-CATEGORY="mlock" run_test sudo -u nobody ./on-fault-limit
+CATEGORY="mlock" run_test ./on-fault-limit
 
 CATEGORY="mmap" run_test ./map_populate
 
-- 
2.42.0

[PATCH v3 3/5] selftests/mm: save and restore nr_hugepages value

[Muhammad Usama Anjum]

Save and restore nr_hugepages before changing it during the test. A test
should not change system wide settings.

Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
 tools/testing/selftests/mm/charge_reserved_hugetlb.sh  | 4 ++++
 tools/testing/selftests/mm/hugetlb_reparenting_test.sh | 3 +++
 2 files changed, 7 insertions(+)

diff --git a/tools/testing/selftests/mm/charge_reserved_hugetlb.sh b/tools/testing/selftests/mm/charge_reserved_hugetlb.sh
index e14bdd4455f2d..d680c00d2853a 100755
--- a/tools/testing/selftests/mm/charge_reserved_hugetlb.sh
+++ b/tools/testing/selftests/mm/charge_reserved_hugetlb.sh
@@ -11,6 +11,8 @@ if [[ $(id -u) -ne 0 ]]; then
   exit $ksft_skip
 fi
 
+nr_hugepgs=$(cat /proc/sys/vm/nr_hugepages)
+
 fault_limit_file=limit_in_bytes
 reservation_limit_file=rsvd.limit_in_bytes
 fault_usage_file=usage_in_bytes
@@ -582,3 +584,5 @@ if [[ $do_umount ]]; then
   umount $cgroup_path
   rmdir $cgroup_path
 fi
+
+echo "$nr_hugepgs" > /proc/sys/vm/nr_hugepages
diff --git a/tools/testing/selftests/mm/hugetlb_reparenting_test.sh b/tools/testing/selftests/mm/hugetlb_reparenting_test.sh
index 615c4d766c909..11f9bbe7dc222 100755
--- a/tools/testing/selftests/mm/hugetlb_reparenting_test.sh
+++ b/tools/testing/selftests/mm/hugetlb_reparenting_test.sh
@@ -11,6 +11,7 @@ if [[ $(id -u) -ne 0 ]]; then
   exit $ksft_skip
 fi
 
+nr_hugepgs=$(cat /proc/sys/vm/nr_hugepages)
 usage_file=usage_in_bytes
 
 if [[ "$1" == "-cgroup-v2" ]]; then
@@ -252,3 +253,5 @@ if [[ $do_umount ]]; then
   umount $CGROUP_ROOT
   rm -rf $CGROUP_ROOT
 fi
+
+echo "$nr_hugepgs" > /proc/sys/vm/nr_hugepages
-- 
2.42.0

[PATCH v3 4/5] selftests/mm: protection_keys: save/restore nr_hugepages settings

[Muhammad Usama Anjum]

Save and restore nr_hugepages before changing it during the test. A test
should not change system wide settings.

Fixes: 5f23f6d082a9 ("x86/pkeys: Add self-tests")
Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
 tools/testing/selftests/mm/protection_keys.c | 34 ++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/tools/testing/selftests/mm/protection_keys.c b/tools/testing/selftests/mm/protection_keys.c
index 48dc151f8fca8..f822ae31af22e 100644
--- a/tools/testing/selftests/mm/protection_keys.c
+++ b/tools/testing/selftests/mm/protection_keys.c
@@ -54,6 +54,7 @@ int test_nr;
 u64 shadow_pkey_reg;
 int dprint_in_signal;
 char dprint_in_signal_buffer[DPRINT_IN_SIGNAL_BUF_SIZE];
+char buf[256];
 
 void cat_into_file(char *str, char *file)
 {
@@ -1744,6 +1745,38 @@ void pkey_setup_shadow(void)
 	shadow_pkey_reg = __read_pkey_reg();
 }
 
+void restore_settings_atexit(void)
+{
+	cat_into_file(buf, "/proc/sys/vm/nr_hugepages");
+}
+
+void save_settings(void)
+{
+	int fd;
+	int err;
+
+	if (geteuid())
+		return;
+
+	fd = open("/proc/sys/vm/nr_hugepages", O_RDONLY);
+	if (fd < 0) {
+		fprintf(stderr, "error opening\n");
+		perror("error: ");
+		exit(__LINE__);
+	}
+
+	/* -1 to guarantee leaving the trailing \0 */
+	err = read(fd, buf, sizeof(buf)-1);
+	if (err < 0) {
+		fprintf(stderr, "error reading\n");
+		perror("error: ");
+		exit(__LINE__);
+	}
+
+	atexit(restore_settings_atexit);
+	close(fd);
+}
+
 int main(void)
 {
 	int nr_iterations = 22;
@@ -1751,6 +1784,7 @@ int main(void)
 
 	srand((unsigned int)time(NULL));
 
+	save_settings();
 	setup_handlers();
 
 	printf("has pkeys: %d\n", pkeys_supported);
-- 
2.42.0

[PATCH v3 5/5] selftests/mm: run_vmtests.sh: add missing tests

[Muhammad Usama Anjum]

Add missing tests to run_vmtests.sh. The mm kselftests are run through
run_vmtests.sh. If a test isn't present in this script, it'll not run
with run_tests or `make -C tools/testing/selftests/mm run_tests`.

Cc: Ryan Roberts <ryan.roberts@arm.com>
Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
Changes since v1:
- Copy the original scripts and their dependence script to install directory as well

Changes since v2:
- Add a comment
- Move tests down in the file
- Add "-d" option which poisons the pages and aren't being useable after
  the test
---
 tools/testing/selftests/mm/Makefile       | 5 +++++
 tools/testing/selftests/mm/run_vmtests.sh | 8 ++++++++
 2 files changed, 13 insertions(+)

diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
index 2453add65d12f..f3aec7be80730 100644
--- a/tools/testing/selftests/mm/Makefile
+++ b/tools/testing/selftests/mm/Makefile
@@ -114,6 +114,11 @@ TEST_PROGS := run_vmtests.sh
 TEST_FILES := test_vmalloc.sh
 TEST_FILES += test_hmm.sh
 TEST_FILES += va_high_addr_switch.sh
+TEST_FILES += charge_reserved_hugetlb.sh
+TEST_FILES += hugetlb_reparenting_test.sh
+
+# required by charge_reserved_hugetlb.sh
+TEST_FILES += write_hugetlb_memory.sh
 
 include ../lib.mk
 
diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh
index e373d592dbf5c..a0f37e4438937 100755
--- a/tools/testing/selftests/mm/run_vmtests.sh
+++ b/tools/testing/selftests/mm/run_vmtests.sh
@@ -19,6 +19,7 @@ usage: ${BASH_SOURCE[0]:-$0} [ options ]
   -t: specify specific categories to tests to run
   -h: display this message
   -n: disable TAP output
+  -d: run destructive tests
 
 The default behavior is to run required tests only.  If -a is specified,
 will run all tests.
@@ -79,6 +80,7 @@ EOF
 }
 
 RUN_ALL=false
+RUN_DESTRUCTIVE_TEST=false
 TAP_PREFIX="# "
 
 while getopts "aht:n" OPT; do
@@ -87,6 +89,7 @@ while getopts "aht:n" OPT; do
 		"h") usage ;;
 		"t") VM_SELFTEST_ITEMS=${OPTARG} ;;
 		"n") TAP_PREFIX= ;;
+		"a") RUN_DESTRUCTIVE_TEST=true ;;
 	esac
 done
 shift $((OPTIND -1))
@@ -304,6 +307,11 @@ CATEGORY="process_mrelease" run_test ./mrelease_test
 CATEGORY="mremap" run_test ./mremap_test
 
 CATEGORY="hugetlb" run_test ./thuge-gen
+CATEGORY="hugetlb" run_test ./charge_reserved_hugetlb.sh -cgroup-v2
+CATEGORY="hugetlb" run_test ./hugetlb_reparenting_test.sh -cgroup-v2
+if $RUN_DESTRUCTIVE_TEST; then
+CATEGORY="hugetlb" run_test ./hugetlb-read-hwpoison
+fi
 
 if [ $VADDR64 -ne 0 ]; then
 
-- 
2.42.0

Re: [PATCH v3 2/5] selftests/mm: run_vmtests: remove sudo and conform to tap

[Ryan Roberts]

On 25/01/2024 15:46, Muhammad Usama Anjum wrote:
> Remove sudo as some test running environments may not have sudo
> available. Instead skip the test if root privileges aren't available in
> the test.
> 
> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> ---
> Changes since v1:
> - Added this patch in v2
> 
> We are allocating 2*RLIMIT_MEMLOCK.rlim_max memory and mmap() isn't
> failing. This seems like true bug in the kernel. Even the root user
> shouldn't be able to allocate more memory than allowed MEMLOCKed memory.
> Any ideas?
> ---
>  tools/testing/selftests/mm/on-fault-limit.c | 36 ++++++++++-----------
>  tools/testing/selftests/mm/run_vmtests.sh   |  2 +-
>  2 files changed, 18 insertions(+), 20 deletions(-)
> 
> diff --git a/tools/testing/selftests/mm/on-fault-limit.c b/tools/testing/selftests/mm/on-fault-limit.c
> index b5888d613f34e..0ea98ffab3589 100644
> --- a/tools/testing/selftests/mm/on-fault-limit.c
> +++ b/tools/testing/selftests/mm/on-fault-limit.c
> @@ -5,40 +5,38 @@
>  #include <string.h>
>  #include <sys/time.h>
>  #include <sys/resource.h>
> +#include "../kselftest.h"
>  
> -static int test_limit(void)
> +static void test_limit(void)
>  {
> -	int ret = 1;
>  	struct rlimit lims;
>  	void *map;
>  
> -	if (getrlimit(RLIMIT_MEMLOCK, &lims)) {
> -		perror("getrlimit");
> -		return ret;
> -	}
> +	if (getrlimit(RLIMIT_MEMLOCK, &lims))
> +		ksft_exit_fail_msg("getrlimit: %s\n", strerror(errno));
>  
> -	if (mlockall(MCL_ONFAULT | MCL_FUTURE)) {
> -		perror("mlockall");
> -		return ret;
> -	}
> +	if (mlockall(MCL_ONFAULT | MCL_FUTURE))
> +		ksft_exit_fail_msg("mlockall: %s\n", strerror(errno));
>  
>  	map = mmap(NULL, 2 * lims.rlim_max, PROT_READ | PROT_WRITE,
>  		   MAP_PRIVATE | MAP_ANONYMOUS | MAP_POPULATE, -1, 0);
> +
> +	ksft_test_result(map == MAP_FAILED, "Failed mmap\n");
> +
>  	if (map != MAP_FAILED)
> -		printf("mmap should have failed, but didn't\n");
> -	else {
> -		ret = 0;
>  		munmap(map, 2 * lims.rlim_max);
> -	}
> -
>  	munlockall();
> -	return ret;
>  }
>  
>  int main(int argc, char **argv)
>  {
> -	int ret = 0;
> +	ksft_print_header();
> +	ksft_set_plan(1);
> +
> +	if (getuid())
> +		ksft_test_result_skip("Require root privileges to run\n");
> +	else
> +		test_limit();
>  
> -	ret += test_limit();
> -	return ret;
> +	ksft_finished();
>  }
> diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh
> index 246d53a5d7f28..e373d592dbf5c 100755
> --- a/tools/testing/selftests/mm/run_vmtests.sh
> +++ b/tools/testing/selftests/mm/run_vmtests.sh
> @@ -291,7 +291,7 @@ echo "$nr_hugepgs" > /proc/sys/vm/nr_hugepages
>  
>  CATEGORY="compaction" run_test ./compaction_test
>  
> -CATEGORY="mlock" run_test sudo -u nobody ./on-fault-limit
> +CATEGORY="mlock" run_test ./on-fault-limit

I think changing this is going to give unintended results. run_vmtests.sh must
already be running as root. "sudo -u nobody" is deprivieging the test to run as
nobody. The rlimit is not enforced for root so this test must run unprivileged
to work. See man page for getrlimit():

  Since Linux 2.6.9, no limits are placed on the amount of memory that a
  privileged process may lock, and this limit instead governs the amount of
  memory that an unprivileged process may lock

So I think the correct fix is actually to install sudo on your CI.

>  
>  CATEGORY="mmap" run_test ./map_populate
>  

Re: [PATCH v3 5/5] selftests/mm: run_vmtests.sh: add missing tests

[Ryan Roberts]

On 25/01/2024 15:46, Muhammad Usama Anjum wrote:
> Add missing tests to run_vmtests.sh. The mm kselftests are run through
> run_vmtests.sh. If a test isn't present in this script, it'll not run
> with run_tests or `make -C tools/testing/selftests/mm run_tests`.
> 
> Cc: Ryan Roberts <ryan.roberts@arm.com>
> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> ---
> Changes since v1:
> - Copy the original scripts and their dependence script to install directory as well
> 
> Changes since v2:
> - Add a comment
> - Move tests down in the file
> - Add "-d" option which poisons the pages and aren't being useable after
>   the test
> ---
>  tools/testing/selftests/mm/Makefile       | 5 +++++
>  tools/testing/selftests/mm/run_vmtests.sh | 8 ++++++++
>  2 files changed, 13 insertions(+)
> 
> diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
> index 2453add65d12f..f3aec7be80730 100644
> --- a/tools/testing/selftests/mm/Makefile
> +++ b/tools/testing/selftests/mm/Makefile
> @@ -114,6 +114,11 @@ TEST_PROGS := run_vmtests.sh
>  TEST_FILES := test_vmalloc.sh
>  TEST_FILES += test_hmm.sh
>  TEST_FILES += va_high_addr_switch.sh
> +TEST_FILES += charge_reserved_hugetlb.sh
> +TEST_FILES += hugetlb_reparenting_test.sh
> +
> +# required by charge_reserved_hugetlb.sh
> +TEST_FILES += write_hugetlb_memory.sh
>  
>  include ../lib.mk
>  
> diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh
> index e373d592dbf5c..a0f37e4438937 100755
> --- a/tools/testing/selftests/mm/run_vmtests.sh
> +++ b/tools/testing/selftests/mm/run_vmtests.sh
> @@ -19,6 +19,7 @@ usage: ${BASH_SOURCE[0]:-$0} [ options ]
>    -t: specify specific categories to tests to run
>    -h: display this message
>    -n: disable TAP output
> +  -d: run destructive tests

You probably want to clarify the behaviour for -a (all). I guess providing -a
should NOT run destructive tests unless -d is also explicitly provided.

>  
>  The default behavior is to run required tests only.  If -a is specified,
>  will run all tests.
> @@ -79,6 +80,7 @@ EOF
>  }
>  
>  RUN_ALL=false
> +RUN_DESTRUCTIVE_TEST=false

Either call this RUN_DESTRUCTIVE (my preference) or at least make it plural
(RUN_DESTRUCTIVE_TESTS).

>  TAP_PREFIX="# "
>  
>  while getopts "aht:n" OPT; do
> @@ -87,6 +89,7 @@ while getopts "aht:n" OPT; do
>  		"h") usage ;;
>  		"t") VM_SELFTEST_ITEMS=${OPTARG} ;;
>  		"n") TAP_PREFIX= ;;
> +		"a") RUN_DESTRUCTIVE_TEST=true ;;

The help you added says the option is -d, but this is looking for -a, and
conflicting with the existing -a=all option.

>  	esac
>  done
>  shift $((OPTIND -1))
> @@ -304,6 +307,11 @@ CATEGORY="process_mrelease" run_test ./mrelease_test
>  CATEGORY="mremap" run_test ./mremap_test
>  
>  CATEGORY="hugetlb" run_test ./thuge-gen
> +CATEGORY="hugetlb" run_test ./charge_reserved_hugetlb.sh -cgroup-v2
> +CATEGORY="hugetlb" run_test ./hugetlb_reparenting_test.sh -cgroup-v2
> +if $RUN_DESTRUCTIVE_TEST; then
> +CATEGORY="hugetlb" run_test ./hugetlb-read-hwpoison
> +fi
>  
>  if [ $VADDR64 -ne 0 ]; then
>  

Re: [PATCH v3 2/5] selftests/mm: run_vmtests: remove sudo and conform to tap

[Muhammad Usama Anjum]

On 2/1/24 5:04 PM, Ryan Roberts wrote:
> On 25/01/2024 15:46, Muhammad Usama Anjum wrote:
>> Remove sudo as some test running environments may not have sudo
>> available. Instead skip the test if root privileges aren't available in
>> the test.
>>
>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>> ---
>> Changes since v1:
>> - Added this patch in v2
>>
>> We are allocating 2*RLIMIT_MEMLOCK.rlim_max memory and mmap() isn't
>> failing. This seems like true bug in the kernel. Even the root user
>> shouldn't be able to allocate more memory than allowed MEMLOCKed memory.
>> Any ideas?
>> ---
>>  tools/testing/selftests/mm/on-fault-limit.c | 36 ++++++++++-----------
>>  tools/testing/selftests/mm/run_vmtests.sh   |  2 +-
>>  2 files changed, 18 insertions(+), 20 deletions(-)
>>
>> diff --git a/tools/testing/selftests/mm/on-fault-limit.c b/tools/testing/selftests/mm/on-fault-limit.c
>> index b5888d613f34e..0ea98ffab3589 100644
>> --- a/tools/testing/selftests/mm/on-fault-limit.c
>> +++ b/tools/testing/selftests/mm/on-fault-limit.c
>> @@ -5,40 +5,38 @@
>>  #include <string.h>
>>  #include <sys/time.h>
>>  #include <sys/resource.h>
>> +#include "../kselftest.h"
>>  
>> -static int test_limit(void)
>> +static void test_limit(void)
>>  {
>> -	int ret = 1;
>>  	struct rlimit lims;
>>  	void *map;
>>  
>> -	if (getrlimit(RLIMIT_MEMLOCK, &lims)) {
>> -		perror("getrlimit");
>> -		return ret;
>> -	}
>> +	if (getrlimit(RLIMIT_MEMLOCK, &lims))
>> +		ksft_exit_fail_msg("getrlimit: %s\n", strerror(errno));
>>  
>> -	if (mlockall(MCL_ONFAULT | MCL_FUTURE)) {
>> -		perror("mlockall");
>> -		return ret;
>> -	}
>> +	if (mlockall(MCL_ONFAULT | MCL_FUTURE))
>> +		ksft_exit_fail_msg("mlockall: %s\n", strerror(errno));
>>  
>>  	map = mmap(NULL, 2 * lims.rlim_max, PROT_READ | PROT_WRITE,
>>  		   MAP_PRIVATE | MAP_ANONYMOUS | MAP_POPULATE, -1, 0);
>> +
>> +	ksft_test_result(map == MAP_FAILED, "Failed mmap\n");
>> +
>>  	if (map != MAP_FAILED)
>> -		printf("mmap should have failed, but didn't\n");
>> -	else {
>> -		ret = 0;
>>  		munmap(map, 2 * lims.rlim_max);
>> -	}
>> -
>>  	munlockall();
>> -	return ret;
>>  }
>>  
>>  int main(int argc, char **argv)
>>  {
>> -	int ret = 0;
>> +	ksft_print_header();
>> +	ksft_set_plan(1);
>> +
>> +	if (getuid())
>> +		ksft_test_result_skip("Require root privileges to run\n");
I'd sent a patch to fix this behavior today. This test should run without
root privileges.
https://lore.kernel.org/all/20240201071307.592317-1-usama.anjum@collabora.com

>> +	else
>> +		test_limit();
>>  
>> -	ret += test_limit();
>> -	return ret;
>> +	ksft_finished();
>>  }
>> diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh
>> index 246d53a5d7f28..e373d592dbf5c 100755
>> --- a/tools/testing/selftests/mm/run_vmtests.sh
>> +++ b/tools/testing/selftests/mm/run_vmtests.sh
>> @@ -291,7 +291,7 @@ echo "$nr_hugepgs" > /proc/sys/vm/nr_hugepages
>>  
>>  CATEGORY="compaction" run_test ./compaction_test
>>  
>> -CATEGORY="mlock" run_test sudo -u nobody ./on-fault-limit
>> +CATEGORY="mlock" run_test ./on-fault-limit
> 
> I think changing this is going to give unintended results. run_vmtests.sh must
> already be running as root. "sudo -u nobody" is deprivieging the test to run as
> nobody. The rlimit is not enforced for root so this test must run unprivileged
> to work. See man page for getrlimit():
> 
>   Since Linux 2.6.9, no limits are placed on the amount of memory that a
>   privileged process may lock, and this limit instead governs the amount of
>   memory that an unprivileged process may lock
> 
> So I think the correct fix is actually to install sudo on your CI.
run_vmtests.sh is invoked without sudo with following:
make -C tools/testing/selftests/mm run_tests

Installing sudo in rootfs wouldn't be trivial enough on the CI side.
Alternatively, we can check if sudo is present before executing this test
to avoid error that sudo isn't found.

> 
>>  
>>  CATEGORY="mmap" run_test ./map_populate
>>  
> 
> 

-- 
BR,
Muhammad Usama Anjum

Re: [PATCH v3 5/5] selftests/mm: run_vmtests.sh: add missing tests

[Muhammad Usama Anjum]

On 2/1/24 5:11 PM, Ryan Roberts wrote:
> On 25/01/2024 15:46, Muhammad Usama Anjum wrote:
>> Add missing tests to run_vmtests.sh. The mm kselftests are run through
>> run_vmtests.sh. If a test isn't present in this script, it'll not run
>> with run_tests or `make -C tools/testing/selftests/mm run_tests`.
>>
>> Cc: Ryan Roberts <ryan.roberts@arm.com>
>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>> ---
>> Changes since v1:
>> - Copy the original scripts and their dependence script to install directory as well
>>
>> Changes since v2:
>> - Add a comment
>> - Move tests down in the file
>> - Add "-d" option which poisons the pages and aren't being useable after
>>   the test
>> ---
>>  tools/testing/selftests/mm/Makefile       | 5 +++++
>>  tools/testing/selftests/mm/run_vmtests.sh | 8 ++++++++
>>  2 files changed, 13 insertions(+)
>>
>> diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
>> index 2453add65d12f..f3aec7be80730 100644
>> --- a/tools/testing/selftests/mm/Makefile
>> +++ b/tools/testing/selftests/mm/Makefile
>> @@ -114,6 +114,11 @@ TEST_PROGS := run_vmtests.sh
>>  TEST_FILES := test_vmalloc.sh
>>  TEST_FILES += test_hmm.sh
>>  TEST_FILES += va_high_addr_switch.sh
>> +TEST_FILES += charge_reserved_hugetlb.sh
>> +TEST_FILES += hugetlb_reparenting_test.sh
>> +
>> +# required by charge_reserved_hugetlb.sh
>> +TEST_FILES += write_hugetlb_memory.sh
>>  
>>  include ../lib.mk
>>  
>> diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh
>> index e373d592dbf5c..a0f37e4438937 100755
>> --- a/tools/testing/selftests/mm/run_vmtests.sh
>> +++ b/tools/testing/selftests/mm/run_vmtests.sh
>> @@ -19,6 +19,7 @@ usage: ${BASH_SOURCE[0]:-$0} [ options ]
>>    -t: specify specific categories to tests to run
>>    -h: display this message
>>    -n: disable TAP output
>> +  -d: run destructive tests
> 
> You probably want to clarify the behaviour for -a (all). I guess providing -a
> should NOT run destructive tests unless -d is also explicitly provided.
> 
>>  
>>  The default behavior is to run required tests only.  If -a is specified,
>>  will run all tests.
>> @@ -79,6 +80,7 @@ EOF
>>  }
>>  
>>  RUN_ALL=false
>> +RUN_DESTRUCTIVE_TEST=false
> 
> Either call this RUN_DESTRUCTIVE (my preference) or at least make it plural
> (RUN_DESTRUCTIVE_TESTS).
> 
>>  TAP_PREFIX="# "
>>  
>>  while getopts "aht:n" OPT; do
>> @@ -87,6 +89,7 @@ while getopts "aht:n" OPT; do
>>  		"h") usage ;;
>>  		"t") VM_SELFTEST_ITEMS=${OPTARG} ;;
>>  		"n") TAP_PREFIX= ;;
>> +		"a") RUN_DESTRUCTIVE_TEST=true ;;
> 
> The help you added says the option is -d, but this is looking for -a, and
> conflicting with the existing -a=all option.
Sorry, that's a typo. I'll resolve your above comments with fix patch as well.

> 
>>  	esac
>>  done
>>  shift $((OPTIND -1))
>> @@ -304,6 +307,11 @@ CATEGORY="process_mrelease" run_test ./mrelease_test
>>  CATEGORY="mremap" run_test ./mremap_test
>>  
>>  CATEGORY="hugetlb" run_test ./thuge-gen
>> +CATEGORY="hugetlb" run_test ./charge_reserved_hugetlb.sh -cgroup-v2
>> +CATEGORY="hugetlb" run_test ./hugetlb_reparenting_test.sh -cgroup-v2
>> +if $RUN_DESTRUCTIVE_TEST; then
>> +CATEGORY="hugetlb" run_test ./hugetlb-read-hwpoison
>> +fi
>>  
>>  if [ $VADDR64 -ne 0 ]; then
>>  
> 
> 

-- 
BR,
Muhammad Usama Anjum

Re: [PATCH v3 2/5] selftests/mm: run_vmtests: remove sudo and conform to tap

[Ryan Roberts]

On 01/02/2024 12:24, Muhammad Usama Anjum wrote:
> On 2/1/24 5:04 PM, Ryan Roberts wrote:
>> On 25/01/2024 15:46, Muhammad Usama Anjum wrote:
>>> Remove sudo as some test running environments may not have sudo
>>> available. Instead skip the test if root privileges aren't available in
>>> the test.
>>>
>>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>>> ---
>>> Changes since v1:
>>> - Added this patch in v2
>>>
>>> We are allocating 2*RLIMIT_MEMLOCK.rlim_max memory and mmap() isn't
>>> failing. This seems like true bug in the kernel. Even the root user
>>> shouldn't be able to allocate more memory than allowed MEMLOCKed memory.
>>> Any ideas?
>>> ---
>>>  tools/testing/selftests/mm/on-fault-limit.c | 36 ++++++++++-----------
>>>  tools/testing/selftests/mm/run_vmtests.sh   |  2 +-
>>>  2 files changed, 18 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/tools/testing/selftests/mm/on-fault-limit.c b/tools/testing/selftests/mm/on-fault-limit.c
>>> index b5888d613f34e..0ea98ffab3589 100644
>>> --- a/tools/testing/selftests/mm/on-fault-limit.c
>>> +++ b/tools/testing/selftests/mm/on-fault-limit.c
>>> @@ -5,40 +5,38 @@
>>>  #include <string.h>
>>>  #include <sys/time.h>
>>>  #include <sys/resource.h>
>>> +#include "../kselftest.h"
>>>  
>>> -static int test_limit(void)
>>> +static void test_limit(void)
>>>  {
>>> -	int ret = 1;
>>>  	struct rlimit lims;
>>>  	void *map;
>>>  
>>> -	if (getrlimit(RLIMIT_MEMLOCK, &lims)) {
>>> -		perror("getrlimit");
>>> -		return ret;
>>> -	}
>>> +	if (getrlimit(RLIMIT_MEMLOCK, &lims))
>>> +		ksft_exit_fail_msg("getrlimit: %s\n", strerror(errno));
>>>  
>>> -	if (mlockall(MCL_ONFAULT | MCL_FUTURE)) {
>>> -		perror("mlockall");
>>> -		return ret;
>>> -	}
>>> +	if (mlockall(MCL_ONFAULT | MCL_FUTURE))
>>> +		ksft_exit_fail_msg("mlockall: %s\n", strerror(errno));
>>>  
>>>  	map = mmap(NULL, 2 * lims.rlim_max, PROT_READ | PROT_WRITE,
>>>  		   MAP_PRIVATE | MAP_ANONYMOUS | MAP_POPULATE, -1, 0);
>>> +
>>> +	ksft_test_result(map == MAP_FAILED, "Failed mmap\n");
>>> +
>>>  	if (map != MAP_FAILED)
>>> -		printf("mmap should have failed, but didn't\n");
>>> -	else {
>>> -		ret = 0;
>>>  		munmap(map, 2 * lims.rlim_max);
>>> -	}
>>> -
>>>  	munlockall();
>>> -	return ret;
>>>  }
>>>  
>>>  int main(int argc, char **argv)
>>>  {
>>> -	int ret = 0;
>>> +	ksft_print_header();
>>> +	ksft_set_plan(1);
>>> +
>>> +	if (getuid())
>>> +		ksft_test_result_skip("Require root privileges to run\n");
> I'd sent a patch to fix this behavior today. This test should run without
> root privileges.
> https://lore.kernel.org/all/20240201071307.592317-1-usama.anjum@collabora.com
> 
>>> +	else
>>> +		test_limit();
>>>  
>>> -	ret += test_limit();
>>> -	return ret;
>>> +	ksft_finished();
>>>  }
>>> diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh
>>> index 246d53a5d7f28..e373d592dbf5c 100755
>>> --- a/tools/testing/selftests/mm/run_vmtests.sh
>>> +++ b/tools/testing/selftests/mm/run_vmtests.sh
>>> @@ -291,7 +291,7 @@ echo "$nr_hugepgs" > /proc/sys/vm/nr_hugepages
>>>  
>>>  CATEGORY="compaction" run_test ./compaction_test
>>>  
>>> -CATEGORY="mlock" run_test sudo -u nobody ./on-fault-limit
>>> +CATEGORY="mlock" run_test ./on-fault-limit
>>
>> I think changing this is going to give unintended results. run_vmtests.sh must
>> already be running as root. "sudo -u nobody" is deprivieging the test to run as
>> nobody. The rlimit is not enforced for root so this test must run unprivileged
>> to work. See man page for getrlimit():
>>
>>   Since Linux 2.6.9, no limits are placed on the amount of memory that a
>>   privileged process may lock, and this limit instead governs the amount of
>>   memory that an unprivileged process may lock
>>
>> So I think the correct fix is actually to install sudo on your CI.
> run_vmtests.sh is invoked without sudo with following:
> make -C tools/testing/selftests/mm run_tests

Unfortunately, I live in a world where my build machine isn't always the same as
the target machine, so I'm not too familiar with this method of invocation.

Regardless, the vast majority of the tests in run_vmtests.sh (as well as the
configuration code in the script itself) require root. So invoking
run_vmtests.sh as anything other than root is a BadIdea (TM). And when
run_vmtests.sh is running as root, then you need the "sudo -u nobody" to
deprivilege this particular test.

> 
> Installing sudo in rootfs wouldn't be trivial enough on the CI side.
> Alternatively, we can check if sudo is present before executing this test
> to avoid error that sudo isn't found.

Yeah, that's probably the easiest solution; just skip it if the requirements are
not met.

> 
>>
>>>  
>>>  CATEGORY="mmap" run_test ./map_populate
>>>  
>>
>>
> 

Re: [PATCH v3 4/5] selftests/mm: protection_keys: save/restore nr_hugepages settings

[Joey Gouly]

Hi Muhammad,

On Thu, Jan 25, 2024 at 08:46:07PM +0500, Muhammad Usama Anjum wrote:
> Save and restore nr_hugepages before changing it during the test. A test
> should not change system wide settings.
> 
> Fixes: 5f23f6d082a9 ("x86/pkeys: Add self-tests")
> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> ---
>  tools/testing/selftests/mm/protection_keys.c | 34 ++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/tools/testing/selftests/mm/protection_keys.c b/tools/testing/selftests/mm/protection_keys.c
> index 48dc151f8fca8..f822ae31af22e 100644
> --- a/tools/testing/selftests/mm/protection_keys.c
> +++ b/tools/testing/selftests/mm/protection_keys.c
> @@ -54,6 +54,7 @@ int test_nr;
>  u64 shadow_pkey_reg;
>  int dprint_in_signal;
>  char dprint_in_signal_buffer[DPRINT_IN_SIGNAL_BUF_SIZE];
> +char buf[256];
>  
>  void cat_into_file(char *str, char *file)
>  {
> @@ -1744,6 +1745,38 @@ void pkey_setup_shadow(void)
>  	shadow_pkey_reg = __read_pkey_reg();
>  }
>  
> +void restore_settings_atexit(void)
> +{
> +	cat_into_file(buf, "/proc/sys/vm/nr_hugepages");
> +}
> +
> +void save_settings(void)
> +{
> +	int fd;
> +	int err;
> +
> +	if (geteuid())
> +		return;
> +
> +	fd = open("/proc/sys/vm/nr_hugepages", O_RDONLY);
> +	if (fd < 0) {
> +		fprintf(stderr, "error opening\n");
> +		perror("error: ");
> +		exit(__LINE__);
> +	}
> +
> +	/* -1 to guarantee leaving the trailing \0 */
> +	err = read(fd, buf, sizeof(buf)-1);
> +	if (err < 0) {
> +		fprintf(stderr, "error reading\n");
> +		perror("error: ");
> +		exit(__LINE__);
> +	}
> +
> +	atexit(restore_settings_atexit);
> +	close(fd);
> +}
> +
>  int main(void)
>  {
>  	int nr_iterations = 22;
> @@ -1751,6 +1784,7 @@ int main(void)
>  
>  	srand((unsigned int)time(NULL));
>  
> +	save_settings();
>  	setup_handlers();
>  
>  	printf("has pkeys: %d\n", pkeys_supported);
> -- 
> 2.42.0
> 

This break the tests for me:

assert() at protection_keys.c::812 test_nr: 19 iteration: 1
running abort_hooks()...

This is because some of the tests fork, so on their atexit() they will set the
nr_hugepages back to the previous setting. Specifically the
test_pkey_alloc_exhaust() test.

Thanks,
Joey

Re: [PATCH v3 4/5] selftests/mm: protection_keys: save/restore nr_hugepages settings

[Muhammad Usama Anjum]

On 3/13/24 7:58 PM, Joey Gouly wrote:
> Hi Muhammad,
> 
> On Thu, Jan 25, 2024 at 08:46:07PM +0500, Muhammad Usama Anjum wrote:
>> Save and restore nr_hugepages before changing it during the test. A test
>> should not change system wide settings.
>>
>> Fixes: 5f23f6d082a9 ("x86/pkeys: Add self-tests")
>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>> ---
>>  tools/testing/selftests/mm/protection_keys.c | 34 ++++++++++++++++++++
>>  1 file changed, 34 insertions(+)
>>
>> diff --git a/tools/testing/selftests/mm/protection_keys.c b/tools/testing/selftests/mm/protection_keys.c
>> index 48dc151f8fca8..f822ae31af22e 100644
>> --- a/tools/testing/selftests/mm/protection_keys.c
>> +++ b/tools/testing/selftests/mm/protection_keys.c
>> @@ -54,6 +54,7 @@ int test_nr;
>>  u64 shadow_pkey_reg;
>>  int dprint_in_signal;
>>  char dprint_in_signal_buffer[DPRINT_IN_SIGNAL_BUF_SIZE];
>> +char buf[256];
>>  
>>  void cat_into_file(char *str, char *file)
>>  {
>> @@ -1744,6 +1745,38 @@ void pkey_setup_shadow(void)
>>  	shadow_pkey_reg = __read_pkey_reg();
>>  }
>>  
>> +void restore_settings_atexit(void)
>> +{
>> +	cat_into_file(buf, "/proc/sys/vm/nr_hugepages");
>> +}
>> +
>> +void save_settings(void)
>> +{
>> +	int fd;
>> +	int err;
>> +
>> +	if (geteuid())
>> +		return;
>> +
>> +	fd = open("/proc/sys/vm/nr_hugepages", O_RDONLY);
>> +	if (fd < 0) {
>> +		fprintf(stderr, "error opening\n");
>> +		perror("error: ");
>> +		exit(__LINE__);
>> +	}
>> +
>> +	/* -1 to guarantee leaving the trailing \0 */
>> +	err = read(fd, buf, sizeof(buf)-1);
>> +	if (err < 0) {
>> +		fprintf(stderr, "error reading\n");
>> +		perror("error: ");
>> +		exit(__LINE__);
>> +	}
>> +
>> +	atexit(restore_settings_atexit);
>> +	close(fd);
>> +}
>> +
>>  int main(void)
>>  {
>>  	int nr_iterations = 22;
>> @@ -1751,6 +1784,7 @@ int main(void)
>>  
>>  	srand((unsigned int)time(NULL));
>>  
>> +	save_settings();
>>  	setup_handlers();
>>  
>>  	printf("has pkeys: %d\n", pkeys_supported);
>> -- 
>> 2.42.0
>>
> 
> This break the tests for me:
> 
> assert() at protection_keys.c::812 test_nr: 19 iteration: 1
> running abort_hooks()...
> 
> This is because some of the tests fork, so on their atexit() they will set the
> nr_hugepages back to the previous setting. Specifically the
> test_pkey_alloc_exhaust() test.
Thank you for reporting. Please can you test the following patch:

--- a/tools/testing/selftests/mm/protection_keys.c
+++ b/tools/testing/selftests/mm/protection_keys.c
@@ -1745,9 +1745,12 @@ void pkey_setup_shadow(void)
 	shadow_pkey_reg = __read_pkey_reg();
 }

+pid_t parent_pid;
+
 void restore_settings_atexit(void)
 {
-	cat_into_file(buf, "/proc/sys/vm/nr_hugepages");
+	if (parent_pid == getpid())
+		cat_into_file(buf, "/proc/sys/vm/nr_hugepages");
 }

 void save_settings(void)
@@ -1773,6 +1776,7 @@ void save_settings(void)
 		exit(__LINE__);
 	}

+	parent_pid = getpid();
 	atexit(restore_settings_atexit);
 	close(fd);
 }


> 
> Thanks,
> Joey
> 

-- 
BR,
Muhammad Usama Anjum

Re: [PATCH v3 4/5] selftests/mm: protection_keys: save/restore nr_hugepages settings

[Joey Gouly]

On Wed, Mar 13, 2024 at 11:12:58PM +0500, Muhammad Usama Anjum wrote:
> On 3/13/24 7:58 PM, Joey Gouly wrote:
> > Hi Muhammad,
> > 
> > On Thu, Jan 25, 2024 at 08:46:07PM +0500, Muhammad Usama Anjum wrote:
> >> Save and restore nr_hugepages before changing it during the test. A test
> >> should not change system wide settings.
> >>
> >> Fixes: 5f23f6d082a9 ("x86/pkeys: Add self-tests")
> >> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> >> ---
> >>  tools/testing/selftests/mm/protection_keys.c | 34 ++++++++++++++++++++
> >>  1 file changed, 34 insertions(+)
> >>
> >> diff --git a/tools/testing/selftests/mm/protection_keys.c b/tools/testing/selftests/mm/protection_keys.c
> >> index 48dc151f8fca8..f822ae31af22e 100644
> >> --- a/tools/testing/selftests/mm/protection_keys.c
> >> +++ b/tools/testing/selftests/mm/protection_keys.c
> >> @@ -54,6 +54,7 @@ int test_nr;
> >>  u64 shadow_pkey_reg;
> >>  int dprint_in_signal;
> >>  char dprint_in_signal_buffer[DPRINT_IN_SIGNAL_BUF_SIZE];
> >> +char buf[256];
> >>  
> >>  void cat_into_file(char *str, char *file)
> >>  {
> >> @@ -1744,6 +1745,38 @@ void pkey_setup_shadow(void)
> >>  	shadow_pkey_reg = __read_pkey_reg();
> >>  }
> >>  
> >> +void restore_settings_atexit(void)
> >> +{
> >> +	cat_into_file(buf, "/proc/sys/vm/nr_hugepages");
> >> +}
> >> +
> >> +void save_settings(void)
> >> +{
> >> +	int fd;
> >> +	int err;
> >> +
> >> +	if (geteuid())
> >> +		return;
> >> +
> >> +	fd = open("/proc/sys/vm/nr_hugepages", O_RDONLY);
> >> +	if (fd < 0) {
> >> +		fprintf(stderr, "error opening\n");
> >> +		perror("error: ");
> >> +		exit(__LINE__);
> >> +	}
> >> +
> >> +	/* -1 to guarantee leaving the trailing \0 */
> >> +	err = read(fd, buf, sizeof(buf)-1);
> >> +	if (err < 0) {
> >> +		fprintf(stderr, "error reading\n");
> >> +		perror("error: ");
> >> +		exit(__LINE__);
> >> +	}
> >> +
> >> +	atexit(restore_settings_atexit);
> >> +	close(fd);
> >> +}
> >> +
> >>  int main(void)
> >>  {
> >>  	int nr_iterations = 22;
> >> @@ -1751,6 +1784,7 @@ int main(void)
> >>  
> >>  	srand((unsigned int)time(NULL));
> >>  
> >> +	save_settings();
> >>  	setup_handlers();
> >>  
> >>  	printf("has pkeys: %d\n", pkeys_supported);
> >> -- 
> >> 2.42.0
> >>
> > 
> > This break the tests for me:
> > 
> > assert() at protection_keys.c::812 test_nr: 19 iteration: 1
> > running abort_hooks()...
> > 
> > This is because some of the tests fork, so on their atexit() they will set the
> > nr_hugepages back to the previous setting. Specifically the
> > test_pkey_alloc_exhaust() test.
> Thank you for reporting. Please can you test the following patch:
> 
> --- a/tools/testing/selftests/mm/protection_keys.c
> +++ b/tools/testing/selftests/mm/protection_keys.c
> @@ -1745,9 +1745,12 @@ void pkey_setup_shadow(void)
>  	shadow_pkey_reg = __read_pkey_reg();
>  }
> 
> +pid_t parent_pid;
> +
>  void restore_settings_atexit(void)
>  {
> -	cat_into_file(buf, "/proc/sys/vm/nr_hugepages");
> +	if (parent_pid == getpid())
> +		cat_into_file(buf, "/proc/sys/vm/nr_hugepages");
>  }
> 
>  void save_settings(void)
> @@ -1773,6 +1776,7 @@ void save_settings(void)
>  		exit(__LINE__);
>  	}
> 
> +	parent_pid = getpid();
>  	atexit(restore_settings_atexit);
>  	close(fd);
>  }
> 

Thanks, works for me:

Tested-by: Joey Gouly <joey.gouly@arm.com>