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