linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/3] selftests/mm: hugetlb cgroup charging: robustness fixes
@ 2025-12-21  8:58 Li Wang
  2025-12-21  8:58 ` [PATCH v2 1/3] selftests/mm/write_to_hugetlbfs: parse -s as size_t Li Wang
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Li Wang @ 2025-12-21  8:58 UTC (permalink / raw)
  To: akpm, linux-kselftest, linux-kernel, linux-mm

Changes in v2:
  - 1/3: Parse -s using sscanf("%zu", ...) instead of strtoull().
  - 2/3: Fix typo in charge_reserved_hugetlb.sh ("reseravation" -> "reservation").
  - 3/3: No changes.

This series fixes a few issues in the hugetlb cgroup charging selftests
(write_to_hugetlbfs.c + charge_reserved_hugetlb.sh) that show up on systems
with large hugepages (e.g. 512MB) and when failures cause the test to wait
indefinitely.

On an aarch64 64k page kernel with 512MB hugepages, the test consistently
fails in write_to_hugetlbfs with ENOMEM and then hangs waiting for the
expected usage values. The root cause is that charge_reserved_hugetlb.sh
mounts hugetlbfs with a fixed size=256M, which is smaller than a single
hugepage, resulting in a mount with size=0 capacity.

In addition, write_to_hugetlbfs previously parsed -s via atoi() into an
int, which can overflow and print negative sizes.

Reproducer / environment:
  - Kernel: 6.12.0-xxx.el10.aarch64+64k
  - Hugepagesize: 524288 kB (512MB)
  - ./charge_reserved_hugetlb.sh -cgroup-v2
  - Observed mount: pagesize=512M,size=0 before this series

After applying the series, the test completes successfully on the above setup.

Li Wang (3):
  selftests/mm/write_to_hugetlbfs: parse -s as size_t
  selftests/mm/charge_reserved_hugetlb.sh: add waits with timeout helper
  selftests/mm/charge_reserved_hugetlb: fix hugetlbfs mount size for
    large hugepages

 .../selftests/mm/charge_reserved_hugetlb.sh   | 51 ++++++++++---------
 .../testing/selftests/mm/write_to_hugetlbfs.c |  9 ++--
 2 files changed, 34 insertions(+), 26 deletions(-)

-- 
2.49.0



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2 1/3] selftests/mm/write_to_hugetlbfs: parse -s as size_t
  2025-12-21  8:58 [PATCH v2 0/3] selftests/mm: hugetlb cgroup charging: robustness fixes Li Wang
@ 2025-12-21  8:58 ` Li Wang
  2025-12-21  9:01   ` David Hildenbrand (Red Hat)
  2025-12-21  8:58 ` [PATCH v2 2/3] selftests/mm/charge_reserved_hugetlb.sh: add waits with timeout helper Li Wang
  2025-12-21  8:58 ` [PATCH v2 3/3] selftests/mm/charge_reserved_hugetlb: fix hugetlbfs mount size for large hugepages Li Wang
  2 siblings, 1 reply; 13+ messages in thread
From: Li Wang @ 2025-12-21  8:58 UTC (permalink / raw)
  To: akpm, linux-kselftest, linux-kernel, linux-mm
  Cc: David Hildenbrand, Mark Brown, Shuah Khan, Waiman Long

write_to_hugetlbfs currently parses the -s size argument with atoi()
into an int. This silently accepts malformed input, cannot report overflow,
and can truncate large sizes.

--- 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
 ...
 # Writing to this path: /mnt/huge/test
 # Writing this size: -1610612736        <--------

Switch the size variable to size_t and parse -s with sscanf("%zu", ...).
Also print the size using %zu.

This avoids incorrect behavior with large -s values and makes the utility
more robust.

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>
---
 tools/testing/selftests/mm/write_to_hugetlbfs.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/mm/write_to_hugetlbfs.c b/tools/testing/selftests/mm/write_to_hugetlbfs.c
index 34c91f7e6128..ecb5f7619960 100644
--- a/tools/testing/selftests/mm/write_to_hugetlbfs.c
+++ b/tools/testing/selftests/mm/write_to_hugetlbfs.c
@@ -68,7 +68,7 @@ int main(int argc, char **argv)
 	int key = 0;
 	int *ptr = NULL;
 	int c = 0;
-	int size = 0;
+	size_t size = 0;
 	char path[256] = "";
 	enum method method = MAX_METHOD;
 	int want_sleep = 0, private = 0;
@@ -86,7 +86,10 @@ int main(int argc, char **argv)
 	while ((c = getopt(argc, argv, "s:p:m:owlrn")) != -1) {
 		switch (c) {
 		case 's':
-			size = atoi(optarg);
+			if (sscanf(optarg, "%zu", &size) != 1) {
+				perror("Invalid -s.");
+				exit_usage();
+			}
 			break;
 		case 'p':
 			strncpy(path, optarg, sizeof(path) - 1);
@@ -131,7 +134,7 @@ int main(int argc, char **argv)
 	}
 
 	if (size != 0) {
-		printf("Writing this size: %d\n", size);
+		printf("Writing this size: %zu\n", size);
 	} else {
 		errno = EINVAL;
 		perror("size not found");
-- 
2.49.0



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2 2/3] selftests/mm/charge_reserved_hugetlb.sh: add waits with timeout helper
  2025-12-21  8:58 [PATCH v2 0/3] selftests/mm: hugetlb cgroup charging: robustness fixes Li Wang
  2025-12-21  8:58 ` [PATCH v2 1/3] selftests/mm/write_to_hugetlbfs: parse -s as size_t Li Wang
