On Wed, 14 Aug 2024 03:26:02 GMT, Jan Kratochvil <jkratoch...@openjdk.org> wrote:
>> test/hotspot/jtreg/containers/cgroup/NestedCgroup.java line 217: >> >>> 215: >>> 216: // KFAIL - verify the >>> CgroupSubsystem::initialize_hierarchy() and >>> jdk.internal.platform.CgroupSubsystem.initializeHierarchy() bug >>> 217: // TestTwoLimits does not see the lower MEMORY_MAX_OUTER >>> limit. >> >> Remove this obsolete(?) comment. > > This comment is documenting what the `TestTwoLimits` testcase does. Which I > find useful. > It is KFAIL - Known Failure - the current OpenJDK code does not properly > simulate what Linux kernel does. If you do: > > cgset -r memory.max=$[1024*1024*1024] a/b > cgset -r memory.max=$[512*1024] a > cgexec -g memory:a/b java... > > Then OpenJDK thinks it is `1024*1024*1024 KB` but Linux kernel will limit > OpenJDK to `512*1024 KB`. So this problem is documented and tested. I see. Please update the comment. `KFAIL` isn't something I was familiar with as an acronym. Code/method references in tests don't seem appropriate since as soon as the code gets changed the comment is out of date/misleading. Refrain from having references to code. More general remarks: - No well-behaved system should set the limit at a deeper nesting level to anything non-`max`. That's why the code currently doesn't handle it. It's an optimization if you will and I don't see the benefit of walking the full hierarchy every time for such bad systems. - Please use a more high level comment for this test. Something like `"Since only non-well-behaved systems have a higher limit (other than unlimited) at a lower hierarchy level in the tree we stop iterating once we've encountered a limit that is not unlimited (-1). This test simulates this by asserting that the first non-unlimited value is detected by HotSpot, not the actual enforced limit of the kernel higher up the hierarchy."` ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/17198#discussion_r1716819006
