RFR: 8322420: [Linux] cgroup v2: Limits in parent nested control groups are not detected [v19]

Severin Gehwolf sgehwolf at openjdk.org
Wed Aug 14 16:53:57 UTC 2024


On Wed, 14 Aug 2024 03:26:02 GMT, Jan Kratochvil <jkratochvil at 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


More information about the core-libs-dev mailing list