RFR: 8299858: [Metrics] Swap memory limit reported incorrectly when too large [v3]

Thomas Stuefe stuefe at openjdk.org
Wed Jan 25 13:17:08 UTC 2023


On Wed, 25 Jan 2023 11:24:55 GMT, Severin Gehwolf <sgehwolf at openjdk.org> wrote:

>> Please review this fix to [JDK-8292541](https://bugs.openjdk.org/browse/JDK-8292541) which adds the same handling for swap values exceeding what's possible in the non-container case. I.e. treats it as unlimited and fixes the reporting. This has been handled on the hotspot side similarly with [JDK-8292083](https://bugs.openjdk.org/browse/JDK-8292083).
>> 
>> Testing:
>> - [x] Container tests on linux x86_64 with cg v1
>> - [x] Container tests on linux x86_64 with cg v2
>> - [ ] GHA (seem to have had some infra issues, so re-running)
>
> Severin Gehwolf has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains one commit:
> 
>   8299858: [Metrics] Swap memory limit reported incorrectly when too large

Mostly good, small nits.

src/java.base/linux/native/libjava/CgroupMetrics.c line 44:

> 42: {
> 43:     jlong pages = sysconf(_SC_PHYS_PAGES);
> 44:     jlong page_size = sysconf(_SC_PAGESIZE);

Preexisting: Check or at least assert for error? sysconf could also signal "indeterminable" state beside an error. I cannot see how either one could happen with _SC_PHYS_PAGES or _SC_PAGESIZE, so an assert would probably be enough.
(I know you do an assert in java, but that has to be switched on explicitly - do we run tests with asserts enabled?)

src/java.base/linux/native/libjava/CgroupMetrics.c line 45:

> 43:     jlong pages = sysconf(_SC_PHYS_PAGES);
> 44:     jlong page_size = sysconf(_SC_PAGESIZE);
> 45:     return pages * page_size;

Preexisting, and theoretical nitpickery: I got curious when I saw this. What does sysconf return if a 32-bit VM runs on a large machine with >8Tb main memory and 4k pages? In that case, _SC_PHYS_PAGES would overflow the 32-bit long return type of sysconf.

src/java.base/linux/native/libjava/CgroupMetrics.c line 54:

> 52:     struct sysinfo si;
> 53:     sysinfo(&si);
> 54:     return (jlong)si.totalswap;

If sysinfo fails, we return a random value.

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

PR: https://git.openjdk.org/jdk/pull/12118


More information about the core-libs-dev mailing list