Re: RFR: 8322420: [Linux] cgroup v2: Limits in parent nested control groups are not detected [v8]
On Tue, 7 May 2024 09:36:10 GMT, Jan Kratochvil wrote: > Should JDK still support `memory.use_hierarchy == 0`? IMO, no. Just get rid of it and assume hierarchical everywhere. We'd be walking the hierarchy for other (lower limits), which should cover this case on those legacy systems. - PR Comment: https://git.openjdk.org/jdk/pull/17198#issuecomment-2097892763
Re: RFR: 8322420: [Linux] cgroup v2: Limits in parent nested control groups are not detected [v8]
On Sun, 10 Mar 2024 14:40:09 GMT, Jan Kratochvil wrote: >> The testcase requires root permissions. >> >> Designed by Severin Gehwolf, implemented by Jan Kratochvil. > > Jan Kratochvil has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 35 commits: > > - Fix whitespace > - Merge branch 'master' into master-cgroup > >Conflicts: > test/hotspot/gtest/os/linux/test_cgroupSubsystem_linux.cpp > - Fix gtest > - Update the Java part > - Fix cgroup1 backward compatibility message > - Merge remote-tracking branch 'centos79/master-cgroup' into master-cgroup > - Disable cgroup.subtree_control testcase on cgroup1 > - Fix gtest > - Merge branch 'master' into master-cgroup > - Merge remote-tracking branch 'f38crac/master-cgroup' into master-cgroup > - ... and 25 more: https://git.openjdk.org/jdk/compare/243cb098...39c90162 Should JDK still support `memory.use_hierarchy == 0`? That has been already removed from Linux kernel: https://github.com/torvalds/linux/commit/bef8620cd8e0a117c1a0719604052e424eb418f9 This patch is apparently present even in kernel-3.10.0-1160.118.1.el7.x86_64 (CentOS-7.9 updates) It is not present in kernel-3.10.0-1160.el7.x86_64 (CentOS-7.9 release) Still CentOS-7 is almost EOLed, is there any other distro for cgroup1? - PR Comment: https://git.openjdk.org/jdk/pull/17198#issuecomment-2097873090
Re: RFR: 8322420: [Linux] cgroup v2: Limits in parent nested control groups are not detected [v8]
On Sun, 10 Mar 2024 14:40:09 GMT, Jan Kratochvil wrote: >> The testcase requires root permissions. >> >> Designed by Severin Gehwolf, implemented by Jan Kratochvil. > > Jan Kratochvil has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 35 commits: > > - Fix whitespace > - Merge branch 'master' into master-cgroup > >Conflicts: > test/hotspot/gtest/os/linux/test_cgroupSubsystem_linux.cpp > - Fix gtest > - Update the Java part > - Fix cgroup1 backward compatibility message > - Merge remote-tracking branch 'centos79/master-cgroup' into master-cgroup > - Disable cgroup.subtree_control testcase on cgroup1 > - Fix gtest > - Merge branch 'master' into master-cgroup > - Merge remote-tracking branch 'f38crac/master-cgroup' into master-cgroup > - ... and 25 more: https://git.openjdk.org/jdk/compare/243cb098...39c90162 > Should I rebase it on top of the commit > [jerboaa@92aaa6f](https://github.com/jerboaa/jdk/commit/92aaa6fd7e3ff8b64de064fecfcd725a157cb5bb) > or wait until you finish it? Up to you. I'll keep you posted once the PRs are out. - PR Comment: https://git.openjdk.org/jdk/pull/17198#issuecomment-2085319806
Re: RFR: 8322420: [Linux] cgroup v2: Limits in parent nested control groups are not detected [v8]
On Sun, 10 Mar 2024 14:40:09 GMT, Jan Kratochvil wrote: >> The testcase requires root permissions. >> >> Designed by Severin Gehwolf, implemented by Jan Kratochvil. > > Jan Kratochvil has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 35 commits: > > - Fix whitespace > - Merge branch 'master' into master-cgroup > >Conflicts: > test/hotspot/gtest/os/linux/test_cgroupSubsystem_linux.cpp > - Fix gtest > - Update the Java part > - Fix cgroup1 backward compatibility message > - Merge remote-tracking branch 'centos79/master-cgroup' into master-cgroup > - Disable cgroup.subtree_control testcase on cgroup1 > - Fix gtest > - Merge branch 'master' into master-cgroup > - Merge remote-tracking branch 'f38crac/master-cgroup' into master-cgroup > - ... and 25 more: https://git.openjdk.org/jdk/compare/243cb098...39c90162 Should I rebase it on top of the commit https://github.com/jerboaa/jdk/commit/92aaa6fd7e3ff8b64de064fecfcd725a157cb5bb or wait until you finish it? - PR Comment: https://git.openjdk.org/jdk/pull/17198#issuecomment-2085243290
Re: RFR: 8322420: [Linux] cgroup v2: Limits in parent nested control groups are not detected [v8]
On Tue, 30 Apr 2024 07:01:16 GMT, Jan Kratochvil wrote: > * Will the patch be accepted only for memory or it has to support also CPU? It should be fine for memory only for a start, but we should allow for on-boarding of other controllers as well. > * Should I code it on top of OpenJDK trunk or on top of > [jerboaa@92aaa6f](https://github.com/jerboaa/jdk/commit/92aaa6fd7e3ff8b64de064fecfcd725a157cb5bb) > ? Since this patch solves a similar problem than https://bugs.openjdk.java.net/browse/JDK-8217338 did at the time when only cg v1 was supported I'd feel more comfortable in paying the technical debt and recode it so that cg v1 and cg v2 behaves the same or at least similar. That should make the code cleaner, easier to test and maintain in the long run. So I'm thinking on top of the refactoring. I'll try to prioritize those work items accordingly. Does that sound OK? - PR Comment: https://git.openjdk.org/jdk/pull/17198#issuecomment-2085186173
Re: RFR: 8322420: [Linux] cgroup v2: Limits in parent nested control groups are not detected [v8]
On Sun, 10 Mar 2024 14:40:09 GMT, Jan Kratochvil wrote: >> The testcase requires root permissions. >> >> Designed by Severin Gehwolf, implemented by Jan Kratochvil. > > Jan Kratochvil has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 35 commits: > > - Fix whitespace > - Merge branch 'master' into master-cgroup > >Conflicts: > test/hotspot/gtest/os/linux/test_cgroupSubsystem_linux.cpp > - Fix gtest > - Update the Java part > - Fix cgroup1 backward compatibility message > - Merge remote-tracking branch 'centos79/master-cgroup' into master-cgroup > - Disable cgroup.subtree_control testcase on cgroup1 > - Fix gtest > - Merge branch 'master' into master-cgroup > - Merge remote-tracking branch 'f38crac/master-cgroup' into master-cgroup > - ... and 25 more: https://git.openjdk.org/jdk/compare/243cb098...39c90162 - Will the patch be accepted only for memory or it has to support also CPU? - Should I code it on top of OpenJDK trunk or on top of https://github.com/jerboaa/jdk/commit/92aaa6fd7e3ff8b64de064fecfcd725a157cb5bb ? - PR Comment: https://git.openjdk.org/jdk/pull/17198#issuecomment-2084523889
Re: RFR: 8322420: [Linux] cgroup v2: Limits in parent nested control groups are not detected [v8]
On Sun, 10 Mar 2024 14:40:09 GMT, Jan Kratochvil wrote: >> The testcase requires root permissions. >> >> Designed by Severin Gehwolf, implemented by Jan Kratochvil. > > Jan Kratochvil has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 35 commits: > > - Fix whitespace > - Merge branch 'master' into master-cgroup > >Conflicts: > test/hotspot/gtest/os/linux/test_cgroupSubsystem_linux.cpp > - Fix gtest > - Update the Java part > - Fix cgroup1 backward compatibility message > - Merge remote-tracking branch 'centos79/master-cgroup' into master-cgroup > - Disable cgroup.subtree_control testcase on cgroup1 > - Fix gtest > - Merge branch 'master' into master-cgroup > - Merge remote-tracking branch 'f38crac/master-cgroup' into master-cgroup > - ... and 25 more: https://git.openjdk.org/jdk/compare/243cb098...39c90162 Thanks for the updates. I like that we have consistency between cgv1 and cgv2 in the latest version in terms of hierarchical limit. What would be even better is to get consistency between CPU and memory lookup (if the restriction is enforced higher up the hierarchy). That is, it would be ideal to make `initialize_hierarchy()` controller specific. Meanwhile I've been working on [some refactoring](https://github.com/jerboaa/jdk/commit/92aaa6fd7e3ff8b64de064fecfcd725a157cb5bb) which builds on top of [JDK-8302744](https://bugs.openjdk.org/browse/JDK-8302744) so as to make the code a bit nicer once this integrates. Then, the idea would be to use scratch controllers (`CgroupCpuController` and `CgroupMemoryController`) to determine whether or not there is a limit and figure out the actual path on a per-controller specific way - (use `CgroupMemoryController->read_memory_limit_in_bytes(phys_mem)` and `CgroupUtil::processor_count(CgroupCpuController* cpu_ctrl, int host_cpus)` in the process). Does that make sense? A few other observations: - The common case is when the JVM runs in a container. Then, the cgroup path is `/` on cgv2 and the and `root_mount == cgroup_path` on cgv1. We don't need to do the extra processing on those systems as the limit will be at the leaf. - The (fairly) uncommon case is the host case where the cgroup limit is applied elsewhere (e.g. systemd slice). This is where we'd need the hierarchy walk. - When we need to walk the hierarchy, we start at the longest path and only traverse if there is _NO_ limit. A system which sets a higher, limit (that isn't `max`), seems ill-defined and I've not come across one. As soon as we've found a lower value than unlimited (`-1`), we stop. Since cg2 is hierarchical, the lowest limit will affect the entire tree (corollary: higher values further down from that point won't have an effect): ``` /a/b --> memory.max 300 `- /c --> memory.max max (this wouldn't have any effect, therefore can be ignored). ``` - PR Comment: https://git.openjdk.org/jdk/pull/17198#issuecomment-2077263573
Re: RFR: 8322420: [Linux] cgroup v2: Limits in parent nested control groups are not detected [v8]
On Sun, 10 Mar 2024 14:40:09 GMT, Jan Kratochvil wrote: >> The testcase requires root permissions. >> >> Designed by Severin Gehwolf, implemented by Jan Kratochvil. > > Jan Kratochvil has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 35 commits: > > - Fix whitespace > - Merge branch 'master' into master-cgroup > >Conflicts: > test/hotspot/gtest/os/linux/test_cgroupSubsystem_linux.cpp > - Fix gtest > - Update the Java part > - Fix cgroup1 backward compatibility message > - Merge remote-tracking branch 'centos79/master-cgroup' into master-cgroup > - Disable cgroup.subtree_control testcase on cgroup1 > - Fix gtest > - Merge branch 'master' into master-cgroup > - Merge remote-tracking branch 'f38crac/master-cgroup' into master-cgroup > - ... and 25 more: https://git.openjdk.org/jdk/compare/243cb098...39c90162 ping, it is already 6+ weeks since last change - PR Comment: https://git.openjdk.org/jdk/pull/17198#issuecomment-2076286758
Re: RFR: 8322420: [Linux] cgroup v2: Limits in parent nested control groups are not detected [v8]
> The testcase requires root permissions. > > Designed by Severin Gehwolf, implemented by Jan Kratochvil. Jan Kratochvil has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 35 commits: - Fix whitespace - Merge branch 'master' into master-cgroup Conflicts: test/hotspot/gtest/os/linux/test_cgroupSubsystem_linux.cpp - Fix gtest - Update the Java part - Fix cgroup1 backward compatibility message - Merge remote-tracking branch 'centos79/master-cgroup' into master-cgroup - Disable cgroup.subtree_control testcase on cgroup1 - Fix gtest - Merge branch 'master' into master-cgroup - Merge remote-tracking branch 'f38crac/master-cgroup' into master-cgroup - ... and 25 more: https://git.openjdk.org/jdk/compare/243cb098...39c90162 - Changes: https://git.openjdk.org/jdk/pull/17198/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=17198&range=07 Stats: 712 lines in 17 files changed: 438 ins; 214 del; 60 mod Patch: https://git.openjdk.org/jdk/pull/17198.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17198/head:pull/17198 PR: https://git.openjdk.org/jdk/pull/17198
