RFR: 8300645: Handle julong values in logging of GET_CONTAINER_INFO macros [v2]
Severin Gehwolf
sgehwolf at openjdk.org
Wed Feb 15 14:51:50 UTC 2023
> 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)
Severin Gehwolf has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains eight additional commits since the last revision:
- Fix the GET_CONTAINER_INFO macro
Previously it would not take in a log_fmt argument and would, therefore,
format OS_CONTAINER_ERROR according to the log format string passed in,
potentially being JULONG_FORMAT. Now with the format passed in
explicitly it's possible to use a different format for the log line
if OS_CONTAINER_ERROR is being returned ('%d'). For the actual log line
in the non-error case no observable differences would be seen.
NOTE: GET_CONTAINER_INFO_LINE would not need the change as it's not
logging OS_CONTAINER_ERROR with a user-supplied format.
- Remove conditional in trace log call
- Reboot patch to original but keep reg-test
- Revert "Refactor checked values reading into separate functions"
This reverts commit 627e07067fefb0a44f8d6d14b3c4843b20e92bcb.
- Merge branch 'master' into jdk-8300645-revert-julong-get-container
- Refactor checked values reading into separate functions
- Merge branch 'master' into jdk-8300645-revert-julong-get-container
- 8300645: Revert julong changes from 8194232 after 8292083
-------------
Changes:
- all: https://git.openjdk.org/jdk/pull/12166/files
- new: https://git.openjdk.org/jdk/pull/12166/files/44d87740..4badfe08
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=12166&range=01
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=12166&range=00-01
Stats: 90315 lines in 2534 files changed: 35147 ins; 22876 del; 32292 mod
Patch: https://git.openjdk.org/jdk/pull/12166.diff
Fetch: git fetch https://git.openjdk.org/jdk pull/12166/head:pull/12166
PR: https://git.openjdk.org/jdk/pull/12166
More information about the hotspot-runtime-dev
mailing list