On Tue, 25 Feb 2025 16:31:05 GMT, Sergey Chernyshev <schernys...@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
