<!DOCTYPE html>
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<p><br>
</p>
<div class="moz-cite-prefix">On 27/03/26 12:45, Sayali Patil wrote:<br>
</div>
<blockquote type="cite"
cite="mid:2ba0f8d8a00fd049e8ed4b97e9ce8ee6b58a2892.1774591179.git.sayalip@linux.ibm.com">
<pre wrap="" class="moz-quote-pre">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 <a class="moz-txt-link-rfc2396E" href="mailto:ziy@nvidia.com"><ziy@nvidia.com></a>
Reviewed-by: David Hildenbrand (Arm) <a class="moz-txt-link-rfc2396E" href="mailto:david@kernel.org"><david@kernel.org></a>
Tested-by: Venkat Rao Bagalkote <a class="moz-txt-link-rfc2396E" href="mailto:venkat88@linux.ibm.com"><venkat88@linux.ibm.com></a>
Signed-off-by: Sayali Patil <a class="moz-txt-link-rfc2396E" href="mailto:sayalip@linux.ibm.com"><sayalip@linux.ibm.com></a>
---
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
}
</pre>
</blockquote>
<font face="monospace">AI review question:<br>
<br>
</font>
<div class="diff-line ai-comment"
style="box-sizing: border-box; display: block; padding: 0px 4px 0px 12px; min-height: 1.3em; background-color: rgb(255, 255, 255); border-left: 2.8px solid rgb(40, 53, 147); font-weight: 500; color: rgb(36, 41, 47); font-family: ui-monospace, SFMono-Regular, "SF Mono", Menlo, Consolas, monospace; font-size: 12px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; white-space: pre-wrap; text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial;">Does this cause excessive memory allocation churn during test execution?</div>
<div class="diff-line ai-comment"
style="box-sizing: border-box; display: block; padding: 0px 4px 0px 12px; min-height: 1.3em; background-color: rgb(255, 255, 255); border-left: 2.8px solid rgb(40, 53, 147); font-weight: 500; color: rgb(36, 41, 47); font-family: ui-monospace, SFMono-Regular, "SF Mono", Menlo, Consolas, monospace; font-size: 12px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; white-space: pre-wrap; text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial;"></div>
<div class="diff-line ai-comment"
style="box-sizing: border-box; display: block; padding: 0px 4px 0px 12px; min-height: 1.3em; background-color: rgb(255, 255, 255); border-left: 2.8px solid rgb(40, 53, 147); font-weight: 500; color: rgb(36, 41, 47); font-family: ui-monospace, SFMono-Regular, "SF Mono", Menlo, Consolas, monospace; font-size: 12px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; white-space: pre-wrap; text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial;">Since cleanup() is invoked before and after every test iteration, restoring</div>
<div class="diff-line ai-comment"
style="box-sizing: border-box; display: block; padding: 0px 4px 0px 12px; min-height: 1.3em; background-color: rgb(255, 255, 255); border-left: 2.8px solid rgb(40, 53, 147); font-weight: 500; color: rgb(36, 41, 47); font-family: ui-monospace, SFMono-Regular, "SF Mono", Menlo, Consolas, monospace; font-size: 12px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; white-space: pre-wrap; text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial;">the original nr_hugepages here (which could be thousands of pages) and then</div>
<div class="diff-line ai-comment"
style="box-sizing: border-box; display: block; padding: 0px 4px 0px 12px; min-height: 1.3em; background-color: rgb(255, 255, 255); border-left: 2.8px solid rgb(40, 53, 147); font-weight: 500; color: rgb(36, 41, 47); font-family: ui-monospace, SFMono-Regular, "SF Mono", Menlo, Consolas, monospace; font-size: 12px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; white-space: pre-wrap; text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial;">immediately setting it back to 10 in the next test case forces the kernel</div>
<div class="diff-line ai-comment"
style="box-sizing: border-box; display: block; padding: 0px 4px 0px 12px; min-height: 1.3em; background-color: rgb(255, 255, 255); border-left: 2.8px solid rgb(40, 53, 147); font-weight: 500; color: rgb(36, 41, 47); font-family: ui-monospace, SFMono-Regular, "SF Mono", Menlo, Consolas, monospace; font-size: 12px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; white-space: pre-wrap; text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial;">to repeatedly allocate and free many hugepages for every single test case.</div>
<div class="diff-line ai-comment"
style="box-sizing: border-box; display: block; padding: 0px 4px 0px 12px; min-height: 1.3em; background-color: rgb(255, 255, 255); border-left: 2.8px solid rgb(40, 53, 147); font-weight: 500; color: rgb(36, 41, 47); font-family: ui-monospace, SFMono-Regular, "SF Mono", Menlo, Consolas, monospace; font-size: 12px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; white-space: pre-wrap; text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial;"></div>
<div class="diff-line ai-comment"
style="box-sizing: border-box; display: block; padding: 0px 4px 0px 12px; min-height: 1.3em; background-color: rgb(255, 255, 255); border-left: 2.8px solid rgb(40, 53, 147); font-weight: 500; color: rgb(36, 41, 47); font-family: ui-monospace, SFMono-Regular, "SF Mono", Menlo, Consolas, monospace; font-size: 12px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; white-space: pre-wrap; text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial;">Also, does this reliably restore nr_hugepages on test failures?</div>
<div class="diff-line ai-comment"
style="box-sizing: border-box; display: block; padding: 0px 4px 0px 12px; min-height: 1.3em; background-color: rgb(255, 255, 255); border-left: 2.8px solid rgb(40, 53, 147); font-weight: 500; color: rgb(36, 41, 47); font-family: ui-monospace, SFMono-Regular, "SF Mono", Menlo, Consolas, monospace; font-size: 12px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; white-space: pre-wrap; text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial;"></div>
<div class="diff-line ai-comment"
style="box-sizing: border-box; display: block; padding: 0px 4px 0px 12px; min-height: 1.3em; background-color: rgb(255, 255, 255); border-left: 2.8px solid rgb(40, 53, 147); font-weight: 500; color: rgb(36, 41, 47); font-family: ui-monospace, SFMono-Regular, "SF Mono", Menlo, Consolas, monospace; font-size: 12px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; white-space: pre-wrap; text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial;">The test script runs with "set -e" active. If a test fails while the</div>
<div class="diff-line ai-comment"
style="box-sizing: border-box; display: block; padding: 0px 4px 0px 12px; min-height: 1.3em; background-color: rgb(255, 255, 255); border-left: 2.8px solid rgb(40, 53, 147); font-weight: 500; color: rgb(36, 41, 47); font-family: ui-monospace, SFMono-Regular, "SF Mono", Menlo, Consolas, monospace; font-size: 12px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; white-space: pre-wrap; text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial;">background write_to_hugetlbfs process is still running, commands earlier</div>
<div class="diff-line ai-comment"
style="box-sizing: border-box; display: block; padding: 0px 4px 0px 12px; min-height: 1.3em; background-color: rgb(255, 255, 255); border-left: 2.8px solid rgb(40, 53, 147); font-weight: 500; color: rgb(36, 41, 47); font-family: ui-monospace, SFMono-Regular, "SF Mono", Menlo, Consolas, monospace; font-size: 12px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; white-space: pre-wrap; text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial;">in cleanup() like "rmdir /mnt/huge" can fail with EBUSY because the</div>
<div class="diff-line ai-comment"
style="box-sizing: border-box; display: block; padding: 0px 4px 0px 12px; min-height: 1.3em; background-color: rgb(255, 255, 255); border-left: 2.8px solid rgb(40, 53, 147); font-weight: 500; color: rgb(36, 41, 47); font-family: ui-monospace, SFMono-Regular, "SF Mono", Menlo, Consolas, monospace; font-size: 12px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; white-space: pre-wrap; text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial;">directory is still a mounted filesystem.</div>
<div class="diff-line ai-comment"
style="box-sizing: border-box; display: block; padding: 0px 4px 0px 12px; min-height: 1.3em; background-color: rgb(255, 255, 255); border-left: 2.8px solid rgb(40, 53, 147); font-weight: 500; color: rgb(36, 41, 47); font-family: ui-monospace, SFMono-Regular, "SF Mono", Menlo, Consolas, monospace; font-size: 12px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; white-space: pre-wrap; text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial;"></div>
<div class="diff-line ai-comment"
style="box-sizing: border-box; display: block; padding: 0px 4px 0px 12px; min-height: 1.3em; background-color: rgb(255, 255, 255); border-left: 2.8px solid rgb(40, 53, 147); font-weight: 500; color: rgb(36, 41, 47); font-family: ui-monospace, SFMono-Regular, "SF Mono", Menlo, Consolas, monospace; font-size: 12px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; white-space: pre-wrap; text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial;">Due to "set -e", this failure causes the script to immediately exit,</div>
<div class="diff-line ai-comment"
style="box-sizing: border-box; display: block; padding: 0px 4px 0px 12px; min-height: 1.3em; background-color: rgb(255, 255, 255); border-left: 2.8px solid rgb(40, 53, 147); font-weight: 500; color: rgb(36, 41, 47); font-family: ui-monospace, SFMono-Regular, "SF Mono", Menlo, Consolas, monospace; font-size: 12px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; white-space: pre-wrap; text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial;">completely bypassing this restore command at the end of cleanup().</div>
<div class="diff-line ai-comment"
style="box-sizing: border-box; display: block; padding: 0px 4px 0px 12px; min-height: 1.3em; background-color: rgb(255, 255, 255); border-left: 2.8px solid rgb(40, 53, 147); font-weight: 500; color: rgb(36, 41, 47); font-family: ui-monospace, SFMono-Regular, "SF Mono", Menlo, Consolas, monospace; font-size: 12px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; white-space: pre-wrap; text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial;"></div>
<div class="diff-line ai-comment"
style="box-sizing: border-box; display: block; padding: 0px 4px 0px 12px; min-height: 1.3em; background-color: rgb(255, 255, 255); border-left: 2.8px solid rgb(40, 53, 147); font-weight: 500; color: rgb(36, 41, 47); font-family: ui-monospace, SFMono-Regular, "SF Mono", Menlo, Consolas, monospace; font-size: 12px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; white-space: pre-wrap; text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial;">Would it be better to restore the original value once at the very end</div>
<div class="diff-line ai-comment"
style="box-sizing: border-box; display: block; padding: 0px 4px 0px 12px; min-height: 1.3em; background-color: rgb(255, 255, 255); border-left: 2.8px solid rgb(40, 53, 147); font-weight: 500; color: rgb(36, 41, 47); font-family: ui-monospace, SFMono-Regular, "SF Mono", Menlo, Consolas, monospace; font-size: 12px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; white-space: pre-wrap; text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial;">of the script using an EXIT trap instead?</div>
<br>
<p data-start="0" data-end="153"><font face="monospace">Yes, it is
better to use an EXIT trap here to avoid unnecessary allocation<br>
churn and to ensure the original value is reliably restored on
all exit<br>
paths.</font></p>
<p data-start="155" data-end="474"><font face="monospace">A similar
change can also be applied to <code data-start="195"
data-end="224">hugetlb_reparenting_test.sh</code>. The<br>
test modifies <code data-start="244" data-end="258"
data-is-only-node="">nr_hugepages</code> during execution and
restores it from<br>
</font>
<code data-start="297" data-end="308">cleanup()</code><font
face="monospace">, while also reconfiguring it again in <code
data-start="347" data-end="356">setup()</code>, which is<br>
invoked multiple times across the test flow. This can lead to
repeated<br>
allocation and freeing of hugepages.</font></p>
<p data-start="476" data-end="535"><font face="monospace">I will
prepare a patch for both tests and include it in v4.</font></p>
<p data-start="537" data-end="553" data-is-last-node=""
data-is-only-node=""><font face="monospace">Thanks,<br
data-start="544" data-end="547">
Sayali</font></p>
<span style="white-space: pre-wrap">
</span>
</body>
</html>