@ 2025-12-21  8:58 ` Li Wang
  2025-12-21  9:15   ` David Hildenbrand (Red Hat)
  2025-12-21  8:58 ` [PATCH v2 3/3] selftests/mm/charge_reserved_hugetlb: fix hugetlbfs mount size for large hugepages Li Wang
  2 siblings, 1 reply; 13+ messages in thread
From: Li Wang @ 2025-12-21  8:58 UTC (permalink / raw)
  To: akpm, linux-kselftest, linux-kernel, linux-mm
  Cc: David Hildenbrand, Mark Brown, Shuah Khan, Waiman Long

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 bugs, timing issues, or unexpected behavior).

--- 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 (120 tries with
0.5s 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   | 47 ++++++++++---------
 1 file changed, 26 insertions(+), 21 deletions(-)

diff --git a/tools/testing/selftests/mm/charge_reserved_hugetlb.sh b/tools/testing/selftests/mm/charge_reserved_hugetlb.sh
index e1fe16bcbbe8..249a5776c074 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,46 @@ function setup_cgroup() {
   fi
 }
 
+function wait_for_file_value() {
+  local path="$1"
+  local expect="$2"
+  local max_tries="120"
+
+  local i cur
+  for ((i=1; i<=max_tries; i++)); do
+    cur="$(cat "$path")"
+    if [[ "$cur" == "$expect" ]]; then
+      return 0
+    fi
+    echo "Waiting for $path to become '$expect' (current: '$cur') (try $i/$max_tries)"
+    sleep 0.5
+  done
+
+  echo "ERROR: timeout waiting for $path to become '$expect' (last: '$cur')"
+  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() {
-- 
2.49.0



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2 3/3] selftests/mm/charge_reserved_hugetlb: fix hugetlbfs mount size for large hugepages
  2025-12-21  8:58 [PATCH v2 0/3] selftests/mm: hugetlb cgroup charging: robustness fixes Li Wang
  2025-12-21  8:58 ` [PATCH v2 1/3] selftests/mm/write_to_hugetlbfs: parse -s as size_t Li Wang
  2025-12-21  8:58 ` [PATCH v2 2/3] selftests/mm/charge_reserved_hugetlb.sh: add waits with timeout helper Li Wang
@ 2025-12-21  8:58 ` Li Wang
  2025-12-21  9:17   ` David Hildenbrand (Red Hat)
  2 siblings, 1 reply; 13+ messages in thread
From: Li Wang @ 2025-12-21  8:58 UTC (permalink / raw)
  To: akpm, linux-kselftest, linux-kernel, linux-mm
  Cc: David Hildenbrand, Mark Brown, Shuah Khan, Waiman Long

charge_reserved_hugetlb.sh mounts a hugetlbfs instance at /mnt/huge with
a fixed size of 256M. On systems with large base hugepages (e.g. 512MB),
this is smaller than a single hugepage, so the hugetlbfs mount ends up
with effectively zero capacity (often visible as size=0 in mount output).

As a result, write_to_hugetlbfs fails with ENOMEM and the test can hang
waiting for progress.

--- Error log ---
  # uname -r
  6.12.0-xxx.el10.aarch64+64k

  #./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
  ...

  # mount |grep /mnt/huge
  none on /mnt/huge type hugetlbfs (rw,relatime,seclabel,pagesize=512M,size=0)

  # grep -i huge /proc/meminfo
  ...
  HugePages_Total:      10
  HugePages_Free:       10
  HugePages_Rsvd:        0
  HugePages_Surp:        0
  Hugepagesize:     524288 kB
  Hugetlb:         5242880 kB

Mount hugetlbfs with size=${size}, the number of bytes the test will
reserve/write, so the filesystem capacity is sufficient regardless of
HugeTLB page size.

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>
---
 tools/testing/selftests/mm/charge_reserved_hugetlb.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/mm/charge_reserved_hugetlb.sh b/tools/testing/selftests/mm/charge_reserved_hugetlb.sh
index 249a5776c074..ac2744dbc0bd 100755
--- a/tools/testing/selftests/mm/charge_reserved_hugetlb.sh
+++ b/tools/testing/selftests/mm/charge_reserved_hugetlb.sh
@@ -295,7 +295,7 @@ function run_test() {
   setup_cgroup "hugetlb_cgroup_test" "$cgroup_limit" "$reservation_limit"
 
   mkdir -p /mnt/huge
-  mount -t hugetlbfs -o pagesize=${MB}M,size=256M none /mnt/huge
+  mount -t hugetlbfs -o pagesize=${MB}M,size=${size} none /mnt/huge
 
   write_hugetlbfs_and_get_usage "hugetlb_cgroup_test" "$size" "$populate" \
     "$write" "/mnt/huge/test" "$method" "$private" "$expect_failure" \
@@ -349,7 +349,7 @@ function run_multiple_cgroup_test() {
   setup_cgroup "hugetlb_cgroup_test2" "$cgroup_limit2" "$reservation_limit2"
 
   mkdir -p /mnt/huge
-  mount -t hugetlbfs -o pagesize=${MB}M,size=256M none /mnt/huge
+  mount -t hugetlbfs -o pagesize=${MB}M,size=${size} none /mnt/huge
 
   write_hugetlbfs_and_get_usage "hugetlb_cgroup_test1" "$size1" \
     "$populate1" "$write1" "/mnt/huge/test1" "$method" "$private" \
-- 
2.49.0



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/3] selftests/mm/write_to_hugetlbfs: parse -s as size_t
  2025-12-21  8:58 ` [PATCH v2 1/3] selftests/mm/write_to_hugetlbfs: parse -s as size_t Li Wang
