On Mon, 17 Feb 2025 14:04:03 GMT, Severin Gehwolf <sgehw...@openjdk.org> wrote:

> The above case, doesn't seem to be reflected by any gtest test case (or 
> others), please add those.

The subgroup path reduction is covered by 
`TestMemoryWithSubgroups#testMemoryLimitSubgroupV1` (it wouldn't be possible in 
gtests as it requires touching cgroup tree on a host system). It actually 
checks that the subgroup directory exists and skips non-existent directories, 
that is reflected in the test log below. The bottom line comes from 
`CgroupV1Controller::set_subsystem_path`.

========== NEW TEST CASE:      Cgroup V1 subgroup memory limit: 100m
[COMMAND]
docker run --tty=true --rm --privileged --cgroupns=host --memory 200m 
jdk-internal:test-containers-docker-TestMemoryWithSubgroups-subgroup sh -c 
mkdir -p /sys/fs/cgroup/memory/test ; echo 100m > 
/sys/fs/cgroup/memory/test/memory.limit_in_bytes ; echo $$ > 
/sys/fs/cgroup/memory/test/cgroup.procs ; /jdk/bin/java 
-Xlog:os+container=trace -version
[2025-02-24T22:33:28.049656218Z] Gathering output for process 23369
[ELAPSED: 5385 ms]
[STDERR]

[STDOUT]
[0.002s][trace][os,container] OSContainer::init: Initializing Container Support
[0.003s][debug][os,container] Detected optional pids controller entry in 
/proc/cgroups
[0.004s][debug][os,container] Detected cgroups hybrid or legacy hierarchy, 
using cgroups v1 controllers
[0.004s][trace][os,container] set_subsystem_path: cgroup v1 path reduced to: 
/test.

With additional logging added before line 77, this could be looking like

[STDOUT]
[0.002s][trace][os,container] OSContainer::init: Initializing Container Support
[0.002s][debug][os,container] Detected optional pids controller entry in 
/proc/cgroups
[0.003s][debug][os,container] Detected cgroups hybrid or legacy hierarchy, 
using cgroups v1 controllers
[0.004s][trace][os,container] set_subsystem_path: skipped non existent 
directory: 
/docker/cc32e455402a8c98d1df6a81c685a540e7e528e714c981b10845c31b64d8a370/test.
[0.004s][trace][os,container] set_subsystem_path: skipped non existent 
directory: 
/cc32e455402a8c98d1df6a81c685a540e7e528e714c981b10845c31b64d8a370/test.
[0.005s][trace][os,container] set_subsystem_path: cgroup v1 path reduced to: 
/test.

Before the fix, the current path adjustment scheme would produce the following 
order:

/sys/fs/cgroup/memory/docker/cc32e455402a8c98d1df6a81c685a540e7e528e714c981b10845c31b64d8a370/test
/sys/fs/cgroup/memory/docker/cc32e455402a8c98d1df6a81c685a540e7e528e714c981b10845c31b64d8a370
/sys/fs/cgroup/memory/docker
/sys/fs/cgroup/memory

Only the last path is valid in the container, others are non-existent. The 
result will be 200m, while the correct is 100m.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/21808#issuecomment-2679936058

Reply via email to