On 27/03/26 12:45, Sayali Patil wrote:

During cleanup, the value of /proc/sys/vm/nr_hugepages is currently being
set to 0.  At the end of the test, if all tests pass, the original
nr_hugepages value is restored.  However, if any test fails, it remains
set to 0.
With this patch, we ensure that the original nr_hugepages value is
restored during cleanup, regardless of whether the test passes or fails.

Fixes: 7d695b1c3695b ("selftests/mm: save and restore nr_hugepages value")
Reviewed-by: Zi Yan <ziy@nvidia.com>
Reviewed-by: David Hildenbrand (Arm) <david@kernel.org>
Tested-by: Venkat Rao Bagalkote <venkat88@linux.ibm.com>
Signed-off-by: Sayali Patil <sayalip@linux.ibm.com>
---
 tools/testing/selftests/mm/charge_reserved_hugetlb.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/mm/charge_reserved_hugetlb.sh b/tools/testing/selftests/mm/charge_reserved_hugetlb.sh
index 447769657634..c9fe68b6fcf9 100755
--- a/tools/testing/selftests/mm/charge_reserved_hugetlb.sh
+++ b/tools/testing/selftests/mm/charge_reserved_hugetlb.sh
@@ -65,7 +65,7 @@ function cleanup() {
   if [[ -e $cgroup_path/hugetlb_cgroup_test2 ]]; then
     rmdir $cgroup_path/hugetlb_cgroup_test2
   fi
-  echo 0 >/proc/sys/vm/nr_hugepages
+  echo "$nr_hugepgs" > /proc/sys/vm/nr_hugepages
   echo CLEANUP DONE
 }

AI review question:

Does this cause excessive memory allocation churn during test execution?

Since cleanup() is invoked before and after every test iteration, restoring

the original nr_hugepages here (which could be thousands of pages) and then

immediately setting it back to 10 in the next test case forces the kernel

to repeatedly allocate and free many hugepages for every single test case.

Also, does this reliably restore nr_hugepages on test failures?

The test script runs with "set -e" active. If a test fails while the

background write_to_hugetlbfs process is still running, commands earlier

in cleanup() like "rmdir /mnt/huge" can fail with EBUSY because the

directory is still a mounted filesystem.

Due to "set -e", this failure causes the script to immediately exit,

completely bypassing this restore command at the end of cleanup().

Would it be better to restore the original value once at the very end

of the script using an EXIT trap instead?

Yes, it is better to use an EXIT trap here to avoid unnecessary allocation
churn and to ensure the original value is reliably restored on all exit
paths.

A similar change can also be applied to hugetlb_reparenting_test.sh. The
test modifies nr_hugepages during execution and restores it from
cleanup(), while also reconfiguring it again in setup(), which is
invoked multiple times across the test flow. This can lead to repeated
allocation and freeing of hugepages.

I will prepare a patch for both tests and include it in v4.

Thanks,
Sayali