@ 2025-12-21  9:01   ` David Hildenbrand (Red Hat)
  0 siblings, 0 replies; 13+ messages in thread
From: David Hildenbrand (Red Hat) @ 2025-12-21  9:01 UTC (permalink / raw)
  To: Li Wang, akpm, linux-kselftest, linux-kernel, linux-mm
  Cc: Mark Brown, Shuah Khan, Waiman Long

On 12/21/25 09:58, Li Wang wrote:
> write_to_hugetlbfs currently parses the -s size argument with atoi()
> into an int. This silently accepts malformed input, cannot report overflow,
> and can truncate large sizes.
> 
> --- 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
>   ...
>   # Writing to this path: /mnt/huge/test
>   # Writing this size: -1610612736        <--------


I mean, whoever does that should not expect anything reasonable to 
happen with these selftests ... so I don't think Fixes: should be added.

Acked-by: David Hildenbrand (Red Hat) <david@kernel.org>

-- 
Cheers

David


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 2/3] selftests/mm/charge_reserved_hugetlb.sh: add waits with timeout helper
  2025-12-21  8:58 ` [PATCH v2 2/3] selftests/mm/charge_reserved_hugetlb.sh: add waits with timeout helper Li Wang
@ 2025-12-21  9:15   ` David Hildenbrand (Red Hat)
  2025-12-21  9:35     ` Li Wang
  0 siblings, 1 reply; 13+ messages in thread
