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