RFR: 8322420: [Linux] cgroup v2: Limits in parent nested control groups are not detected [v2]

[Severin Gehwolf]

On Tue, 27 Aug 2024 14:07:47 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> Severin Gehwolf has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains eight commits:
>> 
>>  - Merge branch 'master' into jdk-8322420_cgroup_hierarchy_walk_init
>>  - Merge branch 'master' into jdk-8322420_cgroup_hierarchy_walk_init
>>  - Remove some duplication
>>  - Fix style
>>  - Merge branch 'master' into jdk-8322420_cgroup_hierarchy_walk_init
>>  - Merge branch 'master' into jdk-8322420_cgroup_hierarchy_walk_init
>>  - Merge branch 'master' into jdk-8322420_cgroup_hierarchy_walk_init
>>  - 8322420: [Linux] cgroup v2: Limits in parent nested control groups are not detected
>
> src/hotspot/os/linux/cgroupUtil_linux.cpp line 67:
> 
>> 65:     *last_slash = '\0'; // strip path
>> 66:     // update to shortened path and try again
>> 67:     mem->set_subsystem_path(cg_path);
> 
> Does this work with zero-length strings? If all that was left was "/" and we just stripped that away?

There is also the loop condition `(last_slash = strrchr(cg_path, '/')) != cg_path` which guards for this case. I.e. for `/foo` we stop and break the loop. Does that make sense?

> src/hotspot/os/linux/cgroupUtil_linux.cpp line 80:
> 
>> 78:   os::free(cg_path);
>> 79:   if (path_iterated) {
>> 80:     mem->set_subsystem_path((char*)"/");
> 
> set_subsystem_path should really take a const char*, and this cast should not be necessary.

Done.

> src/hotspot/os/linux/os_linux.hpp line 34:
> 
>> 32: class os::Linux {
>> 33:   friend class CgroupSubsystem;
>> 34:   friend class CgroupUtil;
> 
> why?

Because `CgroupUtil::adjust_controller(CgroupCpuController* cpu)` uses `os::Linux::active_processor_count()` for the host cpu value which is protected.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20646#discussion_r1733387011
PR Review Comment: https://git.openjdk.org/jdk/pull/20646#discussion_r1733387423
PR Review Comment: https://git.openjdk.org/jdk/pull/20646#discussion_r1733389714