RFR: 8343191: Cgroup v1 subsystem fails to set subsystem path [v15]

Severin Gehwolf sgehwolf at openjdk.org
Tue Feb 25 17:14:10 UTC 2025


On Tue, 25 Feb 2025 16:31:05 GMT, Sergey Chernyshev <schernyshev at openjdk.org> wrote:

>> src/hotspot/os/linux/cgroupV1Subsystem_linux.cpp line 42:
>> 
>>> 40:  * When runs in a container, the method handles the case
>>> 41:  * when a process is moved between cgroups.
>>> 42:  */
>> 
>> This needs to explain exactly what is happening when. The current comment isn't even remotely explaining in detail what it does. What does "... handles the case when a process is moved between cgroups" mean exactly?
>
>> This needs to explain exactly what is happening when. The current comment isn't even remotely explaining in detail what it does. What does "... handles the case when a process is moved between cgroups" mean exactly?
> 
> Either it shall be a high level comment such as in your suggestion [here](https://github.com/openjdk/jdk/pull/21808#pullrequestreview-2620718790), or a deeper description in detail what happens where. Could you please be more specific on what kind of description is required and where? Please note the method has inline comments that are fairly self describing. In the meanwhile I'll try to add a description of what "a process is moved between cgroups" exactly means.

A high level description that mentions what the function does. The inline comments are not very self describing for anyone not having written the patch.

Example:

Sets the subsystem path for a controller. The common container case is
handled by XXX. When a process has been moved from a cgroup to another
the following happens:
 - A
 - B
 - C


I believe this is desperately needed.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21808#discussion_r1970211567


More information about the core-libs-dev mailing list