On Wed, Aug 27, 2025 at 7:41 PM David Hildenbrand <da...@redhat.com> wrote:
>
> On 27.08.25 09:52, Chunyu Hu wrote:
> > The nr_hugepgs variable is used to keep the original nr_hugepages at the
> > hugepage setup step at test beginning. After userfaultfd test, a cleaup is
> > executed, both /sys/kernel/mm/hugepages/hugepages-*/nr_hugepages and
> > /proc/sys//vm/nr_hugepages are reset to 'original' value before userfaultfd
> > test starts.
> >
> > Issue here is the value used to restore /proc/sys/vm/nr_hugepages is
> > nr_hugepgs which is the initial value before the vm_runtests.sh runs, not
> > the value before userfaultfd test starts. 'va_high_addr_swith.sh' tests
> > runs after that will possibly see no hugepages available for test, and got
> > EINVAL when mmap(HUGETLB), making the result invalid.
> >
> > And before pkey tests, nr_hugepgs is changed to be used as a temp variable
> > to save nr_hugepages before pkey test, and restore it after pkey tests
> > finish. The original nr_hugepages value is not tracked anymore, so no way
> > to restore it after all tests finish.
> >
> > Add a new variable nr_hugepgs_origin to save the original nr_hugepages, and
> > and restore it to nr_hugepages after all tests finish. And change to use
> > the nr_hugepgs variable to save the /proc/sys/vm/nr_hugeages after hugepage
> > setup, it's also the value before userfaultfd test starts, and the correct
> > value to be restored after userfaultfd finishes. The va_high_addr_switch.sh
> > broken will be resolved.
> >
> > Signed-off-by: Chunyu Hu <ch...@redhat.com>
> > ---
> > tools/testing/selftests/mm/run_vmtests.sh | 9 +++++++--
> > 1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/testing/selftests/mm/run_vmtests.sh
> > b/tools/testing/selftests/mm/run_vmtests.sh
> > index 471e539d82b8..f1a7ad3ec6a7 100755
> > --- a/tools/testing/selftests/mm/run_vmtests.sh
> > +++ b/tools/testing/selftests/mm/run_vmtests.sh
> > @@ -172,13 +172,13 @@ fi
> >
> > # set proper nr_hugepages
> > if [ -n "$freepgs" ] && [ -n "$hpgsize_KB" ]; then
> > - nr_hugepgs=$(cat /proc/sys/vm/nr_hugepages)
> > + nr_hugepgs_origin=$(cat /proc/sys/vm/nr_hugepages)
>
> I'd call this "orig_nr_hugepgs".
Hi David,
Thank you for your review and valuable feedback. I will rename it with
a v2 and resend the two patches. Do you have suggestions on patch 2?
>
> But it's a shame that the naming is then out of sync with nr_size_hugepgs?
nr_size_hugepgs is for uffd-wp-mremap, the test need all sizes hugepages,
it's used to save and restore the nr_hugepagees of all sizes of hugepages,
it's a test case setup, not like nr_hugepgs which is a global/general setup.
They are not the same kind, maybe they don't need to be aligned...
>
>
> > needpgs=$((needmem_KB / hpgsize_KB))
> > tries=2
> > while [ "$tries" -gt 0 ] && [ "$freepgs" -lt "$needpgs" ]; do
> > lackpgs=$((needpgs - freepgs))
> > echo 3 > /proc/sys/vm/drop_caches
> > - if ! echo $((lackpgs + nr_hugepgs)) >
> > /proc/sys/vm/nr_hugepages; then
> > + if ! echo $((lackpgs + nr_hugepgs_origin)) >
> > /proc/sys/vm/nr_hugepages; then
> > echo "Please run this test as root"
> > exit $ksft_skip
> > fi
> > @@ -189,6 +189,7 @@ if [ -n "$freepgs" ] && [ -n "$hpgsize_KB" ]; then
> > done < /proc/meminfo
> > tries=$((tries - 1))
> > done
> > + nr_hugepgs=$(cat /proc/sys/vm/nr_hugepages)
> > if [ "$freepgs" -lt "$needpgs" ]; then
> > printf "Not enough huge pages available (%d < %d)\n" \
> > "$freepgs" "$needpgs"
> > @@ -532,6 +533,10 @@ CATEGORY="page_frag" run_test ./test_page_frag.sh
> > aligned
> >
> > CATEGORY="page_frag" run_test ./test_page_frag.sh nonaligned
> >
> > +if [ "${HAVE_HUGEPAGES}" = 1 ]; then
> > + echo "$nr_hugepgs_origin" > /proc/sys/vm/nr_hugepages
> > +fi
>
> FWIW, I think the tests should maybe be doing that
> (save+configure+restore) themselves, like we do with THP settings through.
>
> thp_save_settings()
> thp_write_settings()
>
> and friends.
>
> This is not really something run_vmtests.sh should bother with.
>
> A bigger rework, though ...
Totally agree, with the c interface to do that is better. then the
vm_runtest.sh would be clean. It's a bigger rework outside of this topic...
>
> --
> Cheers
>
> David / dhildenb
>
--
----
Thanks,
Chunyu Hu
