8349988: Change cgroup version detection logic to not depend on /proc/cgroups#4271
8349988: Change cgroup version detection logic to not depend on /proc/cgroups#4271jerboaa wants to merge 2 commits intoopenjdk:masterfrom
Conversation
|
👋 Welcome back sgehwolf! A progress list of the required criteria for merging this PR into |
|
@jerboaa This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be: You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been no new commits pushed to the ➡️ To integrate this PR with the above commit message to the |
|
This backport pull request has now been updated with issues from the original commit. |
|
/issue add JDK-8347811 |
|
@jerboaa |
|
@jerboaa |
|
I've included JDK-8354878 - a one-liner - directly in here so as to not regress. |
|
On an affected system (Ubuntu 2025-10) we have this before/after. Before After |
|
@fitzsim Could you please take a look? Thank you! |
Webrevs
|
|
@jerboaa taking a look... |
|
/reviewer credit @fitzsim |
|
/reviewer credit @fitzsim |
|
@jerboaa |
|
This looks good to me. I no longer have the virtual machines at hand to test, so I didn't test it. I confirmed the changes versus the original patch seem fine though:
At least |
|
Thanks for the review! |
|
On my host (Debian, kernel before: after: i.e., the warning about missing memory controller has gone. |
|
I'm not sure how useful this is, but |
In order to get past that you'd need the whitebox changes from this patch as well. Finally you'd have to change the underlying function it calls to also take a boolean (since the test relies on a hint from the filesystem type of Alternatively run container tests on Ubuntu 2025.10 before this fix and after. |
|
|
gnu-andrew
left a comment
There was a problem hiding this comment.
This looks like a pretty clean backport, with the only real difference between NULL and nullptr.
I was confused by the cgroupSubsystem_linux.cpp changes at first but I now see they are coming from a second change. It would have been a bit clearer to have these in their own commit.
|
/approval request Please approve this enhancement to change the cgroups version detection in Hotspot based on the filesystem type of /sys/fs/cgroup and using cgroup.controllers file over the deprecated /proc/cgroups file (which is cg v1 only). This change is needed to properly handle container detection on newer cgroup v2 systems with specific kernels which might no longer expose cg v2 controllers in the legacy file /proc/cgroups. One such system is Ubuntu 2025.10. The patch isn't clean since some later backports aren't in JDK 17u. It got reviewed by Thomas Fitzsimmons, Thomas Stüfe and Andrew Hughes. The one-liner follow-up of JDK-8354878 is included in the patch. Risk is low to medium. Changes container detection code. Linux-only change. Tested with container tests on cg v1 and cg v2 as well as affected system (all pass). Patch is included in later JDKs (21.0.10, 25 GA) for a while now with no known bug-tail. |
|
/approve yes |
|
@gnu-andrew |
|
/integrate |
|
Going to push as commit 4376c27. |
Please review this backport of the cgroup version detection logic which affects JDK 17 as well. Newer systems which are affected, like Ubuntu 2025-XX, get more commonly used. Therefore, I propose to backport it there too. The JDK 21 backport got shipped with the 21.0.10 update in January. The patch is not clean due to
nullptrvsNULLdifferences and more context changes (e.g. JDK-8261242 not in JDK 17). Since the patch is not clean anyway, I've included the one-liner of JDK-8354878 right away.Testing:
Progress
Issues
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk17u-dev.git pull/4271/head:pull/4271$ git checkout pull/4271Update a local copy of the PR:
$ git checkout pull/4271$ git pull https://git.openjdk.org/jdk17u-dev.git pull/4271/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 4271View PR using the GUI difftool:
$ git pr show -t 4271Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk17u-dev/pull/4271.diff
Using Webrev
Link to Webrev Comment