* [PATCH v2 1/2] selftests/mm: run_vmtests.sh: add missing tests @ 2024-01-23 7:36 Muhammad Usama Anjum 2024-01-23 7:36 ` [PATCH v2 2/2] selftests/mm: run_vmtests: remove sudo and conform to tap Muhammad Usama Anjum 2024-01-23 9:33 ` [PATCH v2 1/2] selftests/mm: run_vmtests.sh: add missing tests Ryan Roberts 0 siblings, 2 replies; 4+ messages in thread From: Muhammad Usama Anjum @ 2024-01-23 7:36 UTC (permalink / raw) To: Andrew Morton, Shuah Khan Cc: Muhammad Usama Anjum, kernel, Ryan Roberts, linux-mm, linux-kselftest, linux-kernel 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 --- tools/testing/selftests/mm/Makefile | 3 +++ tools/testing/selftests/mm/run_vmtests.sh | 3 +++ 2 files changed, 6 insertions(+) diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile index 2453add65d12f..c9c8112a7262e 100644 --- a/tools/testing/selftests/mm/Makefile +++ b/tools/testing/selftests/mm/Makefile @@ -114,6 +114,9 @@ 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 += write_hugetlb_memory.sh +TEST_FILES += hugetlb_reparenting_test.sh include ../lib.mk diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh index 246d53a5d7f28..12754af00b39c 100755 --- a/tools/testing/selftests/mm/run_vmtests.sh +++ b/tools/testing/selftests/mm/run_vmtests.sh @@ -248,6 +248,9 @@ CATEGORY="hugetlb" run_test ./map_hugetlb CATEGORY="hugetlb" run_test ./hugepage-mremap CATEGORY="hugetlb" run_test ./hugepage-vmemmap CATEGORY="hugetlb" run_test ./hugetlb-madvise +CATEGORY="hugetlb" run_test ./charge_reserved_hugetlb.sh -cgroup-v2 +CATEGORY="hugetlb" run_test ./hugetlb_reparenting_test.sh -cgroup-v2 +CATEGORY="hugetlb" run_test ./hugetlb-read-hwpoison nr_hugepages_tmp=$(cat /proc/sys/vm/nr_hugepages) # For this test, we need one and just one huge page -- 2.42.0 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2 2/2] selftests/mm: run_vmtests: remove sudo and conform to tap 2024-01-23 7:36 [PATCH v2 1/2] selftests/mm: run_vmtests.sh: add missing tests Muhammad Usama Anjum @ 2024-01-23 7:36 ` Muhammad Usama Anjum 2024-01-23 9:33 ` [PATCH v2 1/2] selftests/mm: run_vmtests.sh: add missing tests Ryan Roberts 1 sibling, 0 replies; 4+ messages in thread From: Muhammad Usama Anjum @ 2024-01-23 7:36 UTC (permalink / raw) To: Andrew Morton, Shuah Khan Cc: Muhammad Usama Anjum, kernel, linux-mm, linux-kselftest, linux-kernel 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 12754af00b39c..863bbc2015332 100755 --- a/tools/testing/selftests/mm/run_vmtests.sh +++ b/tools/testing/selftests/mm/run_vmtests.sh @@ -294,7 +294,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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 1/2] selftests/mm: run_vmtests.sh: add missing tests 2024-01-23 7:36 [PATCH v2 1/2] selftests/mm: run_vmtests.sh: add missing tests Muhammad Usama Anjum 2024-01-23 7:36 ` [PATCH v2 2/2] selftests/mm: run_vmtests: remove sudo and conform to tap Muhammad Usama Anjum @ 2024-01-23 9:33 ` Ryan Roberts 2024-01-23 14:45 ` Muhammad Usama Anjum 1 sibling, 1 reply; 4+ messages in thread From: Ryan Roberts @ 2024-01-23 9:33 UTC (permalink / raw) To: Muhammad Usama Anjum, Andrew Morton, Shuah Khan Cc: kernel, linux-mm, linux-kselftest, linux-kernel On 23/01/2024 07:36, 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 > --- > tools/testing/selftests/mm/Makefile | 3 +++ > tools/testing/selftests/mm/run_vmtests.sh | 3 +++ > 2 files changed, 6 insertions(+) > > diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile > index 2453add65d12f..c9c8112a7262e 100644 > --- a/tools/testing/selftests/mm/Makefile > +++ b/tools/testing/selftests/mm/Makefile > @@ -114,6 +114,9 @@ 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 += write_hugetlb_memory.sh > +TEST_FILES += hugetlb_reparenting_test.sh I see you are exporting 3 scripts, but only invoking 2 of them from run_vmtests.sh below. Is one a helper that gets called indirectly? > > include ../lib.mk > > diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh > index 246d53a5d7f28..12754af00b39c 100755 > --- a/tools/testing/selftests/mm/run_vmtests.sh > +++ b/tools/testing/selftests/mm/run_vmtests.sh > @@ -248,6 +248,9 @@ CATEGORY="hugetlb" run_test ./map_hugetlb > CATEGORY="hugetlb" run_test ./hugepage-mremap > CATEGORY="hugetlb" run_test ./hugepage-vmemmap > CATEGORY="hugetlb" run_test ./hugetlb-madvise > +CATEGORY="hugetlb" run_test ./charge_reserved_hugetlb.sh -cgroup-v2 > +CATEGORY="hugetlb" run_test ./hugetlb_reparenting_test.sh -cgroup-v2 > +CATEGORY="hugetlb" run_test ./hugetlb-read-hwpoison I'm not really a fan of adding this last test here; its destructive because it poisons 8 hugepages. So at a minimum, I think you need to modify the code in run_vmtests.sh to ensure those extra pages are allocated (there is already a section in the script that allocates hugepages). However, given this test is destructive, I'd prefer that it wasn't run as part of the main test set. Because the first time you run it, it will presumably pass, but now some of the hugepages are poisoned so next time you run it, there won't be enough unpoisoned hugepages and a test will fail. So you have very confusing behaviour for a developer who might be running these tests multiple times per boot (e.g. me). Perhaps we can add a -d (destructive) option to the script, and this test will only be run if that option is passed? Thanks, Ryan > > nr_hugepages_tmp=$(cat /proc/sys/vm/nr_hugepages) > # For this test, we need one and just one huge page ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 1/2] selftests/mm: run_vmtests.sh: add missing tests 2024-01-23 9:33 ` [PATCH v2 1/2] selftests/mm: run_vmtests.sh: add missing tests Ryan Roberts @ 2024-01-23 14:45 ` Muhammad Usama Anjum 0 siblings, 0 replies; 4+ messages in thread From: Muhammad Usama Anjum @ 2024-01-23 14:45 UTC (permalink / raw) To: Ryan Roberts Cc: Muhammad Usama Anjum, kernel, linux-mm, linux-kselftest, linux-kernel, Andrew Morton, Shuah Khan Hi Ryan, Thank you so much for reviewing and getting involved. On 1/23/24 2:33 PM, Ryan Roberts wrote: > On 23/01/2024 07:36, 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 >> --- >> tools/testing/selftests/mm/Makefile | 3 +++ >> tools/testing/selftests/mm/run_vmtests.sh | 3 +++ >> 2 files changed, 6 insertions(+) >> >> diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile >> index 2453add65d12f..c9c8112a7262e 100644 >> --- a/tools/testing/selftests/mm/Makefile >> +++ b/tools/testing/selftests/mm/Makefile >> @@ -114,6 +114,9 @@ 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 += write_hugetlb_memory.sh >> +TEST_FILES += hugetlb_reparenting_test.sh > > I see you are exporting 3 scripts, but only invoking 2 of them from > run_vmtests.sh below. Is one a helper that gets called indirectly? Yeah, write_hugetlb_memory.sh is needed by charge_reserved_hugetlb.sh. I'll put a comment there. > >> >> include ../lib.mk >> >> diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh >> index 246d53a5d7f28..12754af00b39c 100755 >> --- a/tools/testing/selftests/mm/run_vmtests.sh >> +++ b/tools/testing/selftests/mm/run_vmtests.sh >> @@ -248,6 +248,9 @@ CATEGORY="hugetlb" run_test ./map_hugetlb >> CATEGORY="hugetlb" run_test ./hugepage-mremap >> CATEGORY="hugetlb" run_test ./hugepage-vmemmap >> CATEGORY="hugetlb" run_test ./hugetlb-madvise >> +CATEGORY="hugetlb" run_test ./charge_reserved_hugetlb.sh -cgroup-v2 >> +CATEGORY="hugetlb" run_test ./hugetlb_reparenting_test.sh -cgroup-v2 >> +CATEGORY="hugetlb" run_test ./hugetlb-read-hwpoison > > I'm not really a fan of adding this last test here; its destructive because it > poisons 8 hugepages. So at a minimum, I think you need to modify the code in > run_vmtests.sh to ensure those extra pages are allocated (there is already a > section in the script that allocates hugepages). > > However, given this test is destructive, I'd prefer that it wasn't run as part > of the main test set. Because the first time you run it, it will presumably > pass, but now some of the hugepages are poisoned so next time you run it, there > won't be enough unpoisoned hugepages and a test will fail. So you have very > confusing behaviour for a developer who might be running these tests multiple > times per boot (e.g. me). > > Perhaps we can add a -d (destructive) option to the script, and this test will > only be run if that option is passed? Ideally we should be able to fix these tests before enabling them and there shouldn't be any side-effect of these. I'm struggling with the configurations where I'm getting consistent results. Studying and analyzing how and how many hugetlbs are being allocated/deallocated isn't straight forward enough in these. I'll spend more time to either put it under some flag or modify the tests to don't entangle with each other. > > Thanks, > Ryan > > >> >> nr_hugepages_tmp=$(cat /proc/sys/vm/nr_hugepages) >> # For this test, we need one and just one huge page > > -- BR, Muhammad Usama Anjum ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-01-23 14:45 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-01-23 7:36 [PATCH v2 1/2] selftests/mm: run_vmtests.sh: add missing tests Muhammad Usama Anjum 2024-01-23 7:36 ` [PATCH v2 2/2] selftests/mm: run_vmtests: remove sudo and conform to tap Muhammad Usama Anjum 2024-01-23 9:33 ` [PATCH v2 1/2] selftests/mm: run_vmtests.sh: add missing tests Ryan Roberts 2024-01-23 14:45 ` Muhammad Usama Anjum
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox