RFR: 8300645: Revert julong changes from 8194232 after 8292083

[Severin Gehwolf]

On Mon, 13 Feb 2023 16:43:57 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

>> Please review this change to Hotspot's container detection code. The core API of `OSContainer` uses `jlong` and should therefore also use `jlong` for reading values from the cgroup files. This fixes an oddity where on error, i.e. a file doesn't exist a negative number is being returned from the `GET_CONTAINER_INFO` macro, which potentially logs values. This results in incorrect log output as `-2` gets converted to `unsigned long` and printed according to that format.
>> 
>> The bug which introduced this change was [JDK-8194232](https://bugs.openjdk.org/browse/JDK-8194232) which was an attempt to deal with the fact that the cgroup v1 interface files might contain large numbers if no limit is in place. Since the memory limits are bound above by the physical host values after  [JDK-8292083](https://bugs.openjdk.org/browse/JDK-8292083) there is no need to "guess" what an unlimited value means any longer. If it's larger or equal to physical, treat it as unlimited.
>> 
>> Therefore, I propose to revert changes from [JDK-8194232](https://bugs.openjdk.org/browse/JDK-8194232) and handle comparisons to `unsigned long` values accordingly in the implementation. This avoids confusing trace output on some systems.
>> 
>> Testing:
>> - [x] Container tests on x86_64 fastdebug with cgroup v1 (legacy), with and without `swapaccount=0`.
>> - [x] Container tests on x86_64 fastdebug with cgroup v2 (unified), with and without `swapaccount=0`.
>> - [x] Added regression test. Fails on cg v1 with `swappaccount=0` on unpatched. Passes after this patch.
>> - [x] GHA (only unrelated failures in [compiler/vectorization/runner*](https://github.com/jerboaa/jdk/actions/runs/3989491372#user-content-compiler_vectorization_runner_arrayindexfilltest) on x86 - 32 bit and MacOSX x64)
>
> src/hotspot/os/linux/cgroupV1Subsystem_linux.cpp line 94:
> 
>> 92:   GET_CONTAINER_INFO(jlong, _memory->controller(), "/memory.limit_in_bytes",
>> 93:                      "Memory Limit is: " JLONG_FORMAT, JLONG_FORMAT, memlimit);
>> 94:   assert(memlimit > 0, "invariant");
> 
> This doesn't seem right to me. If parsing from this file can give us a negative number, then we ought to deal with that case explicitly on all builds.

The thing with `GET_CONTAINER_INFO` is that it's a preprocessor macro which might trigger a short return. So this cannot happen (currently). The assert is a safeguard. I'm not sure we need this handled in product code. Thoughts?

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

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