8343191: Cgroup v1 subsystem fails to set subsystem path

src/hotspot/os/linux/cgroupUtil_linux.cpp

@@ -63,6 +63,11 @@ void CgroupUtil::adjust_controller(CgroupMemoryController* mem) {
   char* limit_cg_path = nullptr;
   jlong limit = mem->read_memory_limit_in_bytes(phys_mem);
   jlong lowest_limit = phys_mem;
+  if (limit > 0 && limit < lowest_limit) {
+    lowest_limit = limit;
+    os::free(limit_cg_path); // handles nullptr
+    limit_cg_path = os::strdup(cg_path);
+  }
   while ((last_slash = strrchr(cg_path, '/')) != cg_path) {
     *last_slash = '\0'; // strip path
     // update to shortened path and try again
@@ -118,6 +123,11 @@ void CgroupUtil::adjust_controller(CgroupCpuController* cpu) {
   int cpus = CgroupUtil::processor_count(cpu, host_cpus);
   int lowest_limit = host_cpus;
   char* limit_cg_path = nullptr;
+  if (cpus != host_cpus && cpus < lowest_limit) {
+    lowest_limit = cpus;
+    os::free(limit_cg_path); // handles nullptr
+    limit_cg_path = os::strdup(cg_path);
+  }
   while ((last_slash = strrchr(cg_path, '/')) != cg_path) {
     *last_slash = '\0'; // strip path
     // update to shortened path and try again

src/hotspot/os/linux/cgroupV1Subsystem_linux.cpp

@@ -49,28 +49,39 @@ void CgroupV1Controller::set_subsystem_path(const char* cgroup_path) {
   _cgroup_path = os::strdup(cgroup_path);
   stringStream ss;
   if (_root != nullptr && cgroup_path != nullptr) {
+    ss.print_raw(_mount_point);
     if (strcmp(_root, "/") == 0) {
-      ss.print_raw(_mount_point);
+      // host processes and containers with cgroupns=private
       if (strcmp(cgroup_path,"/") != 0) {
         ss.print_raw(cgroup_path);
+        if (strstr((char*)cgroup_path, "../") != nullptr) {
+          log_warning(os, container)("Cgroup v1 path at [%s] is [%s], cgroup limits can be wrong.",
+            _mount_point, cgroup_path);
+        }
       }
-      _path = os::strdup(ss.base());
     } else {
-      if (strcmp(_root, cgroup_path) == 0) {
-        ss.print_raw(_mount_point);
-        _path = os::strdup(ss.base());
-      } else {
-        char *p = strstr((char*)cgroup_path, _root);
-        if (p != nullptr && p == _root) {
-          if (strlen(cgroup_path) > strlen(_root)) {
-            ss.print_raw(_mount_point);
-            const char* cg_path_sub = cgroup_path + strlen(_root);
-            ss.print_raw(cg_path_sub);
-            _path = os::strdup(ss.base());
+      // containers with cgroupns=host, default setting is _root==cgroup_path
+      if (strcmp(_root, cgroup_path) != 0) {
+        if (*cgroup_path != '\0' && strcmp(cgroup_path, "/") != 0) {
+          // When moved to a subgroup, between subgroups, the path suffix will change.
+          const char *suffix = cgroup_path;
+          while (suffix != nullptr) {
+            stringStream pp;
+            pp.print_raw(_mount_point);
+            pp.print_raw(suffix);
+            if (os::file_exists(pp.base())) {
+              ss.print_raw(suffix);
+              if (suffix != cgroup_path) {
+                log_trace(os, container)("set_subsystem_path: cgroup v1 path reduced to: %s.", suffix);
+              }
+              break;
+            }
+            suffix = strchr(suffix+1, '/');
           }
         }
       }
     }
+    _path = os::strdup(ss.base());
   }
 }
 

src/hotspot/os/linux/cgroupV2Subsystem_linux.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, 2022, Red Hat Inc.
+ * Copyright (c) 2020, 2024, Red Hat Inc.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -316,6 +316,10 @@ char* CgroupV2Controller::construct_path(char* mount_path, const char* cgroup_pa
   ss.print_raw(mount_path);
   if (strcmp(cgroup_path, "/") != 0) {
     ss.print_raw(cgroup_path);
+    if (strstr((char*)cgroup_path, "../") != nullptr) {
+      log_warning(os, container)("Cgroup v2 path at [%s] is [%s], cgroup limits can be wrong.",
+        mount_path, cgroup_path);
+    }
   }
   return os::strdup(ss.base());
 }

src/java.base/linux/classes/jdk/internal/platform/cgroupv1/CgroupV1SubsystemController.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -25,6 +25,7 @@
 
 package jdk.internal.platform.cgroupv1;
 
+import java.lang.System.Logger.Level;
 import jdk.internal.platform.CgroupSubsystem;
 import jdk.internal.platform.CgroupSubsystemController;
 
@@ -44,27 +45,28 @@ public CgroupV1SubsystemController(String root, String mountPoint) {
 
     public void setPath(String cgroupPath) {
         if (root != null && cgroupPath != null) {
+            String path = mountPoint;
             if (root.equals("/")) {
+                // host processes and containers with cgroupns=private
                 if (!cgroupPath.equals("/")) {
-                    path = mountPoint + cgroupPath;
-                }
-                else {
-                    path = mountPoint;
-                }
-            }
-            else {
-                if (root.equals(cgroupPath)) {
-                    path = mountPoint;
+                    path += cgroupPath;
+                    if (cgroupPath.indexOf("../") != -1) {
+                        System.getLogger("jdk.internal.platform").log(Level.WARNING, String.format(
+                                "Cgroup v1 path at [%s] is [%s], cgroup limits can be wrong.",
+                                mountPoint, cgroupPath));
+                    }
                 }
-                else {
-                    if (cgroupPath.startsWith(root)) {
-                        if (cgroupPath.length() > root.length()) {
-                            String cgroupSubstr = cgroupPath.substring(root.length());
-                            path = mountPoint + cgroupSubstr;
-                        }
+            } else {
+                // containers with cgroupns=host, default setting is _root==cgroup_path
+                if (!cgroupPath.equals(root)) {
+                    if (!cgroupPath.equals("/")) {
+                        // When moved to a subgroup, between subgroups, the path suffix will change.
+                        // Rely on path adjustment that determines the actual suffix.
+                        path += cgroupPath;
                     }
                 }
             }
+            this.path = path;
         }
     }
 

src/java.base/linux/classes/jdk/internal/platform/cgroupv2/CgroupV2SubsystemController.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, Red Hat Inc.
+ * Copyright (c) 2020, 2024, Red Hat Inc.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -25,6 +25,7 @@
 
 package jdk.internal.platform.cgroupv2;
 
+import java.lang.System.Logger.Level;
 import java.nio.file.Paths;
 
 import jdk.internal.platform.CgroupSubsystem;
@@ -36,6 +37,11 @@ public class CgroupV2SubsystemController implements CgroupSubsystemController {
 
     public CgroupV2SubsystemController(String mountPath, String cgroupPath) {
         this.path = Paths.get(mountPath, cgroupPath).toString();
+        if (cgroupPath.indexOf("../") != -1) {
+            System.getLogger("jdk.internal.platform").log(Level.WARNING, String.format(
+                    "Cgroup v2 path at [%s] is [%s], cgroup limits can be wrong.",
+                    mountPath, cgroupPath));
+        }
     }
 
     @Override

test/hotspot/gtest/runtime/test_cgroupSubsystem_linux.cpp

@@ -434,9 +434,23 @@ TEST(cgroupTest, set_cgroupv1_subsystem_path) {
     "/user.slice/user-1000.slice/[email protected]", // cgroup_path
     "/sys/fs/cgroup/mem"                             // expected_path
   };
-  int length = 2;
+  TestCase container_moving_cgroup = {
+    "/sys/fs/cgroup/cpu,cpuacct",                                            // mount_path
+    "/system.slice/garden.service/garden/good/2f57368b-0eda-4e52-64d8-af5c", // root_path
+    "/system.slice/garden.service/garden/bad/2f57368b-0eda-4e52-64d8-af5c",  // cgroup_path
+    "/sys/fs/cgroup/cpu,cpuacct/system.slice/garden.service/garden/bad/2f57368b-0eda-4e52-64d8-af5c" // expected_path
+  };
+  TestCase container_prefix = {
+    "/sys/fs/cgroup/memory",    // mount_path
+    "/a",                       // root_path
+    "/a/b",                     // cgroup_path
+    "/sys/fs/cgroup/memory/a/b" // expected_path
+  };
+  int length = 4;
   TestCase* testCases[] = { &host,
-                            &container_engine };
+                            &container_engine,
+                            &container_moving_cgroup,
+                            &container_prefix };
   for (int i = 0; i < length; i++) {
     CgroupV1Controller* ctrl = new CgroupV1Controller( (char*)testCases[i]->root_path,
                                                        (char*)testCases[i]->mount_path,

test/hotspot/jtreg/containers/docker/TestMemoryWithSubgroups.java

@@ -0,0 +1,129 @@
+/*
+ * Copyright (C) 2024, BELLSOFT. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+import jdk.test.lib.containers.docker.Common;
+import jdk.test.lib.containers.docker.DockerTestUtils;
+import jdk.test.lib.containers.docker.DockerRunOptions;
+import jdk.test.lib.process.OutputAnalyzer;
+import jdk.test.lib.process.ProcessTools;
+import jdk.internal.platform.Metrics;
+
+import java.util.ArrayList;
+
+/*
+ * @test
+ * @bug 8343191
+ * @requires os.family == "linux"
+ * @modules java.base/jdk.internal.platform
+ * @library /test/lib
+ * @build jdk.test.whitebox.WhiteBox CheckOperatingSystemMXBean
+ * @run driver jdk.test.lib.helpers.ClassFileInstaller -jar whitebox.jar jdk.test.whitebox.WhiteBox
+ * @run main TestMemoryWithSubgroups
+ */
+public class TestMemoryWithSubgroups {
+
+    private static final String imageName = Common.imageName("subgroup");
+
+    public static void main(String[] args) throws Exception {
+        Metrics metrics = Metrics.systemMetrics();
+        if (metrics == null) {
+            System.out.println("Cgroup not configured.");
+            return;
+        }
+        if (!DockerTestUtils.canTestDocker()) {
+            System.out.println("Unable to run docker tests.");
+            return;
+        }
+        if ("cgroupv1".equals(metrics.getProvider())) {
+
+            Common.prepareWhiteBox();
+            DockerTestUtils.buildJdkContainerImage(imageName);
+
+            try {
+                testMemoryLimitSubgroupV1("100m", "104857600", false);
+                testMemoryLimitSubgroupV1("500m", "524288000", false);
+                testMemoryLimitSubgroupV1("100m", "104857600", true);
+                testMemoryLimitSubgroupV1("500m", "524288000", true);
+            } finally {
+                DockerTestUtils.removeDockerImage(imageName);
+            }
+        } else if ("cgroupv2".equals(metrics.getProvider())) {
+
+            Common.prepareWhiteBox();
+            DockerTestUtils.buildJdkContainerImage(imageName);
+
+            try {
+                testMemoryLimitSubgroupV2("100m", "104857600", false);
+                testMemoryLimitSubgroupV2("500m", "524288000", false);
+                testMemoryLimitSubgroupV2("100m", "104857600", true);
+                testMemoryLimitSubgroupV2("500m", "524288000", true);
+            } finally {
+                DockerTestUtils.removeDockerImage(imageName);
+            }
+        } else {
+            System.out.println("Metrics are from neither cgroup v1 nor v2, skipped for now.");
+        }
+    }
+
+    private static void testMemoryLimitSubgroupV1(String valueToSet, String expectedValue, boolean privateNamespace)
+            throws Exception {
+
+        Common.logNewTestCase("Cgroup V1 subgroup memory limit: " + valueToSet);
+
+        DockerRunOptions opts = new DockerRunOptions(imageName, "sh", "-c");
+        opts.javaOpts = new ArrayList<>();
+        opts.appendTestJavaOptions = false;
+        opts.addDockerOpts("--privileged")
+            .addDockerOpts("--cgroupns=" + (privateNamespace ? "private" : "host"))
+            .addDockerOpts("--memory", "1g");
+        opts.addClassOptions("mkdir -p /sys/fs/cgroup/memory/test ; " +
+            "echo " + valueToSet + " > /sys/fs/cgroup/memory/test/memory.limit_in_bytes ; " +
+            "echo $$ > /sys/fs/cgroup/memory/test/cgroup.procs ; " +
+            "/jdk/bin/java -Xlog:os+container=trace -version");
+
+        Common.run(opts)
+            .shouldMatch("Lowest limit was:.*" + expectedValue);
+    }
+
+    private static void testMemoryLimitSubgroupV2(String valueToSet, String expectedValue, boolean privateNamespace)
+            throws Exception {
+
+        Common.logNewTestCase("Cgroup V2 subgroup memory limit: " + valueToSet);
+
+        DockerRunOptions opts = new DockerRunOptions(imageName, "sh", "-c");
+        opts.javaOpts = new ArrayList<>();
+        opts.appendTestJavaOptions = false;
+        opts.addDockerOpts("--privileged")
+            .addDockerOpts("--cgroupns=" + (privateNamespace ? "private" : "host"))
+            .addDockerOpts("--memory", "1g");
+        opts.addClassOptions("mkdir -p /sys/fs/cgroup/memory/test ; " +
+            "echo $$ > /sys/fs/cgroup/memory/test/cgroup.procs ; " +
+            "echo '+memory' > /sys/fs/cgroup/cgroup.subtree_control ; " +
+            "echo '+memory' > /sys/fs/cgroup/memory/cgroup.subtree_control ; " +
+            "echo " + valueToSet + " > /sys/fs/cgroup/memory/test/memory.max ; " +
+            "/jdk/bin/java -Xlog:os+container=trace -version");
+
+        Common.run(opts)
+            .shouldMatch("Lowest limit was:.*" + expectedValue);
+    }
+}

test/jdk/jdk/internal/platform/cgroup/CgroupV1SubsystemControllerTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, Red Hat, Inc.
+ * Copyright (c) 2022, 2024, Red Hat, Inc.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -64,15 +64,39 @@ public void testCgPathNonEmptyRoot() {
         assertEquals(expectedPath, ctrl.path());
     }
 
+    /*
+     * Less common cases: Containers
+     */
     @Test
     public void testCgPathSubstring() {
         String root = "/foo/bar/baz";
         String mountPoint = "/sys/fs/cgroup/memory";
         CgroupV1SubsystemController ctrl = new CgroupV1SubsystemController(root, mountPoint);
         String cgroupPath = "/foo/bar/baz/some";
         ctrl.setPath(cgroupPath);
-        String expectedPath = mountPoint + "/some";
+        String expectedPath = mountPoint + cgroupPath;
         assertEquals(expectedPath, ctrl.path());
     }
 
+    @Test
+    public void testCgPathPrefixRoot() {
+        String root = "/a";
+        String mountPoint = "/sys/fs/cgroup/memory";
+        CgroupV1SubsystemController ctrl = new CgroupV1SubsystemController(root, mountPoint);
+        String cgroupPath = "/a/b";
+        ctrl.setPath(cgroupPath);
+        String expectedPath = mountPoint + cgroupPath;
+        assertEquals(expectedPath, ctrl.path());
+    }
+
+    @Test
+    public void testCgPathFallbackToMountPoint() {
+        String root = "/system.slice/garden.service/garden/good/2f57368b-0eda-4e52-64d8-af5c";
+        String mountPoint = "/sys/fs/cgroup/cpu,cpuacct";
+        CgroupV1SubsystemController ctrl = new CgroupV1SubsystemController(root, mountPoint);
+        String cgroupPath = "/system.slice/garden.service/garden/bad/2f57368b-0eda-4e52-64d8-af5c";
+        ctrl.setPath(cgroupPath);
+        String expectedPath = mountPoint + cgroupPath;
+        assertEquals(expectedPath, ctrl.path());
+    }
 }