RFR: JDK-8266490: Extend the OSContainer API to support the pids controller of cgroups [v2]

Severin Gehwolf sgehwolf at openjdk.java.net
Tue Jun 29 08:25:06 UTC 2021


On Wed, 23 Jun 2021 13:37:59 GMT, Matthias Baesken <mbaesken at openjdk.org> wrote:

>> Hello, please review this PR; it extend the OSContainer API in order to also support the pids controller of cgroups.
>> 
>> I noticed that unlike the other controllers "cpu", "cpuset", "cpuacct", "memory"  on some older Linux distros (SLES 12.1, RHEL 7.1) the pids controller might not be there (or not fully supported) so it was added as optional  , see the coding
>> 
>> 
>>   if (!cg_infos[PIDS_IDX]._data_complete) {
>>     log_debug(os, container)("Optional cgroup v1 pids subsystem not found");
>>     // keep the other controller info, pids is optional
>>   }
>
> Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Adjustments following Severins comments

This looks pretty good now. Looking forward to seeing container tests for this new code.

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

> 557:     return OSCONTAINER_ERROR;
> 558:   }
> 559:   // Unlimited memory in Cgroups V2 is the literal string 'max'

Please don't add version specific comments to version agnostic code. Suggestion: "Unlimited memory in cgroups is the literal string 'max' for some controllers, for example the pids controller."

src/hotspot/os/linux/cgroupV1Subsystem_linux.cpp line 268:

> 266:   char * pidsmax_str = pids_max_val();
> 267:   jlong pidsmax = limit_from_str(pidsmax_str);
> 268:   return pidsmax;

Do we need this local variable? Consider using `return limit_from_str(pidsmax_str);` instead.

src/hotspot/os/linux/cgroupV2Subsystem_linux.cpp line 250:

> 248:   char * pidsmax_str = pids_max_val();
> 249:   jlong pidsmax = limit_from_str(pidsmax_str);
> 250:   return pidsmax;

Same here. Use `return limit_from_str(pidsmax_str);`

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

PR: https://git.openjdk.java.net/jdk/pull/4518


More information about the core-libs-dev mailing list