From: David Hildenbrand (Red Hat) @ 2025-12-21  9:15 UTC (permalink / raw)
  To: Li Wang, akpm, linux-kselftest, linux-kernel, linux-mm
  Cc: Mark Brown, Shuah Khan, Waiman Long

On 12/21/25 09:58, 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 bugs, timing issues, or unexpected behavior).

Did you actually hit that in practice? Just wondering.

> 
> --- 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 (120 tries with
> 0.5s 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   | 47 ++++++++++---------
>   1 file changed, 26 insertions(+), 21 deletions(-)
> 
> diff --git a/tools/testing/selftests/mm/charge_reserved_hugetlb.sh b/tools/testing/selftests/mm/charge_reserved_hugetlb.sh
> index e1fe16bcbbe8..249a5776c074 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,46 @@ function setup_cgroup() {
>     fi
>   }
>   
> +function wait_for_file_value() {
> +  local path="$1"
> +  local expect="$2"
> +  local max_tries="120"
> +
> +  local i cur

I would just move "cur" into the loop; I don't see a reason to print it 
on the error path when you printed the value on the last "Waiting" line?

	local cur="$(cat "$path")"

Also, not sure if you really need the "local i" here.

What if the path does not exist, do we want to catch that earlier and 
bail out instead of letting "cat" fail here?

> +  for ((i=1; i<=max_tries; i++)); do
> +    cur="$(cat "$path")"
> +    if [[ "$cur" == "$expect" ]]; then
> +      return 0
> +    fi
> +    echo "Waiting for $path to become '$expect' (current: '$cur') (try $i/$max_tries)"
> +    sleep 0.5

Any reason we don't go for the more intuitive "wait 1s" - max 60s wait?

> +  done


Nothing else jumped at me.

-- 
Cheers

David


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 3/3] selftests/mm/charge_reserved_hugetlb: fix hugetlbfs mount size for large hugepages
  2025-12-21  8:58 ` [PATCH v2 3/3] selftests/mm/charge_reserved_hugetlb: fix hugetlbfs mount size for large hugepages Li Wang
@ 2025-12-21  9:17   ` David Hildenbrand (Red Hat)
  2025-12-21  9:44     ` Li Wang
  0 siblings, 1 reply; 13+ messages in thread
From: David Hildenbrand (Red Hat) @ 2025-12-21  9:17 UTC (permalink / raw)
  To: Li Wang, akpm, linux-kselftest, linux-kernel, linux-mm
  Cc: Mark Brown, Shuah Khan, Waiman Long

On 12/21/25 09:58, Li Wang wrote:
> charge_reserved_hugetlb.sh mounts a hugetlbfs instance at /mnt/huge with
> a fixed size of 256M. On systems with large base hugepages (e.g. 512MB),
> this is smaller than a single hugepage, so the hugetlbfs mount ends up
> with effectively zero capacity (often visible as size=0 in mount output).
> 
> As a result, write_to_hugetlbfs fails with ENOMEM and the test can hang
> waiting for progress.

I'm curious, what's the history of using "256MB" in the first place (or 
specifying any size?).

-- 
Cheers

David


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 2/3] selftests/mm/charge_reserved_hugetlb.sh: add waits with timeout helper
  2025-12-21  9:15   ` David Hildenbrand (Red Hat)
@ 2025-12-21  9:35     ` Li Wang
  2025-12-21  9:52       ` David Hildenbrand (Red Hat)
  0 siblings, 1 reply; 13+ messages in thread
From: Li Wang @ 2025-12-21  9:35 UTC (permalink / raw)
  To: David Hildenbrand (Red Hat)
  Cc: akpm, linux-kselftest, linux-kernel, linux-mm, Mark Brown,
	Shuah Khan, Waiman Long

David Hildenbrand (Red Hat) <david@kernel.org> wrote:

> On 12/21/25 09:58, 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 bugs, timing issues, or unexpected behavior).
>
> Did you actually hit that in practice? Just wondering.

