On Fri, 7 Mar 2025 04:34:24 GMT, David Holmes wrote:
>>> OK for me now. `test_cgroupSubsystem_linux.cpp` needs a copyright update as
>>> well.
>>
>> Thanks for your review @jerboaa ! I cheched the
>> test_cgroupSubsystem_linux.cpp, it's already updated to 2025 in the master
>> branch.
>
> @se
On Thu, 31 Oct 2024 15:00:25 GMT, Sergey Chernyshev
wrote:
> Cgroup V1 subsustem fails to initialize mounted controllers properly in
> certain cases, that may lead to controllers left undetected/inactive. We
> observed the behavior in CloudFoundry deployments, it affects also host
On Sun, 2 Mar 2025 19:07:32 GMT, Severin Gehwolf wrote:
> OK for me now. `test_cgroupSubsystem_linux.cpp` needs a copyright update as
> well.
Thanks for your review @jerboaa ! I cheched the test_cgroupSubsystem_linux.cpp,
it's already updated to 2025 in the master branch.
-
PR Co
On Fri, 28 Feb 2025 17:08:19 GMT, Severin Gehwolf wrote:
>> Sergey Chernyshev has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> updated comment
>
> src/java.base/linux/classes/
On Fri, 28 Feb 2025 12:27:13 GMT, Matthias Baesken wrote:
>> Sergey Chernyshev has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> updated comment
>
> test/jdk/jdk/internal/platform/cgroup/CgroupV1Subsystem
On Fri, 28 Feb 2025 17:07:59 GMT, Severin Gehwolf wrote:
>> src/hotspot/os/linux/cgroupV2Subsystem_linux.cpp line 2:
>>
>>> 1: /*
>>> 2: * Copyright (c) 2020, 2024, Red Hat Inc.
>>
>> Guess this must be 2025 now ? Same for other files ...
>
> yes indeed.
indeed, updated the copyright headers.
For example:
>
> 5:cpuacct,cpu,cpuset:/daemons
> ...
> [3] This field contains the pathname of the control group
>in the hierarchy to which the process belongs. This
>pathname is relative to the mount point
On Tue, 25 Feb 2025 17:11:30 GMT, Severin Gehwolf wrote:
>>> 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?
>>
>> Eit
For example:
>
> 5:cpuacct,cpu,cpuset:/daemons
> ...
> [3] This field contains the pathname of the control group
>in the hierarchy to which the process belongs. This
>pathname is relative to the mount point
On Mon, 24 Feb 2025 23:33:01 GMT, Sergey Chernyshev
wrote:
>> `CgroupV1Controller::set_subsystem_path` needs high level comment update to
>> describe the logic happening.
>>
>> Testing:
>>>>And after the patch this would become this, right?
>>>>
On Tue, 25 Feb 2025 14:06:03 GMT, Severin Gehwolf wrote:
> 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
On Mon, 17 Feb 2025 14:04:03 GMT, Severin Gehwolf wrote:
> The above case, doesn't seem to be reflected by any gtest test case (or
> others), please add those.
The subgroup path reduction is covered by
`TestMemoryWithSubgroups#testMemoryLimitSubgroupV1` (it wouldn't be possible in
gtests as i
On Fri, 21 Feb 2025 20:05:07 GMT, Sergey Chernyshev
wrote:
>> Good catch, but as long as cgp#nameCount may change in the loop, this exact
>> patch i cannot take.
>
> How about this?
>
> int currentNameCount = cgp.getNameCount();
> cgp = (currentNameCount > 1) ? c
For example:
>
> 5:cpuacct,cpu,cpuset:/daemons
> ...
> [3] This field contains the pathname of the control group
>in the hierarchy to which the process belongs. This
>pathname is relative to the mount point
On Mon, 17 Feb 2025 14:04:03 GMT, Severin Gehwolf wrote:
> `CgroupV1Controller::set_subsystem_path` needs high level comment update to
> describe the logic happening.
>
Done, added
-
PR Comment: https://git.openjdk.org/jdk/pull/21808#issuecomment-2679664923
On Thu, 20 Feb 2025 17:32:54 GMT, Sergey Chernyshev
wrote:
>> src/java.base/linux/classes/jdk/internal/platform/cgroupv1/CgroupV1SubsystemController.java
>> line 73:
>>
>>> 71: break;
>>> 72: }
>
On Mon, 17 Feb 2025 11:28:38 GMT, Severin Gehwolf wrote:
>> Sergey Chernyshev has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> added details in the log message
>
> test/hotspot/jtreg/containers/docker/TestMe
For example:
>
> 5:cpuacct,cpu,cpuset:/daemons
> ...
> [3] This field contains the pathname of the control group
>in the hierarchy to which the process belongs. This
>pathname is relative to the mount point
For example:
>
> 5:cpuacct,cpu,cpuset:/daemons
> ...
> [3] This field contains the pathname of the control group
>in the hierarchy to which the process belongs. This
>pathname is relative to the mount point
On Mon, 17 Feb 2025 11:09:12 GMT, Severin Gehwolf wrote:
>> Sergey Chernyshev has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> added details in the log message
>
> src/java.base/linux/classes/
For example:
>
> 5:cpuacct,cpu,cpuset:/daemons
> ...
> [3] This field contains the pathname of the control group
>in the hierarchy to which the process belongs. This
>pathname is relative to the mount point
On Mon, 17 Feb 2025 15:48:51 GMT, Matthias Baesken wrote:
>> Sergey Chernyshev has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> added details in the log message
>
> src/java.base/linux/classes/
On Thu, 12 Dec 2024 01:11:33 GMT, Sergey Chernyshev
wrote:
>> Cgroup V1 subsustem fails to initialize mounted controllers properly in
>> certain cases, that may lead to controllers left undetected/inactive. We
>> observed the behavior in CloudFoundry deployments, it
On Wed, 11 Dec 2024 15:24:12 GMT, Severin Gehwolf wrote:
>> Would you recommand also to include the paths in that warning? Something like
>> ```cgroup controller path at '/sys/fs/cgroup' seems to have moved to
>> '../../test', detected limits won't be accurate```
>> This way it will have all the
On Wed, 11 Dec 2024 15:24:12 GMT, Severin Gehwolf wrote:
>> Would you recommand also to include the paths in that warning? Something like
>> ```cgroup controller path at '/sys/fs/cgroup' seems to have moved to
>> '../../test', detected limits won't be accurate```
>> This way it will have all the
For example:
>
> 5:cpuacct,cpu,cpuset:/daemons
> ...
> [3] This field contains the pathname of the control group
>in the hierarchy to which the process belongs. This
>pathname is relative to the mount point
On Fri, 6 Dec 2024 09:51:52 GMT, Severin Gehwolf wrote:
>> Sergey Chernyshev has updated the pull request with a new target base due to
>> a merge or a rebase. The pull request now contains 13 commits:
>>
>> - diverged after integration of JDK-8344177
>>
For example:
>
> 5:cpuacct,cpu,cpuset:/daemons
> ...
> [3] This field contains the pathname of the control group
>in the hierarchy to which the process belongs. This
>pathname is relative to the mount point
For example:
>
> 5:cpuacct,cpu,cpuset:/daemons
> ...
> [3] This field contains the pathname of the control group
>in the hierarchy to which the process belongs. This
>pathname is relative to the mount point
On Sat, 30 Nov 2024 00:30:02 GMT, Sergey Chernyshev
wrote:
>> Cgroup V1 subsustem fails to initialize mounted controllers properly in
>> certain cases, that may lead to controllers left undetected/inactive. We
>> observed the behavior in CloudFoundry deployments, it
On Mon, 2 Dec 2024 10:47:01 GMT, Severin Gehwolf wrote:
> And after the patch this would become this, right?
>
> ```
> /sys/fs/cgroup/cpu,cpuacct/system.slice/garden.service/garden/bad/2f57368b-0eda-4e52-64d8-af5c
> /sys/fs/cgroup/cpu,cpuacct/
> ```
It depends on whether it was a subgroup in th
On Fri, 29 Nov 2024 17:19:34 GMT, Severin Gehwolf wrote:
> In the cloudflare case we'd end up with a subsystem path of
> `/sys/fs/cgroup/cpu,cpuacct/system.slice/garden.service/garden/bad/2f57368b-0eda-4e52-64d8-af5c`.
> Since the cgroup_path != _root we trigger path adjustment increasing the
On Wed, 27 Nov 2024 11:07:18 GMT, Severin Gehwolf wrote:
> Right. I'm still not convinced this extra reduction buys us much. The adjust
> controller logic will handle it if kept as is in the Metrics version.
The adjust controller logic won't handle it, because it reduces the path from
child to
On Wed, 27 Nov 2024 10:18:24 GMT, Severin Gehwolf wrote:
>> Sergey Chernyshev has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> adjust path suffix in cgroup (v1) version specific code, when root !=
>> c
For example:
>
> 5:cpuacct,cpu,cpuset:/daemons
> ...
> [3] This field contains the pathname of the control group
>in the hierarchy to which the process belongs. This
>pathname is relative to the mount point
For example:
>
> 5:cpuacct,cpu,cpuset:/daemons
> ...
> [3] This field contains the pathname of the control group
>in the hierarchy to which the process belongs. This
>pathname is relative to the mount point
On Wed, 27 Nov 2024 09:08:30 GMT, Sergey Chernyshev
wrote:
> Done. Please see the updated PR.
The metrics part still needs the update - in the cgroup version specific
`setPath()`.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/21808#discussion_r1860451322
On Mon, 25 Nov 2024 09:44:33 GMT, Severin Gehwolf wrote:
> ```
> "echo '+memory' > /sys/fs/cgroup/memory/cgroup.subtree_control ; "
> ```
>
> ... relies on the user in the container image to be `root`. So depending on
> which base image is being used - by means of
> `-Djdk.test.docker.image.na
On Mon, 25 Nov 2024 09:40:03 GMT, Severin Gehwolf wrote:
> Version specific code can be had in `set_subsystem_path()` of the
> corresponding impl (like an earlier version of your patch). `lowest_limit`
> and `limit_cg_path` fixes are version agnostic and can and should be fixed in
> `CgroupUti
For example:
>
> 5:cpuacct,cpu,cpuset:/daemons
> ...
> [3] This field contains the pathname of the control group
>in the hierarchy to which the process belongs. This
>pathname is relative to the mount point
On Sat, 23 Nov 2024 00:06:26 GMT, Sergey Chernyshev
wrote:
> It looks like there's no way to see at this point, if we are in cgroup v1 or
> v2 - if I am not mistaken.
On the other hand, a type parameter can be added to
`CgroupUtil::adjust_controller()`. Would you recomme
On Fri, 22 Nov 2024 16:48:04 GMT, Severin Gehwolf wrote:
> The added code in `CgroupUtil::adjust_controller` runs for cg v1 and cg v2
> when path adjustment is deemed needed. So I'm not clear why it's needed for
> cg v2
It looks like there's no way to see at this point, if we are in cgroup v1
On Fri, 22 Nov 2024 10:57:31 GMT, Severin Gehwolf wrote:
> One thing to note is that the new test requires root privileges (AFAIK). We
> should skip the test if we are being run as root.
The test works just like other docker tests, root privileges are not required.
In systems that can't run do
On Fri, 22 Nov 2024 10:54:34 GMT, Severin Gehwolf wrote:
> Good spot! How about this to fix it?
>
> ```
> jlong limit = mem->read_memory_limit_in_bytes(phys_mem);
> jlong lowest_limit = limit < 0 ? phys_mem: limit;
> ```
Make sense to me.
> I'm worried about the added complexity. 1.) Is this
For example:
>
> 5:cpuacct,cpu,cpuset:/daemons
> ...
> [3] This field contains the pathname of the control group
>in the hierarchy to which the process belongs. This
>pathname is relative to the mount point
On Tue, 12 Nov 2024 23:29:47 GMT, Sergey Chernyshev
wrote:
>> src/java.base/linux/classes/jdk/internal/platform/cgroupv1/CgroupV1SubsystemController.java
>> line 46:
>>
>>> 44: }
>>> 45:
>>> 46: public void setPath(String cgroupPath) {
On Tue, 12 Nov 2024 19:41:50 GMT, Severin Gehwolf wrote:
>> Sergey Chernyshev has updated the pull request with a new target base due to
>> a merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contai
For example:
>
> 5:cpuacct,cpu,cpuset:/daemons
> ...
> [3] This field contains the pathname of the control group
>in the hierarchy to which the process belongs. This
>pathname is relative to the mount point
On Thu, 7 Nov 2024 22:31:21 GMT, Sergey Chernyshev
wrote:
>> Cgroup V1 subsustem fails to initialize mounted controllers properly in
>> certain cases, that may lead to controllers left undetected/inactive. We
>> observed the behavior in CloudFoundry deployments, it
On Mon, 11 Nov 2024 10:23:02 GMT, Severin Gehwolf wrote:
> The JBS issue doesn't mention `NullPointerException`. It would be good to
> list the observed NPE issue.
Example for NPE:
public class Test {
public static void main(String[] args) {
java.lang.management.ManagementFactory.
On Mon, 11 Nov 2024 15:48:48 GMT, Severin Gehwolf wrote:
> So on cg v1 you start out and end with a `subsystem_path() == null` and on cg
> v2 you start out and end with a `subsystem_path() ==
> /../../../../../../test`. In both cases the memory limit of 400m won't be
> detected.
Only when doc
On Mon, 11 Nov 2024 15:16:15 GMT, Severin Gehwolf wrote:
> On cg v2, on the other hand, `set_subsystem_path()` will never set the path
> to a `null` value.
Exactly. That's why JDK-8322420 is not in effect and also
[JDK-8336881](https://bugs.openjdk.org/browse/JDK-8336881) does not fix it on
J
On Thu, 7 Nov 2024 22:31:21 GMT, Sergey Chernyshev
wrote:
>> Cgroup V1 subsustem fails to initialize mounted controllers properly in
>> certain cases, that may lead to controllers left undetected/inactive. We
>> observed the behavior in CloudFoundry deployments, it
On Thu, 7 Nov 2024 22:31:21 GMT, Sergey Chernyshev
wrote:
>> Cgroup V1 subsustem fails to initialize mounted controllers properly in
>> certain cases, that may lead to controllers left undetected/inactive. We
>> observed the behavior in CloudFoundry deployments, it
For example:
>
> 5:cpuacct,cpu,cpuset:/daemons
> ...
> [3] This field contains the pathname of the control group
>in the hierarchy to which the process belongs. This
>pathname is relative to the mount point
For example:
>
> 5:cpuacct,cpu,cpuset:/daemons
> ...
> [3] This field contains the pathname of the control group
>in the hierarchy to which the process belongs. This
>pathname is relative to the mount point
On Thu, 31 Oct 2024 15:00:25 GMT, Sergey Chernyshev
wrote:
> Cgroup V1 subsustem fails to initialize mounted controllers properly in
> certain cases, that may lead to controllers left undetected/inactive. We
> observed the behavior in CloudFoundry deployments, it affects also host
On Thu, 31 Oct 2024 15:00:25 GMT, Sergey Chernyshev
wrote:
> Cgroup V1 subsustem fails to initialize mounted controllers properly in
> certain cases, that may lead to controllers left undetected/inactive. We
> observed the behavior in CloudFoundry deployments, it affects also host
On Thu, 31 Oct 2024 15:00:25 GMT, Sergey Chernyshev
wrote:
> Cgroup V1 subsustem fails to initialize mounted controllers properly in
> certain cases, that may lead to controllers left undetected/inactive. We
> observed the behavior in CloudFoundry deployments, it affects also host
Cgroup V1 subsustem fails to initialize mounted controllers properly in certain
cases, that may lead to controllers left undetected/inactive. We observed the
behavior in CloudFoundry deployments, it affects also host systems.
The relevant /proc/self/mountinfo line is
2207 2196 0:43
/system.sl
Hi Alan,
Thank you for your comments! I will post this to net-nev too as you
suggested.
Am 28.03.24 um 00:23 schrieb Alan Bateman:
On 27/03/2024 17:05, Sergey Chernyshev wrote:
In the discussion of .ofLiteral() it was not concluded that
.ofPosixLiteral() would be insecure or
t for no good reason. I think this was also the conclusion for the
ofLiteral() api.
Sergey Chernyshev wrote on 26. Mar 2024 17:51 (GMT +01:00):
Hello Core Libs Dev team,
I would like to propose a PR to extend the InetAddress API in JDK 23,
ows exactly
what it does.
Please share your thoughts on whether such a change might be desirable
in JDK 23.
Thank you for your help!
Best regards
Sergey Chernyshev
[1] https://bugs.openjdk.org/browse/JDK-8315767
[2] https://bugs.openjdk.org/browse/JDK-8272215
63 matches
Mail list logo
