On Thu, 18 Jul 2024 14:48:02 GMT, Jan Kratochvil <jkratoch...@openjdk.org> wrote:
>> The testcase requires root permissions. >> >> Fix by Severin Gehwolf. >> Testcase by Jan Kratochvil. > > Jan Kratochvil has updated the pull request incrementally with one additional > commit since the last revision: > > Unify 4 copies of adjust_controller() This patch seems OK (though, I'm biased). Please clean up the test. src/hotspot/os/linux/cgroupUtil_linux.cpp line 64: > 62: return cpu->adjust_controller(cpu_total); > 63: } > 64: return cpu; I guess an alternative - and maybe more readable solution - would be to inline `cpu->adjust_controller()` and `mem->adjust_controller()` code here. We have cg version agnostic api to query the limits. We'd just need accessors for `cgroup_path()` and a setter, `set_cgroup_path()` in `CgroupCpuController/CgroupMemoryController` impls. test/hotspot/jtreg/containers/cgroup/NestedCgroup.java line 28: > 26: * @key cgroups > 27: * @requires os.family == "linux" > 28: * @requires vm.flagless If you really want to keep that test, then we should add support for the `libcg` dependency in `jtreg-ext` lib so that we can write `requires os.family == "linux" & dep.libcgroup` or some such. ------------- PR Review: https://git.openjdk.org/jdk/pull/17198#pullrequestreview-2191041991 PR Review Comment: https://git.openjdk.org/jdk/pull/17198#discussion_r1686225373 PR Review Comment: https://git.openjdk.org/jdk/pull/17198#discussion_r1686230300