RFR: 8334254: Cleanup CGroups initialization code

[Severin Gehwolf]

On Thu, 13 Jun 2024 18:20:33 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

> Hi,
> 
> Please consider this patch which attempts to improve the code quality of some of the cgroups code, somewhat.
> 
> 1. Get rid of `#define`s and upgrade to a `CG_INFO` enum, with some helpers
> 2. Make some of the code prettier by removing copy-pasted and edited code
> 3. Make `CgroupInfo` RAII, ridding us of the `cleanup` function
> 4. `set_subsystem_path` is **only** used at construction time, only call it at that time and make those functions private. Also remove `virtual`ness, as this is unnecessary now.
> 
> Testing:
> 
> 1. cgroupTest group in gtest.
> 
> Thank you.

Sure, this is fine. Just a few nits. I'll run this through my testing meanwhile.

src/hotspot/os/linux/cgroupSubsystem_linux.cpp line 382:

> 380:   for (int i = CG_INFO_START; i < CG_INFO_REQUIRED_END; i++) {
> 381:     if (!cg_infos[i]._data_complete) {
> 382:       log_debug(os, container)("Required cgroup v1 memory subsystem not found");

This log message is misleading. We either need to figure out which index we have and map that to `memory`, `cpuacct`, etc. or leave out `memory`.

src/hotspot/os/linux/cgroupSubsystem_linux.cpp line 382:

> 380:   for (int i = CG_INFO_START; i < CG_INFO_REQUIRED_END; i++) {
> 381:     if (!cg_infos[i]._data_complete) {
> 382:       log_debug(os, container)("Required cgroup v1 memory subsystem not found");

Suggestion:

      log_debug(os, container)("Required cgroup v1 subsystem controller not found");

src/hotspot/os/linux/cgroupSubsystem_linux.hpp line 74:

> 72:   PIDS_IDX, // pids is not required
> 73:   CG_INFO_REQUIRED_END = PIDS_IDX,
> 74:   CG_INFO_LENGTH = PIDS_IDX+1

style: space before and after `+`

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

PR Review: https://git.openjdk.org/jdk/pull/19709#pullrequestreview-2117867415
PR Review Comment: https://git.openjdk.org/jdk/pull/19709#discussion_r1639506491
PR Review Comment: https://git.openjdk.org/jdk/pull/19709#discussion_r1639507444
PR Review Comment: https://git.openjdk.org/jdk/pull/19709#discussion_r1639505162