Launch write_to_hugetlbfs as a separate process and move only its PID
into the target cgroup before waiting for completion. This avoids moving
the test shell itself, prevents unintended charging to the shell, and
ensures hugetlb and memcg accounting is attributed only to the intended
workload.
Add a short delay before the hugetlb allocation to avoid a race where
memory may be charged before the task migration takes effect, which
can lead to incorrect accounting and intermittent test failures.
The test currently validates both hugetlb usage and memory.current.
However, memory.current includes internal memcg allocations and
per-CPU batched accounting (MEMCG_CHARGE_BATCH), which are not
synchronized and can vary across systems, leading to
non-deterministic results.
Since hugetlb memory is accounted via hugetlb.<size>.current,
memory.current is not a reliable indicator here. Drop memory.current
checks and rely only on hugetlb controller statistics for stable
and accurate validation.
Fixes: 29750f71a9b4 ("hugetlb_cgroup: add hugetlb_cgroup reservation tests")
Signed-off-by: Sayali Patil <[email protected]>
---
.../selftests/mm/hugetlb_reparenting_test.sh | 42 ++++++++-----------
.../testing/selftests/mm/write_to_hugetlbfs.c | 5 ++-
2 files changed, 22 insertions(+), 25 deletions(-)
diff --git a/tools/testing/selftests/mm/hugetlb_reparenting_test.sh
b/tools/testing/selftests/mm/hugetlb_reparenting_test.sh
index 073a71fa36b4..1e87ac67d43e 100755
--- a/tools/testing/selftests/mm/hugetlb_reparenting_test.sh
+++ b/tools/testing/selftests/mm/hugetlb_reparenting_test.sh
@@ -104,22 +104,17 @@ function assert_with_retry() {
}
function assert_state() {
- local expected_a="$1"
- local expected_a_hugetlb="$2"
- local expected_b=""
+ local expected_a_hugetlb="$1"
local expected_b_hugetlb=""
- if [ ! -z ${3:-} ] && [ ! -z ${4:-} ]; then
- expected_b="$3"
- expected_b_hugetlb="$4"
+ if [ ! -z ${2:-} ]; then
+ expected_b_hugetlb="$2"
fi
- assert_with_retry "$CGROUP_ROOT/a/memory.$usage_file" "$expected_a"
assert_with_retry \
"$CGROUP_ROOT/a/hugetlb.${MB_DISPLAY}${UNIT}.$usage_file"
"$expected_a_hugetlb"
- if [[ -n "$expected_b" && -n "$expected_b_hugetlb" ]]; then
- assert_with_retry "$CGROUP_ROOT/a/b/memory.$usage_file" "$expected_b"
+ if [[ -n "$expected_b_hugetlb" ]]; then
assert_with_retry \
"$CGROUP_ROOT/a/b/hugetlb.${MB_DISPLAY}${UNIT}.$usage_file"
"$expected_b_hugetlb"
fi
@@ -153,18 +148,17 @@ write_hugetlbfs() {
local size="$3"
if [[ $cgroup2 ]]; then
- echo $$ >$CGROUP_ROOT/$cgroup/cgroup.procs
+ cg_file="$CGROUP_ROOT/$cgroup/cgroup.procs"
else
echo 0 >$CGROUP_ROOT/$cgroup/cpuset.mems
echo 0 >$CGROUP_ROOT/$cgroup/cpuset.cpus
- echo $$ >"$CGROUP_ROOT/$cgroup/tasks"
- fi
- ./write_to_hugetlbfs -p "$path" -s "$size" -m 0 -o
- if [[ $cgroup2 ]]; then
- echo $$ >$CGROUP_ROOT/cgroup.procs
- else
- echo $$ >"$CGROUP_ROOT/tasks"
+ cg_file="$CGROUP_ROOT/$cgroup/tasks"
fi
+
+ # Spawn write_to_hugetlbfs in a separate task to ensure correct cgroup
accounting
+ ./write_to_hugetlbfs -p "$path" -s "$size" -m 0 -o -d & pid=$!
+ echo "$pid" > "$cg_file"
+ wait "$pid"
echo
}
@@ -202,21 +196,21 @@ if [[ ! $cgroup2 ]]; then
write_hugetlbfs a "$MNT"/test $size
echo Assert memory charged correctly for parent use.
- assert_state 0 $size 0 0
+ assert_state $size 0
write_hugetlbfs a/b "$MNT"/test2 $size
echo Assert memory charged correctly for child use.
- assert_state 0 $(($size * 2)) 0 $size
+ assert_state $(($size * 2)) $size
rmdir "$CGROUP_ROOT"/a/b
echo Assert memory reparent correctly.
- assert_state 0 $(($size * 2))
+ assert_state $(($size * 2))
rm -rf "$MNT"/*
umount "$MNT"
echo Assert memory uncharged correctly.
- assert_state 0 0
+ assert_state 0
cleanup
fi
@@ -230,16 +224,16 @@ echo write
write_hugetlbfs a/b "$MNT"/test2 $size
echo Assert memory charged correctly for child only use.
-assert_state 0 $(($size)) 0 $size
+assert_state $(($size)) $size
rmdir "$CGROUP_ROOT"/a/b
echo Assert memory reparent correctly.
-assert_state 0 $size
+assert_state $size
rm -rf "$MNT"/*
umount "$MNT"
echo Assert memory uncharged correctly.
-assert_state 0 0
+assert_state 0
cleanup
diff --git a/tools/testing/selftests/mm/write_to_hugetlbfs.c
b/tools/testing/selftests/mm/write_to_hugetlbfs.c
index ecb5f7619960..6b01b0485bd0 100644
--- a/tools/testing/selftests/mm/write_to_hugetlbfs.c
+++ b/tools/testing/selftests/mm/write_to_hugetlbfs.c
@@ -83,7 +83,7 @@ int main(int argc, char **argv)
setvbuf(stdout, NULL, _IONBF, 0);
self = argv[0];
- while ((c = getopt(argc, argv, "s:p:m:owlrn")) != -1) {
+ while ((c = getopt(argc, argv, "s:p:m:owlrnd")) != -1) {
switch (c) {
case 's':
if (sscanf(optarg, "%zu", &size) != 1) {
@@ -118,6 +118,9 @@ int main(int argc, char **argv)
case 'n':
reserve = 0;
break;
+ case 'd':
+ sleep(1);
+ break;
default:
errno = EINVAL;
perror("Invalid arg");
--
2.52.0