Yes.

On an aarch64 64k setup with 512MB hugepages, the test failed earlier
(hugetlbfs got mounted with an effective size of 0 due to size=256M), so
write_to_hugetlbfs couldn’t allocate the expected pages. After that, the
script’s wait loops never observed the target value, so they spun forever.

Detail see below logs.

>
> >
> > --- 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 (120 tries with
> > 0.5s 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   | 47 ++++++++++---------
> >   1 file changed, 26 insertions(+), 21 deletions(-)
> >
> > diff --git a/tools/testing/selftests/mm/charge_reserved_hugetlb.sh b/tools/testing/selftests/mm/charge_reserved_hugetlb.sh
> > index e1fe16bcbbe8..249a5776c074 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,46 @@ function setup_cgroup() {
> >     fi
> >   }
> >
> > +function wait_for_file_value() {
> > +  local path="$1"
> > +  local expect="$2"
> > +  local max_tries="120"
> > +
> > +  local i cur
>
> I would just move "cur" into the loop; I don't see a reason to print it
> on the error path when you printed the value on the last "Waiting" line?
>
>         local cur="$(cat "$path")"

+1

>
> Also, not sure if you really need the "local i" here.
>
> What if the path does not exist, do we want to catch that earlier and
> bail out instead of letting "cat" fail here?

Yes, we can add a file check before the "cat" loop.

>
> > +  for ((i=1; i<=max_tries; i++)); do
> > +    cur="$(cat "$path")"
> > +    if [[ "$cur" == "$expect" ]]; then
> > +      return 0
> > +    fi
> > +    echo "Waiting for $path to become '$expect' (current: '$cur') (try $i/$max_tries)"
> > +    sleep 0.5
>
> Any reason we don't go for the more intuitive "wait 1s" - max 60s wait?

Sure, the total loop time are same.

-- 
Regards,
Li Wang



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 3/3] selftests/mm/charge_reserved_hugetlb: fix hugetlbfs mount size for large hugepages
  2025-12-21  9:17   ` David Hildenbrand (Red Hat)
@ 2025-12-21  9:44     ` Li Wang
  2025-12-21  9:49       ` David Hildenbrand (Red Hat)
  0 siblings, 1 reply; 13+ messages in thread
From: Li Wang @ 2025-12-21  9:44 UTC (permalink / raw)
  To: David Hildenbrand (Red Hat)
  Cc: akpm, linux-kselftest, linux-kernel, linux-mm, Mark Brown,
	Shuah Khan, Waiman Long

David Hildenbrand (Red Hat) <david@kernel.org> wrote:
> On 12/21/25 09:58, Li Wang wrote:
> > charge_reserved_hugetlb.sh mounts a hugetlbfs instance at /mnt/huge with
> > a fixed size of 256M. On systems with large base hugepages (e.g. 512MB),
> > this is smaller than a single hugepage, so the hugetlbfs mount ends up
> > with effectively zero capacity (often visible as size=0 in mount output).
> >
> > As a result, write_to_hugetlbfs fails with ENOMEM and the test can hang
> > waiting for progress.
>
> I'm curious, what's the history of using "256MB" in the first place (or
> specifying any size?).

Seems the script initializes it with "256MB" from:

commit 29750f71a9b4cfae57cdddfbd8ca287eddca5503
Author: Mina Almasry <almasrymina@google.com>
Date:   Wed Apr 1 21:11:38 2020 -0700

    hugetlb_cgroup: add hugetlb_cgroup reservation tests


-- 
Regards,
Li Wang



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 3/3] selftests/mm/charge_reserved_hugetlb: fix hugetlbfs mount size for large hugepages
  2025-12-21  9:44     ` Li Wang
@ 2025-12-21  9:49       ` David Hildenbrand (Red Hat)
  2025-12-21 11:56         ` Li Wang
  0 siblings, 1 reply; 13+ messages in thread
From: David Hildenbrand (Red Hat) @ 2025-12-21  9:49 UTC (permalink / raw)
  To: Li Wang
  Cc: akpm, linux-kselftest, linux-kernel, linux-mm, Mark Brown,
	Shuah Khan, Waiman Long

On 12/21/25 10:44, Li Wang wrote:
> David Hildenbrand (Red Hat) <david@kernel.org> wrote:
>> On 12/21/25 09:58, Li Wang wrote:
>>> charge_reserved_hugetlb.sh mounts a hugetlbfs instance at /mnt/huge with
>>> a fixed size of 256M. On systems with large base hugepages (e.g. 512MB),
>>> this is smaller than a single hugepage, so the hugetlbfs mount ends up
>>> with effectively zero capacity (often visible as size=0 in mount output).
>>>
>>> As a result, write_to_hugetlbfs fails with ENOMEM and the test can hang
>>> waiting for progress.
>>
>> I'm curious, what's the history of using "256MB" in the first place (or
>> specifying any size?).
> 
> Seems the script initializes it with "256MB" from:
> 
> commit 29750f71a9b4cfae57cdddfbd8ca287eddca5503
> Author: Mina Almasry <almasrymina@google.com>
> Date:   Wed Apr 1 21:11:38 2020 -0700
> 
>      hugetlb_cgroup: add hugetlb_cgroup reservation tests

What would happen if we don't specify a size at all?

-- 
Cheers

David


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 2/3] selftests/mm/charge_reserved_hugetlb.sh: add waits with timeout helper
  2025-12-21  9:35     ` Li Wang
@ 2025-12-21  9:52       ` David Hildenbrand (Red Hat)
  2025-12-21 10:08         ` Li Wang
  0 siblings, 1 reply; 13+ messages in thread
From: David Hildenbrand (Red Hat) @ 2025-12-21  9:52 UTC (permalink / raw)
  To: Li Wang
  Cc: akpm, linux-kselftest, linux-kernel, linux-mm, Mark Brown,
	Shuah Khan, Waiman Long

On 12/21/25 10:35, Li Wang wrote:
> David Hildenbrand (Red Hat) <david@kernel.org> wrote:
> 
>> On 12/21/25 09:58, 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 bugs, timing issues, or unexpected behavior).
>>
>> Did you actually hit that in practice? Just wondering.
> 
> Yes.
> 
> On an aarch64 64k setup with 512MB hugepages, the test failed earlier
> (hugetlbfs got mounted with an effective size of 0 due to size=256M), so
> write_to_hugetlbfs couldn’t allocate the expected pages. After that, the
> script’s wait loops never observed the target value, so they spun forever.

Okay, so essentially what you fix in patch #3, correct?

It might make sense to reorder #2 and #3, and likely current #3 should 
get a Fixes: tag.

Then you can just briefly describe here that this was previously hit due 
to other tests issues. Although I wonder how much value this patch here 
as after #3 is in. But it looks like a cleanup and the timeout of 60s 
sounds reasonable.

I know the reservation of hugetlb folios can take a rather long time in 
some environments, though.

-- 
Cheers

David


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 2/3] selftests/mm/charge_reserved_hugetlb.sh: add waits with timeout helper
  2025-12-21  9:52       ` David Hildenbrand (Red Hat)
