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 hotspot-runtime-dev
mailing list