charge_reserved_hugetlb.sh backgrounds write_hugetlb_memory.sh and immediately stores $? in write_result.

That only records whether the background job was started successfully, not whether write_hugetlb_memory.sh itself later failed. As a result, the test can miss reservation failure and OOM-kill outcomes that are inferred from the writer exit status.

Redirect the writer output straight to the temporary log file and wait for the background process before inspecting write_result, so the test records the actual exit status.

Signed-off-by: CaoRuichuang <create0818@163.com>
---
 tools/testing/selftests/mm/charge_reserved_hugetlb.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/mm/charge_reserved_hugetlb.sh b/tools/testing/selftests/mm/charge_reserved_hugetlb.sh
index 447769657..2f52ad7c8 100755
--- a/tools/testing/selftests/mm/charge_reserved_hugetlb.sh
+++ b/tools/testing/selftests/mm/charge_reserved_hugetlb.sh
@@ -193,9 +193,9 @@ function write_hugetlbfs_and_get_usage() {
     [[ "$private" == "-r" ]] && [[ "$expect_failure" != 1 ]]; then
 
     bash write_hugetlb_memory.sh "$size" "$populate" "$write" \
-      "$cgroup" "$path" "$method" "$private" "-l" "$reserve" 2>&1 | tee $output &
+      "$cgroup" "$path" "$method" "$private" "-l" "$reserve" \
+      >"$output" 2>&1 &
 
-    local write_result=$?
     local write_pid=$!
 
     until grep -q -i "DONE" $output; do
@@ -223,6 +223,8 @@ function write_hugetlbfs_and_get_usage() {
       sleep 0.5
     fi
 
+    wait "$write_pid"
+    local write_result=$?
     echo write_result is $write_result
   else
     bash write_hugetlb_memory.sh "$size" "$populate" "$write" \
-- 
2.39.5 (Apple Git-154)

On Mon,  6 Apr 2026 03:46:23 +0800 CaoRuichuang <create0818@163.com> wrote:

Please word-wrap your emails!

I fixed it.

> charge_reserved_hugetlb.sh backgrounds write_hugetlb_memory.sh and
> immediately stores $?  in write_result.

Yes.

> That only records whether the background job was started successfully,
> not whether write_hugetlb_memory.sh itself later failed.  As a result,
> the test can miss reservation failure and OOM-kill outcomes that are
> inferred from the writer exit status.

Seems that way.

> Redirect the writer output straight to the temporary log file and wait
> for the background process before inspecting write_result, so the test
> records the actual exit status.

Looks good to me.

It appears that this code is from 2020.

Question is, will this change break the selftests because it discovers
previously undiscovered failures!

> index 447769657..2f52ad7c8 100755
> --- a/tools/testing/selftests/mm/charge_reserved_hugetlb.sh
> +++ b/tools/testing/selftests/mm/charge_reserved_hugetlb.sh
> @@ -193,9 +193,9 @@ function write_hugetlbfs_and_get_usage() {
>      [[ "$private" == "-r" ]] && [[ "$expect_failure" != 1 ]]; then
>  
>      bash write_hugetlb_memory.sh "$size" "$populate" "$write" \
> -      "$cgroup" "$path" "$method" "$private" "-l" "$reserve" 2>&1 | tee $output &
> +      "$cgroup" "$path" "$method" "$private" "-l" "$reserve" \
> +      >"$output" 2>&1 &
>  
> -    local write_result=$?
>      local write_pid=$!
>  
>      until grep -q -i "DONE" $output; do
> @@ -223,6 +223,8 @@ function write_hugetlbfs_and_get_usage() {
>        sleep 0.5
>      fi
>  
> +    wait "$write_pid"
> +    local write_result=$?
>      echo write_result is $write_result
>    else
>      bash write_hugetlb_memory.sh "$size" "$populate" "$write" \

Do we actually need to play these backgrounding games?  We run
write_hugetlb_memory.sh then synchronously wait for it, polling its
output.  Why don't we simply run write_hugetlb_memory.sh synchronously?
Do away with that grep loop and the replaying of
write_hugetlb_memory.sh's output.

The 29750f71a9b4c changelog doesn't explain why it's done this way. 
Maybe Mina recalls?


Thanks, I'll set this aside for consideration after -rc1.  I expect a
cc:stable will be wanted.

Hi Andrew,

Thanks for the review.

I re-evaluated this after your question about the backgrounding model,
and I don't think this patch should move forward as-is.

In the async branch, write_hugetlb_memory.sh is invoked with -l, and
write_to_hugetlbfs prints DONE and then deliberately sleeps until
cleanup_hugetlb_memory() sends SIGINT. Waiting for $write_pid in that
path therefore blocks before cleanup runs, so my patch can deadlock
instead of reporting a meaningful exit status.

So I'll drop this change for now rather than push a flawed fix.

While validating the test on a minimal Ubuntu VM, I did find a different,
reproducible issue in the same selftest: cleanup currently depends on
killall from psmisc. I sent a separate patch for that one.

If there is a clearer direction for simplifying or tightening this async
flow after -rc1, I'll follow up on it.

Thanks,
CaoRuichuang