Re: [PATCH v1 1/3] selftests/mm: fix va_high_addr_switch.sh return value

[Chunyu Hu <chuhu@redhat.com>]

On Mon, Dec 08, 2025 at 10:44:45AM -0500, Luiz Capitulino wrote:
> On 2025-12-07 07:22, Chunyu Hu wrote:
> > Patch series "Fix va_high_addr_switch.sh test failure - again", v1.
> > 
> > There are two issues exist for the  va_high_addr_switch test. One issue is
> > the test return value is ignored in va_high_addr_switch.sh. The second is
> > the va_high_addr_switch requires 6 hugepages but it requires 5.
> > 
> > Besides that, the nr_hugepages setup in run_vmtests.sh for arm64 can be
> > done in va_high_addr_switch.sh too.
> > 
> > This patch: (of 3)
> > 
> > The return value should be return value of va_high_addr_switch, otherwise
> > a test failure would be silently ignored.
> > 
> > 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>
> 
> This fix is good, but there are two additional issues that need fixing
> (maybe in separate patches):
> 
> 1. In main() we do:
> 
> """
>         ret = run_test(testcases, sz_testcases);
>         if (argc == 2 && !strcmp(argv[1], "--run-hugetlb"))
>                 ret = run_test(hugetlb_testcases, sz_hugetlb_testcases);
> """
> 
> The second run_test() overwrites the return code of the first one, so if
> the first fails and the second one succeeds, the test will report
> success.

Good catch. I can add a fix for this in v2.

> 
> 2. The following comment in va_high_addr_switch.sh is wrong in two
> counts: there's an eligibility check for powerpc64 and the test doesn't
> reject other architectures as it runs on arm64 as well.
> 
> """
>     # The test supports x86_64 and powerpc64. We currently have no useful
>     # eligibility check for powerpc64, and the test itself will reject other
>     # architectures.
> """

I can update the comment in v2.

> 
> For this fix:
> 
> Reviewed-by: Luiz Capitulino <luizcap@redhat.com>
> 
> > ---
> >   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
> 
> 
> 
> 
> 
> 
> 
> 
>