@ 2025-12-21 10:08         ` Li Wang
  0 siblings, 0 replies; 13+ messages in thread
From: Li Wang @ 2025-12-21 10:08 UTC (permalink / raw)
  To: David Hildenbrand (Red Hat)
  Cc: akpm, linux-kselftest, linux-kernel, linux-mm, Mark Brown,
	Shuah Khan, Waiman Long

On Sun, Dec 21, 2025 at 5:52 PM David Hildenbrand (Red Hat)
<david@kernel.org> wrote:
>
> On 12/21/25 10:35, Li Wang wrote:
> > David Hildenbrand (Red Hat) <david@kernel.org> wrote:
> >
> >> On 12/21/25 09:58, 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 bugs, timing issues, or unexpected behavior).
> >>
> >> Did you actually hit that in practice? Just wondering.
> >
> > Yes.
> >
> > On an aarch64 64k setup with 512MB hugepages, the test failed earlier
> > (hugetlbfs got mounted with an effective size of 0 due to size=256M), so
> > write_to_hugetlbfs couldn’t allocate the expected pages. After that, the
> > script’s wait loops never observed the target value, so they spun forever.
>
> Okay, so essentially what you fix in patch #3, correct?
>
> It might make sense to reorder #2 and #3, and likely current #3 should
> get a Fixes: tag.

+1

> Then you can just briefly describe here that this was previously hit due
> to other tests issues. Although I wonder how much value this patch here
> as after #3 is in. But it looks like a cleanup and the timeout of 60s
> sounds reasonable.

The reason is that I improved the infinite loop before debugging the #3 issue.
But your suggestion makes sense, and I will reorder patch #2 and #3.

-- 
Regards,
Li Wang



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 3/3] selftests/mm/charge_reserved_hugetlb: fix hugetlbfs mount size for large hugepages
  2025-12-21  9:49       ` David Hildenbrand (Red Hat)
