Re: [PATCH v3 3/3] selftests/mm/charge_reserved_hugetlb.sh: add waits with timeout helper

Waiman Long
2025-12-22T03:54:40Z

[-- Attachment #1: Type: text/plain, Size: 7349 bytes --]

On 12/21/25 7:56 PM, Li Wang wrote:
> On Mon, Dec 22, 2025 at 4:30 AM Waiman Long<llong@redhat.com> wrote:
>>
>> On 12/21/25 7:26 AM, Li Wang wrote:
>>> The hugetlb cgroup usage wait loops in charge_reserved_hugetlb.sh were
>>> unbounded and could hang forever if the expected cgroup file value never
>>> appears (e.g. due to write_to_hugetlbfs in Error mapping).
>>>
>>> --- Error log ---
>>> # uname -r
>>> 6.12.0-xxx.el10.aarch64+64k
>>>
>>> # ls /sys/kernel/mm/hugepages/hugepages-*
>>> hugepages-16777216kB/ hugepages-2048kB/ hugepages-524288kB/
>>>
>>> #./charge_reserved_hugetlb.sh -cgroup-v2
>>> # -----------------------------------------
>>> ...
>>> # nr hugepages = 10
>>> # writing cgroup limit: 5368709120
>>> # writing reseravation limit: 5368709120
>>> ...
>>> # write_to_hugetlbfs: Error mapping the file: Cannot allocate memory
>>> # Waiting for hugetlb memory reservation to reach size 2684354560.
>>> # 0
>>> # Waiting for hugetlb memory reservation to reach size 2684354560.
>>> # 0
>>> # Waiting for hugetlb memory reservation to reach size 2684354560.
>>> # 0
>>> # Waiting for hugetlb memory reservation to reach size 2684354560.
>>> # 0
>>> # Waiting for hugetlb memory reservation to reach size 2684354560.
>>> # 0
>>> # Waiting for hugetlb memory reservation to reach size 2684354560.
>>> # 0
>>> ...
>>>
>>> Introduce a small helper, wait_for_file_value(), and use it for:
>>> - waiting for reservation usage to drop to 0,
>>> - waiting for reservation usage to reach a given size,
>>> - waiting for fault usage to reach a given size.
>>>
>>> This makes the waits consistent and adds a hard timeout (60 tries with
>>> 1s sleep) so the test fails instead of stalling indefinitely.
>>>
>>> Signed-off-by: Li Wang<liwang@redhat.com>
>>> Cc: David Hildenbrand<david@kernel.org>
>>> Cc: Mark Brown<broonie@kernel.org>
>>> Cc: Shuah Khan<shuah@kernel.org>
>>> Cc: Waiman Long<longman@redhat.com>
>>> ---
>>> .../selftests/mm/charge_reserved_hugetlb.sh | 51 +++++++++++--------
>>> 1 file changed, 30 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/tools/testing/selftests/mm/charge_reserved_hugetlb.sh b/tools/testing/selftests/mm/charge_reserved_hugetlb.sh
>>> index fa6713892d82..447769657634 100755
>>> --- a/tools/testing/selftests/mm/charge_reserved_hugetlb.sh
>>> +++ b/tools/testing/selftests/mm/charge_reserved_hugetlb.sh
>>> @@ -100,7 +100,7 @@ function setup_cgroup() {
>>> echo writing cgroup limit: "$cgroup_limit"
>>> echo "$cgroup_limit" >$cgroup_path/$name/hugetlb.${MB}MB.$fault_limit_file
>>>
>>> - echo writing reseravation limit: "$reservation_limit"
>>> + echo writing reservation limit: "$reservation_limit"
>>> echo "$reservation_limit" > \
>>> $cgroup_path/$name/hugetlb.${MB}MB.$reservation_limit_file
>>>
>>> @@ -112,41 +112,50 @@ function setup_cgroup() {
>>> fi
>>> }
>>>
>>> +function wait_for_file_value() {
>>> + local path="$1"
>>> + local expect="$2"
>>> + local max_tries=60
>>> +
>>> + if [[ ! -r "$path" ]]; then
>>> + echo "ERROR: cannot read '$path', missing or permission denied"
>>> + return 1
>>> + fi
>>> +
>>> + for ((i=1; i<=max_tries; i++)); do
>>> + local cur="$(cat "$path")"
>>> + if [[ "$cur" == "$expect" ]]; then
>>> + return 0
>>> + fi
>>> + echo "Waiting for $path to become '$expect' (current: '$cur') (try $i/$max_tries)"
>>> + sleep 1
>>> + done
>>> +
>>> + echo "ERROR: timeout waiting for $path to become '$expect'"
>>> + return 1
>>> +}
>>> +
>>> function wait_for_hugetlb_memory_to_get_depleted() {
>>> local cgroup="$1"
>>> local path="$cgroup_path/$cgroup/hugetlb.${MB}MB.$reservation_usage_file"
>>> - # Wait for hugetlbfs memory to get depleted.
>>> - while [ $(cat $path) != 0 ]; do
>>> - echo Waiting for hugetlb memory to get depleted.
>>> - cat $path
>>> - sleep 0.5
>>> - done
>>> +
>>> + wait_for_file_value "$path" "0"
>>> }
>>>
>>> function wait_for_hugetlb_memory_to_get_reserved() {
>>> local cgroup="$1"
>>> local size="$2"
>>> -
>>> local path="$cgroup_path/$cgroup/hugetlb.${MB}MB.$reservation_usage_file"
>>> - # Wait for hugetlbfs memory to get written.
>>> - while [ $(cat $path) != $size ]; do
>>> - echo Waiting for hugetlb memory reservation to reach size $size.
>>> - cat $path
>>> - sleep 0.5
>>> - done
>>> +
>>> + wait_for_file_value "$path" "$size"
>>> }
>>>
>>> function wait_for_hugetlb_memory_to_get_written() {
>>> local cgroup="$1"
>>> local size="$2"
>>> -
>>> local path="$cgroup_path/$cgroup/hugetlb.${MB}MB.$fault_usage_file"
>>> - # Wait for hugetlbfs memory to get written.
>>> - while [ $(cat $path) != $size ]; do
>>> - echo Waiting for hugetlb memory to reach size $size.
>>> - cat $path
>>> - sleep 0.5
>>> - done
>>> +
>>> + wait_for_file_value "$path" "$size"
>>> }
>>>
>>> function write_hugetlbfs_and_get_usage() {
>> wait_for_file_value() now return 0 onr success and 1 on timeout.
>> However, none of the callers of the wait_for_hugetlb_memory* are
>> checking their return values and acting accordingly. Are we expecting
>> that the test will show failure because the waiting isn't completed or
>> should we explicitly exit with ksft_fail (1) value?
> Hmm, it seems the test shouldn't exit too early.
>
> As the wait_for_hugetlb_memory* is only trying 60s to examine the file
> value, if timeouted, we still need to keep going because the test requires
> CLEANUP work and exit/report from there.
>
> The key point of each subtest is to save the '$write_result' value and
> examine it
> which controls the whole test to exit.
>
> e.g.
>
> This is an intentional error test:
>
> # ./charge_reserved_hugetlb.sh -cgroup-v2
> CLEANUP DONE
> ...
> Writing to this path: /mnt/huge/test
> Writing this size: 2684354560
> Not populating.
> Not writing to memory.
> Using method=0
> Shared mapping.
> RESERVE mapping.
> Allocating using HUGETLBFS.
> write_to_hugetlbfs: Error mapping the file: Cannot allocate memory
> Waiting for /sys/fs/cgroup/hugetlb_cgroup_test/hugetlb.512MB.rsvd.current
> to become '2684354560' (current: '0') (try 1/60)
> Waiting for /sys/fs/cgroup/hugetlb_cgroup_test/hugetlb.512MB.rsvd.current
> to become '2684354560' (current: '0') (try 2/60)
> Waiting for /sys/fs/cgroup/hugetlb_cgroup_test/hugetlb.512MB.rsvd.current
> to become '2684354560' (current: '0') (try 3/60)
> Waiting for /sys/fs/cgroup/hugetlb_cgroup_test/hugetlb.512MB.rsvd.current
> to become '2684354560' (current: '0') (try 4/60)
> ...
> Waiting for /sys/fs/cgroup/hugetlb_cgroup_test/hugetlb.512MB.rsvd.current
> to become '2684354560' (current: '0') (try 60/60)
> ERROR: timeout waiting for
> /sys/fs/cgroup/hugetlb_cgroup_test/hugetlb.512MB.rsvd.current to
> become '2684354560'
> After write:
> hugetlb_usage=0
> reserved_usage=0
> 0
> 0
> Memory charged to hugtlb=0
> Memory charged to reservation=0
> expected (2684354560) != actual (0): Reserved memory not charged to
> reservation usage.
> CLEANUP DONE

Thank for running a test case. As long as the test will still report a
failure, it will be fine with me. I just want to note that the return
value value of wait_for_file_value() isn't currently used at all.

Cheers, Longman

[-- Attachment #2: Type: text/html, Size: 7918 bytes --]