Hello Severin ,
> the new cgroups implementation
> supporting v1 and v2 Metrics.getMemoryAndSwapLimit() will never return 0
Wouldn’t it be possible that the coding of getMemoryAndSwapLimit returns a
negative value (might not happen on a "healthy" system but you never know) :
jdk/src/java.base/linux/classes/jdk/internal/platform/cgroupv1/CgroupV1Subsystem.java
444 public long getMemoryAndSwapLimit() {
445 long retval = getLongValue(memory, "memory.memsw.limit_in_bytes");
446 if (retval > CgroupV1SubsystemController.UNLIMITED_MIN) {
447 if (memory.isHierarchical()) {
448 // memory.memsw.limit_in_bytes returned unlimited, attempt
449 // hierarchical memory limit
450 String match = "hierarchical_memsw_limit";
451 retval =
CgroupV1SubsystemController.getLongValueMatchingLine(memory,
452 "memory.stat",
453 match);
454 }
455 }
456 return CgroupV1SubsystemController.longValOrUnlimited(retval);
457 }
jdk/src/java.base/linux/classes/jdk/internal/platform/cgroupv2/CgroupV2Subsystem.java
278 public long getMemoryAndSwapLimit() {
279 String strVal = CgroupSubsystemController.getStringValue(unified,
"memory.swap.max");
280 return limitFromString(strVal);
281 }
So the check you want to clean up does no harm, and might handle "strange"
cases - why not keep it?
Best regards, Matthias
-----Original Message-----
From: Severin Gehwolf <sgehw...@redhat.com>
Sent: Mittwoch, 15. Juli 2020 11:47
To: core-libs-dev <core-libs-dev@openjdk.java.net>; serviceability-dev
<serviceability-...@openjdk.java.net>
Cc: Baesken, Matthias <matthias.baes...@sap.com>; Bob Vandette
<bob.vande...@oracle.com>
Subject: Re: [PING?] RFR(s): 8247863: Unreachable code in
OperatingSystemImpl.getTotalSwapSpaceSize()
Anyone?
On Mon, 2020-06-29 at 17:53 +0200, Severin Gehwolf wrote:
> Hi,
>
> Could I please get a review of this dead-code removal? During review of
> JDK-8244500 it was discovered that with the new cgroups implementation
> supporting v1 and v2 Metrics.getMemoryAndSwapLimit() will never return
> 0 when relevant cgroup files are missing. E.g. on a system where the
> kernel doesn't support swap limit capabilities. Therefore this code
> introduced with JDK-8236617 can no longer be reached and should get
> removed.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8247863
> webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8247863/01/webrev/
>
> Testing: Matthias tested this on the affected system and it did pass
> for him. Docker tests on cgroup v1 and cgroup v2.
>
> Thanks,
> Severin