@ 2025-12-21 11:56         ` Li Wang
  0 siblings, 0 replies; 13+ messages in thread
From: Li Wang @ 2025-12-21 11:56 UTC (permalink / raw)
  To: David Hildenbrand (Red Hat)
  Cc: akpm, linux-kselftest, linux-kernel, linux-mm, Mark Brown,
	Shuah Khan, Waiman Long

On Sun, Dec 21, 2025 at 5:49 PM David Hildenbrand (Red Hat)
<david@kernel.org> wrote:
>
> On 12/21/25 10:44, Li Wang wrote:
> > David Hildenbrand (Red Hat) <david@kernel.org> wrote:
> >> On 12/21/25 09:58, Li Wang wrote:
> >>> charge_reserved_hugetlb.sh mounts a hugetlbfs instance at /mnt/huge with
> >>> a fixed size of 256M. On systems with large base hugepages (e.g. 512MB),
> >>> this is smaller than a single hugepage, so the hugetlbfs mount ends up
> >>> with effectively zero capacity (often visible as size=0 in mount output).
> >>>
> >>> As a result, write_to_hugetlbfs fails with ENOMEM and the test can hang
> >>> waiting for progress.
> >>
> >> I'm curious, what's the history of using "256MB" in the first place (or
> >> specifying any size?).
> >
> > Seems the script initializes it with "256MB" from:
> >
> > commit 29750f71a9b4cfae57cdddfbd8ca287eddca5503
> > Author: Mina Almasry <almasrymina@google.com>
> > Date:   Wed Apr 1 21:11:38 2020 -0700
> >
> >      hugetlb_cgroup: add hugetlb_cgroup reservation tests
>
> What would happen if we don't specify a size at all?

It still works well, I have gone through the whole file and
there is no subtest that relies on the 256M capability.

So we could just:

    mount -t hugetlbfs -o pagesize=${MB}M none /mnt/huge

-- 
Regards,
Li Wang



^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2025-12-21 11:56 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-12-21  8:58 [PATCH v2 0/3] selftests/mm: hugetlb cgroup charging: robustness fixes Li Wang
2025-12-21  8:58 ` [PATCH v2 1/3] selftests/mm/write_to_hugetlbfs: parse -s as size_t Li Wang
2025-12-21  9:01   ` David Hildenbrand (Red Hat)
2025-12-21  8:58 ` [PATCH v2 2/3] selftests/mm/charge_reserved_hugetlb.sh: add waits with timeout helper Li Wang
2025-12-21  9:15   ` David Hildenbrand (Red Hat)
2025-12-21  9:35     ` Li Wang
2025-12-21  9:52       ` David Hildenbrand (Red Hat)
2025-12-21 10:08         ` Li Wang
2025-12-21  8:58 ` [PATCH v2 3/3] selftests/mm/charge_reserved_hugetlb: fix hugetlbfs mount size for large hugepages Li Wang
2025-12-21  9:17   ` David Hildenbrand (Red Hat)
2025-12-21  9:44     ` Li Wang
2025-12-21  9:49       ` David Hildenbrand (Red Hat)
2025-12-21 11:56         ` Li Wang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox