Patch series "Fix va_high_addr_switch.sh test failure - again", v2.
The series address several issues exist for the va_high_addr_switch test: 1) the test return value is ignored in va_high_addr_switch.sh. 2) the va_high_addr_switch test requires 6 hugepages not 5. 3) the reurn value of the first test in va_high_addr_switch.c can be overridden by the second test. 4) the nr_hugepages setup in run_vmtests.sh for arm64 can be done in va_high_addr_switch.sh too. 5) update a comment for check_test_requirements.
Changes in v2: - shorten the comment in for hugepages setup in v1 - add a new patch to fix the return value overridden issue in va_high_addr_switch.c - fix a code comment for check_test_requirements. - update the series summary in patch 1 - add reviewed-by from Luiz Capitulino on patch 1 and patch 3
This patch: (of 5)
The return value should be return value of va_high_addr_switch, otherwise a test failure would be silently ignored.
Reviewed-by: Luiz Capitulino luizcap@redhat.com Fixes: d9d957bd7b61 ("selftests/mm: alloc hugepages in va_high_addr_switch test") CC: Luiz Capitulino luizcap@redhat.com Signed-off-by: Chunyu Hu chuhu@redhat.com --- Chunyu Hu (5): selftests/mm: fix va_high_addr_switch.sh return value selftests/mm: allocate 6 hugepages in va_high_addr_switch.sh selftests/mm: remove arm64 nr_hugepages setup for va_high_addr_switch test selftests/mm: va_high_addr_switch return fail when either test failed selftests/mm: fix comment for check_test_requirements
tools/testing/selftests/mm/run_vmtests.sh | 8 -------- tools/testing/selftests/mm/va_high_addr_switch.c | 10 +++++++--- tools/testing/selftests/mm/va_high_addr_switch.sh | 12 +++++++----- 3 files changed, 14 insertions(+), 16 deletions(-)
According to the doc below, I don't add the cover letter, not sure if cover letter is preferred, and if that's the case, the doc need an update. https://www.ozlabs.org/~akpm/stuff/tpp.txt --- tools/testing/selftests/mm/va_high_addr_switch.sh | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/tools/testing/selftests/mm/va_high_addr_switch.sh b/tools/testing/selftests/mm/va_high_addr_switch.sh index a7d4b02b21dd..f89fe078a8e6 100755 --- a/tools/testing/selftests/mm/va_high_addr_switch.sh +++ b/tools/testing/selftests/mm/va_high_addr_switch.sh @@ -114,4 +114,6 @@ save_nr_hugepages # 4 keep_mapped pages, and one for tmp usage setup_nr_hugepages 5 ./va_high_addr_switch --run-hugetlb +retcode=$? restore_nr_hugepages +exit $retcode
The va_high_addr_switch test requires 6 hugepages, not 5. If running the test directly by: ./va_high_addr_switch.sh, the test will hit a mmap 'FAIL' caused by not enough hugepages: ``` mmap(addr_switch_hint - hugepagesize, 2*hugepagesize, MAP_HUGETLB): 0x7f330f800000 - OK mmap(addr_switch_hint , 2*hugepagesize, MAP_FIXED | MAP_HUGETLB): 0xffffffffffffffff - FAILED ``` The failure can't be hit if run the tests by running 'run_vmtests.sh -t hugevm' because the nr_hugepages is set to 128 at the beginning of run_vmtests.sh and va_high_addr_switch.sh skip the setup of nr_hugepages because already enough.
CC: Luiz Capitulino luizcap@redhat.com Fixes: d9d957bd7b61 ("selftests/mm: alloc hugepages in va_high_addr_switch test") Signed-off-by: Chunyu Hu chuhu@redhat.com --- tools/testing/selftests/mm/va_high_addr_switch.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/mm/va_high_addr_switch.sh b/tools/testing/selftests/mm/va_high_addr_switch.sh index f89fe078a8e6..a0c93d348b11 100755 --- a/tools/testing/selftests/mm/va_high_addr_switch.sh +++ b/tools/testing/selftests/mm/va_high_addr_switch.sh @@ -111,8 +111,8 @@ setup_nr_hugepages()
check_test_requirements save_nr_hugepages -# 4 keep_mapped pages, and one for tmp usage -setup_nr_hugepages 5 +# The HugeTLB tests require 6 pages +setup_nr_hugepages 6 ./va_high_addr_switch --run-hugetlb retcode=$? restore_nr_hugepages
arm64 and x86_64 has the same nr_hugepages requriement for running the va_high_addr_switch test. Since commit d9d957bd7b61 ("selftests/mm: alloc hugepages in va_high_addr_switch test"), the setup can be done in va_high_addr_switch.sh. So remove the duplicated setup.
Reviewed-by: Luiz Capitulino luizcap@redhat.com CC: Luiz Capitulino luizcap@redhat.com Signed-off-by: Chunyu Hu chuhu@redhat.com --- tools/testing/selftests/mm/run_vmtests.sh | 8 -------- 1 file changed, 8 deletions(-)
diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh index d9173f2312b7..2dadbfc6e535 100755 --- a/tools/testing/selftests/mm/run_vmtests.sh +++ b/tools/testing/selftests/mm/run_vmtests.sh @@ -412,15 +412,7 @@ if [ $VADDR64 -ne 0 ]; then fi
# va high address boundary switch test - ARCH_ARM64="arm64" - prev_nr_hugepages=$(cat /proc/sys/vm/nr_hugepages) - if [ "$ARCH" == "$ARCH_ARM64" ]; then - echo 6 > /proc/sys/vm/nr_hugepages - fi CATEGORY="hugevm" run_test bash ./va_high_addr_switch.sh - if [ "$ARCH" == "$ARCH_ARM64" ]; then - echo $prev_nr_hugepages > /proc/sys/vm/nr_hugepages - fi fi # VADDR64
# vmalloc stability smoke test
When the first test failed, and the hugetlb test passed, the result would be pass, but we expect a fail. Fix this issue by returning fail if either is not KSFT_PASS.
CC: Luiz Capitulino luizcap@redhat.com Signed-off-by: Chunyu Hu chuhu@redhat.com --- tools/testing/selftests/mm/va_high_addr_switch.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/mm/va_high_addr_switch.c b/tools/testing/selftests/mm/va_high_addr_switch.c index 02f290a69132..51401e081b20 100644 --- a/tools/testing/selftests/mm/va_high_addr_switch.c +++ b/tools/testing/selftests/mm/va_high_addr_switch.c @@ -322,7 +322,7 @@ static int supported_arch(void)
int main(int argc, char **argv) { - int ret; + int ret, hugetlb_ret = KSFT_PASS;
if (!supported_arch()) return KSFT_SKIP; @@ -331,6 +331,10 @@ int main(int argc, char **argv)
ret = run_test(testcases, sz_testcases); if (argc == 2 && !strcmp(argv[1], "--run-hugetlb")) - ret = run_test(hugetlb_testcases, sz_hugetlb_testcases); - return ret; + hugetlb_ret = run_test(hugetlb_testcases, sz_hugetlb_testcases); + + if (ret == KSFT_PASS && hugetlb_ret == KSFT_PASS) + return KSFT_PASS; + else + return KSFT_FAIL; }
The test supports arm64 as well so the comment is incorrect. And there's a check for arm64 in va_high_addr_switch.c.
CC: Luiz Capitulino luizcap@redhat.com Fixes: 983e760bcdb6 ("selftest/mm: va_high_addr_switch: add ppc64 support check") Fixes: f556acc2facd ("selftests/mm: skip test for non-LPA2 and non-LVA systems") Signed-off-by: Chunyu Hu chuhu@redhat.com --- tools/testing/selftests/mm/va_high_addr_switch.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/mm/va_high_addr_switch.sh b/tools/testing/selftests/mm/va_high_addr_switch.sh index a0c93d348b11..9492c2d72634 100755 --- a/tools/testing/selftests/mm/va_high_addr_switch.sh +++ b/tools/testing/selftests/mm/va_high_addr_switch.sh @@ -61,9 +61,9 @@ check_supported_ppc64()
check_test_requirements() { - # The test supports x86_64 and powerpc64. We currently have no useful - # eligibility check for powerpc64, and the test itself will reject other - # architectures. + # The test supports x86_64, powerpc64 and arm64. There's check for arm64 + # in va_high_addr_switch.c. The test itself will reject other architectures. + case `uname -m` in "x86_64") check_supported_x86_64
On Sun, 21 Dec 2025 12:00:21 +0800 Chunyu Hu chuhu@redhat.com wrote:
Patch series "Fix va_high_addr_switch.sh test failure - again", v2.
The series address several issues exist for the va_high_addr_switch test:
- the test return value is ignored in va_high_addr_switch.sh.
- the va_high_addr_switch test requires 6 hugepages not 5.
- the reurn value of the first test in va_high_addr_switch.c can be overridden by the second test.
- the nr_hugepages setup in run_vmtests.sh for arm64 can be done in va_high_addr_switch.sh too.
- update a comment for check_test_requirements.
Changes in v2:
- shorten the comment in for hugepages setup in v1
- add a new patch to fix the return value overridden issue in va_high_addr_switch.c
- fix a code comment for check_test_requirements.
- update the series summary in patch 1
- add reviewed-by from Luiz Capitulino on patch 1 and patch 3
The "Changes in v2" material is best placed below the "---" separator - I prefer not to capture such short-term development-time info within the permanent kernel record.
This patch: (of 5)
The return value should be return value of va_high_addr_switch, otherwise a test failure would be silently ignored.
Reviewed-by: Luiz Capitulino luizcap@redhat.com Fixes: d9d957bd7b61 ("selftests/mm: alloc hugepages in va_high_addr_switch test") CC: Luiz Capitulino luizcap@redhat.com Signed-off-by: Chunyu Hu chuhu@redhat.com
...
According to the doc below, I don't add the cover letter, not sure if cover letter is preferred, and if that's the case, the doc need an update.
Funnily enough, your series was in the exact format which I use when committing patch series. Usually people put the cover letter in a separate [0/N] email and I move that into the [1/N] patch's changelog, as you've done here.
God does that still exist? Pretty soon it will be able to legally drink in bars.
I think its content got absorbed into a Documentation/ file a long time ago!
On Sun, Dec 21, 2025 at 10:57:09AM -0800, Andrew Morton wrote:
On Sun, 21 Dec 2025 12:00:21 +0800 Chunyu Hu chuhu@redhat.com wrote:
Patch series "Fix va_high_addr_switch.sh test failure - again", v2.
The series address several issues exist for the va_high_addr_switch test:
- the test return value is ignored in va_high_addr_switch.sh.
- the va_high_addr_switch test requires 6 hugepages not 5.
- the reurn value of the first test in va_high_addr_switch.c can be overridden by the second test.
- the nr_hugepages setup in run_vmtests.sh for arm64 can be done in va_high_addr_switch.sh too.
- update a comment for check_test_requirements.
Changes in v2:
- shorten the comment in for hugepages setup in v1
- add a new patch to fix the return value overridden issue in va_high_addr_switch.c
- fix a code comment for check_test_requirements.
- update the series summary in patch 1
- add reviewed-by from Luiz Capitulino on patch 1 and patch 3
The "Changes in v2" material is best placed below the "---" separator - I prefer not to capture such short-term development-time info within the permanent kernel record.
That makes sense.
This patch: (of 5)
The return value should be return value of va_high_addr_switch, otherwise a test failure would be silently ignored.
Reviewed-by: Luiz Capitulino luizcap@redhat.com Fixes: d9d957bd7b61 ("selftests/mm: alloc hugepages in va_high_addr_switch test") CC: Luiz Capitulino luizcap@redhat.com Signed-off-by: Chunyu Hu chuhu@redhat.com
...
According to the doc below, I don't add the cover letter, not sure if cover letter is preferred, and if that's the case, the doc need an update.
Funnily enough, your series was in the exact format which I use when committing patch series. Usually people put the cover letter in a separate [0/N] email and I move that into the [1/N] patch's changelog, as you've done here.
yes, I see cover-letter is the actualy way people is using and looks like I did some of your work putting that cover letter into the first patch. I think I'll add cover-letter in the future.
God does that still exist? Pretty soon it will be able to legally drink in bars.
I think its content got absorbed into a Documentation/ file a long time ago!
I happened to open it before I submitting my patch, and wanted to know what would happen if I follow that. And it looks like cover letter has become the actual convention.
linux-kselftest-mirror@lists.linaro.org
-
Andrew Morton -
Chunyu Hu