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

[Severin Gehwolf]

On Thu, 5 Sep 2024 18:25:49 GMT, Ashutosh Mehra <asmehra at openjdk.org> wrote:

>>> @jerboaa I agree it would probably just result in incorrect reporting of available swap space. I guess only JFR is using that stat. I can't really comment on the importance of the swap value for JFR. If we can live with this issue I guess then this scenario can be ignored.
>> 
>> I've filed https://bugs.openjdk.org/browse/JDK-8339593 to summarize this. Right now, I don't think it's critical enough to warrant implementing it in all possible cases. See the bug for details. The goal of this fix would be to prevent spurious OOM kills when the JDK runs in a systemd service with some resource control applied. Which I think this fix covers.
>> 
>>> > FWIW, I wasn't able to re-create such a config on cg v1 exposing this swap issue. Were you?
>>> 
>>> I have cg v2 on my system and I can produce this kind configuration manually:
>>> 
>>>     1. A `test` cgroup with `memory.max` set to ~3g and `memory.swap.max` to `max`
>>> 
>>>     2. A child of `test` called `bar` with `memory.max` set to `max` and and `memory.swap.max` set to 0.
>>> 
>>> 
>>> ```
>>> $ pwd
>>> /sys/fs/cgroup/test
>>> $ cat memory.max
>>> 3145728000
>>> $ cat memory.swap.max
>>> max
>>> $ cd bar
>>> $ pwd
>>> /sys/fs/cgroup/test/bar
>>> $ cat memory.max
>>> max
>>> $ cat memory.swap.max
>>> 0
>>> ```
>>> 
>>> I don't know if this can be replicated with systemd. I haven't tried that.
>> 
>> It's possible and I [did on cg v2 as mentioned in my previous comment](https://github.com/openjdk/jdk/pull/20646#issuecomment-2328467306). In any case, the cost/benefit calculation to cover all possible configs out there isn't worth it just yet.
>> 
>>> I also wonder why don't we cache the memory limits (and other metrics like cpu count) instead of reading from the config file every time?
>> 
>> It's already there? This has been added with https://bugs.openjdk.org/browse/JDK-8227006 and https://bugs.openjdk.org/browse/JDK-8232207. The current code doing it for CPU is [here](https://github.com/openjdk/jdk/blob/96a0502d624e3eff1b00a7c63e8b3a27870b475e/src/hotspot/os/linux/cgroupSubsystem_linux.cpp#L562-L584) and for memory it's [here](https://github.com/openjdk/jdk/blob/master/src/hotspot/os/linux/cgroupSubsystem_linux.cpp#L595-L607). It's only been done for the core metrics, though.
>
>> I've filed https://bugs.openjdk.org/browse/JDK-8339593 to summarize this. Right now, I don't think it's critical enough to warrant implementing it in all possible cases. 
> 
> Okay, I am fine with this.
> 
>> It's already there? This has been added with https://bugs.openjdk.org/browse/JDK-8227006 and https://bugs.openjdk.org/browse/JDK-8232207. The current code doing it for CPU is [here](https://github.com/openjdk/jdk/blob/96a0502d624e3eff1b00a7c63e8b3a27870b475e/src/hotspot/os/linux/cgroupSubsystem_linux.cpp#L562-L584) and for memory it's [here](https://github.com/openjdk/jdk/blob/master/src/hotspot/os/linux/cgroupSubsystem_linux.cpp#L595-L607). It's only been done for the core metrics, though.
> 
> Thanks for pointing it out. Somehow I missed it.
> 
> And thanks for running through the scenario.

Thanks for the review @ashu-mehra!

@tstuefe do you mind doing a re-review of this? I think since you've last seen it the only addition was https://github.com/openjdk/jdk/commit/0e17c9914f187d9ee1af32674f03b06e8173bc18 Many thanks in advance!

-------------

PR Comment: https://git.openjdk.org/jdk/pull/20646#issuecomment-2333552609