RFR: 8343191: Cgroup v1 subsystem fails to set subsystem path [v11]

[Severin Gehwolf]

On Mon, 24 Feb 2025 23:33:01 GMT, Sergey Chernyshev <schernyshev at openjdk.org> wrote:

>> `CgroupV1Controller::set_subsystem_path` needs high level comment update to describe the logic happening.
>> 
>> Testing:
>>>>And after the patch this would become this, right?
>>>>```
>>>>/sys/fs/cgroup/cpu,cpuacct/system.slice/garden.service/garden/bad/2f57368b-0eda-4e52-64d8-af5c
>>>>/sys/fs/cgroup/cpu,cpuacct/
>>>>```
>>>
>>>It depends on whether it was a subgroup in the initial path. If bad/2f57368b-0eda-4e52-64d8-af5c is the subgroup, the reduction will be
>>>
>>>```
>>>/sys/fs/cgroup/cpu,cpuacct/system.slice/garden.service/garden/bad/2f57368b-0eda-4e52-64d8-af5c
>>>/sys/fs/cgroup/cpu,cpuacct/bad/2f57368b-0eda-4e52-64d8-af5c
>>>/sys/fs/cgroup/cpu,cpuacct/bad
>>>/sys/fs/cgroup/cpu,cpuacct/
>>>```
>> 
>> The above case, doesn't seem to be reflected by any gtest test case (or others), please add those.
>
>> 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 conta...

@sercher As far as I can see this is a fairly simple case which would be covered by a simpler patch. My comment was in regards to my comment [here](https://github.com/openjdk/jdk/pull/21808#discussion_r1865630320). Where you replied with [this answer](https://github.com/openjdk/jdk/pull/21808#discussion_r1865663436). I don't see where anything you've described in your answer is being tested, covering this new code:

https://github.com/openjdk/jdk/pull/21808/files#diff-8910f554ed4a7bc465e01679328b3e9bd64ceaa6c85f00f0c575670e748ebba9R63-R77

That is, some sub-group without a match, but some with one.

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

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