All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mike Rapoport <rppt@kernel.org>
To: CaoRuichuang <create0818@163.com>
Cc: akpm@linux-foundation.org, david@kernel.org, ljs@kernel.org,
	Liam.Howlett@oracle.com, vbabka@kernel.org, surenb@google.com,
	mhocko@suse.com, shuah@kernel.org, linux-mm@kvack.org,
	linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] selftests: mm: stop relying on killall in charge_reserved_hugetlb
Date: Thu, 9 Apr 2026 13:48:13 +0300	[thread overview]
Message-ID: <adeD7VijQIaT8gWU@kernel.org> (raw)
In-Reply-To: <20260405213126.1161-1-create0818@163.com>

Hi,

On Mon, Apr 06, 2026 at 05:31:26AM +0800, CaoRuichuang wrote:
> charge_reserved_hugetlb.sh tears down background writers with
> killall -2 --wait write_to_hugetlbfs. That depends on killall from
> psmisc, which is not installed on minimal Ubuntu images, so the test
> fails in cleanup with "killall: command not found".

Can't we just skip the test if killall is not available just like we skip
it if it's not run as root?

> Track the writer PIDs we start ourselves and signal them directly
> during cleanup instead. Make write_hugetlb_memory.sh exec
> write_to_hugetlbfs so the recorded PID names the long-lived test
> process, and redirect async output straight to the temporary log file
> instead of going through a tee pipeline.
> 
> Signed-off-by: CaoRuichuang <create0818@163.com>
> ---
>  .../selftests/mm/charge_reserved_hugetlb.sh   | 25 +++++++++++++++----
>  .../selftests/mm/write_hugetlb_memory.sh      |  2 +-
>  2 files changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/testing/selftests/mm/charge_reserved_hugetlb.sh b/tools/testing/selftests/mm/charge_reserved_hugetlb.sh
> index 447769657..757df4878 100755
> --- a/tools/testing/selftests/mm/charge_reserved_hugetlb.sh
> +++ b/tools/testing/selftests/mm/charge_reserved_hugetlb.sh
> @@ -44,6 +44,8 @@ else
>  fi
>  export cgroup_path
>  
> +write_pids=()
> +
>  function cleanup() {
>    if [[ $cgroup2 ]]; then
>      echo $$ >$cgroup_path/cgroup.procs
> @@ -193,10 +195,12 @@ 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=$!
> +    write_pids+=("$write_pid")
>  
>      until grep -q -i "DONE" $output; do
>        echo waiting for DONE signal.
> @@ -261,10 +265,21 @@ function write_hugetlbfs_and_get_usage() {
>  function cleanup_hugetlb_memory() {
>    set +e
>    local cgroup="$1"
> -  if [[ "$(pgrep -f write_to_hugetlbfs)" != "" ]]; then
> -    echo killing write_to_hugetlbfs
> -    killall -2 --wait write_to_hugetlbfs
> -    wait_for_hugetlb_memory_to_get_depleted $cgroup
> +  local write_pid
> +
> +  if (( ${#write_pids[@]} )); then
> +    for write_pid in "${write_pids[@]}"; do
> +      if kill -0 "$write_pid" 2>/dev/null; then
> +        echo killing write_to_hugetlbfs pid "$write_pid"
> +        kill -2 "$write_pid"
> +        wait "$write_pid"
> +      fi
> +    done
> +    write_pids=()
> +
> +    if [[ -n "$cgroup" ]]; then
> +      wait_for_hugetlb_memory_to_get_depleted "$cgroup"
> +    fi
>    fi
>    set -e
>  
> diff --git a/tools/testing/selftests/mm/write_hugetlb_memory.sh b/tools/testing/selftests/mm/write_hugetlb_memory.sh
> index 3d2d2eb9d..49164bbfc 100755
> --- a/tools/testing/selftests/mm/write_hugetlb_memory.sh
> +++ b/tools/testing/selftests/mm/write_hugetlb_memory.sh
> @@ -19,5 +19,5 @@ echo $$ > ${cgroup_path:-/dev/cgroup/memory}/"$cgroup"/cgroup.procs
>  echo "Method is $method"
>  
>  set +e
> -./write_to_hugetlbfs -p "$path" -s "$size" "$write" "$populate" -m "$method" \
> +exec ./write_to_hugetlbfs -p "$path" -s "$size" "$write" "$populate" -m "$method" \
>        "$private" "$want_sleep" "$reserve"
> -- 
> 2.39.5 (Apple Git-154)
> 

-- 
Sincerely yours,
Mike.

      reply	other threads:[~2026-04-09 10:48 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-05 21:31 [PATCH] selftests: mm: stop relying on killall in charge_reserved_hugetlb CaoRuichuang
2026-04-09 10:48 ` Mike Rapoport [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=adeD7VijQIaT8gWU@kernel.org \
    --to=rppt@kernel.org \
    --cc=Liam.Howlett@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=create0818@163.com \
    --cc=david@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=ljs@kernel.org \
    --cc=mhocko@suse.com \
    --cc=shuah@kernel.org \
    --cc=surenb@google.com \
    --cc=vbabka@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.