RFR: JDK-8273526: Extend the OSContainer API pids controller with pids.current [v2]

Severin Gehwolf sgehwolf at openjdk.java.net
Fri Sep 10 12:41:08 UTC 2021


On Fri, 10 Sep 2021 11:07:52 GMT, Matthias Baesken <mbaesken at openjdk.org> wrote:

>> https://bugs.openjdk.java.net/browse/JDK-8266490
>> extended the OSContainer API in order to also support the pids controller of cgroups. However only pids.max output was added with 8266490.
>> There is a second parameter pids.current , see https://www.kernel.org/doc/html/latest/admin-guide/cgroup-v2.html#pid
>> that would be helpful too and can be added to the OSContainer API .
>> pids.current :
>> A read-only single value file which exists on all cgroups.
>> The number of processes currently in the cgroup and its descendants.
>> 
>> Best regards, Matthias
>
> Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Some simplifications and test adjustment suggested by Severin

Sorry for not being clear earlier. I meant that we can do those simplifications throughout.

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

> 253: }
> 254: 
> 255: char* CgroupV1Subsystem::pids_current_val() {

This function can get removed. It's unused now.

src/hotspot/os/linux/cgroupV1Subsystem_linux.hpp line 110:

> 108: 
> 109:     char * pids_max_val();
> 110:     char * pids_current_val();

This is not needed.

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

> 236: }
> 237: 
> 238: char* CgroupV2Subsystem::pids_current_val() {

We don't need that function.

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

> 268:  *    OSCONTAINER_ERROR for not supported
> 269:  */
> 270: jlong CgroupV2Subsystem::pids_current() {

This should use something akin to `memory_usage_in_bytes` in this class. `pids_current_val()` isn't needed.

src/java.base/linux/classes/jdk/internal/platform/cgroupv2/CgroupV2Subsystem.java line 317:

> 315:     public long getPidsCurrent() {
> 316:         String pidsCurrentStr = CgroupSubsystemController.getStringValue(unified, "pids.current");
> 317:         return CgroupSubsystem.limitFromString(pidsCurrentStr);

This should use: `return getLongVal("pids.current");` instead.

src/java.base/share/classes/sun/launcher/LauncherHelper.java line 415:

> 413:         ostream.println(formatLimitString(val, INDENT + "Current number of processes: ",
> 414:                                           longRetvalNotSupported, false));
> 415: 

I'm not sure we should add this to `-XshowSettings:system` output at all. What's reported there is enforced limits. Note that current memory usage isn't shown either.

test/hotspot/jtreg/containers/docker/TestPids.java line 101:

> 99:                     System.out.println("Found " + lineMarker + " with value: " + ivalue);
> 100:                     try {
> 101:                         int ai = Integer.parseInt(ivalue);

Could you move the debug print line to after line 101, please. It could say:

`System.out.println("Found " + lineMarker + " with value: " + ai + ". PASS.");`

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

Changes requested by sgehwolf (Reviewer).

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


More information about the core-libs-dev mailing list