8343191: Cgroup v1 subsystem fails to set subsystem path

src/hotspot/os/linux/cgroupSubsystem_linux.cpp

@@ -103,23 +103,27 @@ CgroupSubsystem* CgroupSubsystemFactory::create() {
    *
    */
   assert(is_cgroup_v1(&cg_type_flags), "Cgroup v1 expected");
+  bool all_controllers_read_only = true;
+  for (int n = 0; n < CG_INFO_LENGTH && all_controllers_read_only; n++) {
+    all_controllers_read_only &= cg_infos[n]._read_only;
+  }
   for (int i = 0; i < CG_INFO_LENGTH; i++) {
     CgroupInfo info = cg_infos[i];
     if (info._data_complete) { // pids controller might have incomplete data
       if (strcmp(info._name, "memory") == 0) {
-        memory = new CgroupV1MemoryController(CgroupV1Controller(info._root_mount_path, info._mount_path, info._read_only));
+        memory = new CgroupV1MemoryController(CgroupV1Controller(info._root_mount_path, info._mount_path, all_controllers_read_only));
         memory->set_subsystem_path(info._cgroup_path);
       } else if (strcmp(info._name, "cpuset") == 0) {
-        cpuset = new CgroupV1Controller(info._root_mount_path, info._mount_path, info._read_only);
+        cpuset = new CgroupV1Controller(info._root_mount_path, info._mount_path, all_controllers_read_only);
         cpuset->set_subsystem_path(info._cgroup_path);
       } else if (strcmp(info._name, "cpu") == 0) {
-        cpu = new CgroupV1CpuController(CgroupV1Controller(info._root_mount_path, info._mount_path, info._read_only));
+        cpu = new CgroupV1CpuController(CgroupV1Controller(info._root_mount_path, info._mount_path, all_controllers_read_only));
         cpu->set_subsystem_path(info._cgroup_path);
       } else if (strcmp(info._name, "cpuacct") == 0) {
-        cpuacct = new CgroupV1Controller(info._root_mount_path, info._mount_path, info._read_only);
+        cpuacct = new CgroupV1Controller(info._root_mount_path, info._mount_path, all_controllers_read_only);
         cpuacct->set_subsystem_path(info._cgroup_path);
       } else if (strcmp(info._name, "pids") == 0) {
-        pids = new CgroupV1Controller(info._root_mount_path, info._mount_path, info._read_only);
+        pids = new CgroupV1Controller(info._root_mount_path, info._mount_path, all_controllers_read_only);
         pids->set_subsystem_path(info._cgroup_path);
       }
     } else {

src/hotspot/os/linux/cgroupV1Subsystem_linux.cpp

@@ -49,28 +49,19 @@ void CgroupV1Controller::set_subsystem_path(const char* cgroup_path) {
   _cgroup_path = os::strdup(cgroup_path);
   stringStream ss;
   if (_root != nullptr && cgroup_path != nullptr) {
-    if (strcmp(_root, "/") == 0) {
-      ss.print_raw(_mount_point);
+    ss.print_raw(_mount_point);
+    if (strcmp(_root, "/") == 0 || !is_read_only()) {
+      // host processes / containers w/private cgroup namespace
       if (strcmp(cgroup_path,"/") != 0) {
+        // hosts only
         ss.print_raw(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());
-          }
-        }
-      }
+    } else if (strcmp(_root, cgroup_path) != 0) {
+      // containers only, warn if doesn't match
+      log_warning(os, container)("Cgroup v1 controller (%s) mounting root [%s] doesn't match cgroup [%s]",
+        _mount_point, _root, cgroup_path);
     }
+    _path = os::strdup(ss.base());
   }
 }
 
@@ -218,11 +209,9 @@ bool CgroupV1Subsystem::is_containerized() {
   // containerized iff all required controllers are mounted
   // read-only. See OSContainer::is_containerized() for
   // the full logic.
+  // (all v1 controllers are initialized with a single combined read-only flag)
   //
-  return _memory->controller()->is_read_only() &&
-         _cpu->controller()->is_read_only() &&
-         _cpuacct->is_read_only() &&
-         _cpuset->is_read_only();
+  return _memory->controller()->is_read_only();
 }
 
 /* memory_usage_in_bytes

src/java.base/linux/classes/jdk/internal/platform/CgroupInfo.java

@@ -40,6 +40,7 @@ public class CgroupInfo {
     private final boolean enabled;
     private String mountPoint;
     private String mountRoot;
+    private String mountOptions;
     private String cgroupPath;
 
     private CgroupInfo(String name, int hierarchyId, boolean enabled) {
@@ -76,6 +77,14 @@ public void setMountRoot(String mountRoot) {
         this.mountRoot = mountRoot;
     }
 
+    public String getMountOptions() {
+        return mountOptions;
+    }
+
+    public void setMountOptions(String mountOptions) {
+        this.mountOptions = mountOptions;
+    }
+
     public String getCgroupPath() {
         return cgroupPath;
     }

src/java.base/linux/classes/jdk/internal/platform/CgroupSubsystemFactory.java

@@ -75,8 +75,9 @@ public class CgroupSubsystemFactory {
     private static final Pattern MOUNTINFO_PATTERN = Pattern.compile(
         "^[^\\s]+\\s+[^\\s]+\\s+[^\\s]+\\s+" + // (1), (2), (3)
         "([^\\s]+)\\s+([^\\s]+)\\s+" +         // (4), (5)     - group 1, 2: root, mount point
-        "[^-]+-\\s+" +                         // (6), (7), (8)
-        "([^\\s]+)\\s+" +                      // (9)          - group 3: filesystem type
+        "([^\\s]+)\\s+" +                      // (6)          - group 3: mount options
+        "[^-]*-\\s+" +                         // (7), (8)
+        "([^\\s]+)\\s+" +                      // (9)          - group 4: filesystem type
         ".*$");                                // (10), (11)
 
     static CgroupMetrics create() {
@@ -309,7 +310,8 @@ private static boolean amendCgroupInfos(String mntInfoLine,
         if (lineMatcher.matches()) {
             String mountRoot = lineMatcher.group(1);
             String mountPath = lineMatcher.group(2);
-            String fsType = lineMatcher.group(3);
+            String mountOptions = lineMatcher.group(3);
+            String fsType = lineMatcher.group(4);
             if (fsType.equals("cgroup")) {
                 Path p = Paths.get(mountPath);
                 String[] controllerNames = p.getFileName().toString().split(",");
@@ -322,7 +324,7 @@ private static boolean amendCgroupInfos(String mntInfoLine,
                         case PIDS_CTRL:
                         case BLKIO_CTRL: {
                             CgroupInfo info = infos.get(controllerName);
-                            setMountPoints(info, mountPath, mountRoot);
+                            setMountPoints(info, mountPath, mountRoot, mountOptions);
                             cgroupv1ControllerFound = true;
                             break;
                         }
@@ -336,7 +338,7 @@ private static boolean amendCgroupInfos(String mntInfoLine,
                     // All controllers have the same mount point and root mount
                     // for unified hierarchy.
                     for (CgroupInfo info: infos.values()) {
-                        setMountPoints(info, mountPath, mountRoot);
+                        setMountPoints(info, mountPath, mountRoot, mountOptions);
                     }
                 }
                 cgroupv2ControllerFound = true;
@@ -345,7 +347,7 @@ private static boolean amendCgroupInfos(String mntInfoLine,
         return cgroupv1ControllerFound || cgroupv2ControllerFound;
     }
 
-    private static void setMountPoints(CgroupInfo info, String mountPath, String mountRoot) {
+    private static void setMountPoints(CgroupInfo info, String mountPath, String mountRoot, String mountOptions) {
         if (info.getMountPoint() != null) {
             // On some systems duplicate controllers get mounted in addition to
             // the main cgroup controllers (which are under /sys/fs/cgroup). In that
@@ -359,6 +361,7 @@ private static void setMountPoints(CgroupInfo info, String mountPath, String mou
             info.setMountPoint(mountPath);
             info.setMountRoot(mountRoot);
         }
+        info.setMountOptions(mountOptions);
     }
 
     public static final class CgroupTypeResult {

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

@@ -30,8 +30,8 @@ public class CgroupV1MemorySubSystemController extends CgroupV1SubsystemControll
     private boolean hierarchical;
     private boolean swapenabled;
 
-    public CgroupV1MemorySubSystemController(String root, String mountPoint) {
-        super(root, mountPoint);
+    public CgroupV1MemorySubSystemController(String root, String mountPoint, boolean isContainerized) {
+        super(root, mountPoint, isContainerized);
     }
 
     boolean isHierarchical() {

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

@@ -72,6 +72,24 @@ private static CgroupV1Subsystem initSubSystem(Map<String, CgroupInfo> infos) {
         CgroupV1Subsystem subsystem = new CgroupV1Subsystem();
 
         boolean anyActiveControllers = false;
+        boolean allControllersReadOnly = true;
+
+        for (CgroupInfo info: infos.values()) {
+            String mo = info.getMountOptions();
+            boolean readOnly = false;
+            if (mo != null) {
+                for (String o : mo.split(",")) {
+                    if (o.equals("ro")) {
+                        readOnly = true;
+                        break;
+                    }
+                }
+            }
+            if (!readOnly) {
+                allControllersReadOnly = false;
+                break;
+            }
+        }
         /*
          * Find the cgroup mount points for subsystem controllers
          * by looking up relevant data in the infos map
@@ -80,7 +98,7 @@ private static CgroupV1Subsystem initSubSystem(Map<String, CgroupInfo> infos) {
             switch (info.getName()) {
             case "memory": {
                 if (info.getMountRoot() != null && info.getMountPoint() != null) {
-                    CgroupV1MemorySubSystemController controller = new CgroupV1MemorySubSystemController(info.getMountRoot(), info.getMountPoint());
+                    CgroupV1MemorySubSystemController controller = new CgroupV1MemorySubSystemController(info.getMountRoot(), info.getMountPoint(), allControllersReadOnly);
                     controller.setPath(info.getCgroupPath());
                     boolean isHierarchial = getHierarchical(controller);
                     controller.setHierarchical(isHierarchial);
@@ -93,7 +111,7 @@ private static CgroupV1Subsystem initSubSystem(Map<String, CgroupInfo> infos) {
             }
             case "cpuset": {
                 if (info.getMountRoot() != null && info.getMountPoint() != null) {
-                    CgroupV1SubsystemController controller = new CgroupV1SubsystemController(info.getMountRoot(), info.getMountPoint());
+                    CgroupV1SubsystemController controller = new CgroupV1SubsystemController(info.getMountRoot(), info.getMountPoint(), allControllersReadOnly);
                     controller.setPath(info.getCgroupPath());
                     subsystem.setCpuSetController(controller);
                     anyActiveControllers = true;
@@ -102,7 +120,7 @@ private static CgroupV1Subsystem initSubSystem(Map<String, CgroupInfo> infos) {
             }
             case "cpuacct": {
                 if (info.getMountRoot() != null && info.getMountPoint() != null) {
-                    CgroupV1SubsystemController controller = new CgroupV1SubsystemController(info.getMountRoot(), info.getMountPoint());
+                    CgroupV1SubsystemController controller = new CgroupV1SubsystemController(info.getMountRoot(), info.getMountPoint(), allControllersReadOnly);
                     controller.setPath(info.getCgroupPath());
                     subsystem.setCpuAcctController(controller);
                     anyActiveControllers = true;
@@ -111,7 +129,7 @@ private static CgroupV1Subsystem initSubSystem(Map<String, CgroupInfo> infos) {
             }
             case "cpu": {
                 if (info.getMountRoot() != null && info.getMountPoint() != null) {
-                    CgroupV1SubsystemController controller = new CgroupV1SubsystemController(info.getMountRoot(), info.getMountPoint());
+                    CgroupV1SubsystemController controller = new CgroupV1SubsystemController(info.getMountRoot(), info.getMountPoint(), allControllersReadOnly);
                     controller.setPath(info.getCgroupPath());
                     subsystem.setCpuController(controller);
                     anyActiveControllers = true;
@@ -120,7 +138,7 @@ private static CgroupV1Subsystem initSubSystem(Map<String, CgroupInfo> infos) {
             }
             case "blkio": {
                 if (info.getMountRoot() != null && info.getMountPoint() != null) {
-                    CgroupV1SubsystemController controller = new CgroupV1SubsystemController(info.getMountRoot(), info.getMountPoint());
+                    CgroupV1SubsystemController controller = new CgroupV1SubsystemController(info.getMountRoot(), info.getMountPoint(), allControllersReadOnly);
                     controller.setPath(info.getCgroupPath());
                     subsystem.setBlkIOController(controller);
                     anyActiveControllers = true;
@@ -129,7 +147,7 @@ private static CgroupV1Subsystem initSubSystem(Map<String, CgroupInfo> infos) {
             }
             case "pids": {
                 if (info.getMountRoot() != null && info.getMountPoint() != null) {
-                    CgroupV1SubsystemController controller = new CgroupV1SubsystemController(info.getMountRoot(), info.getMountPoint());
+                    CgroupV1SubsystemController controller = new CgroupV1SubsystemController(info.getMountRoot(), info.getMountPoint(), allControllersReadOnly);
                     controller.setPath(info.getCgroupPath());
                     subsystem.setPidsController(controller);
                     anyActiveControllers = true;

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

@@ -25,6 +25,8 @@
 
 package jdk.internal.platform.cgroupv1;
 
+import java.lang.System.Logger.Level;
+
 import jdk.internal.platform.CgroupSubsystem;
 import jdk.internal.platform.CgroupSubsystemController;
 
@@ -36,35 +38,30 @@ public class CgroupV1SubsystemController implements CgroupSubsystemController {
     String root;
     String mountPoint;
     String path;
+    boolean isContainerized = false;
 
-    public CgroupV1SubsystemController(String root, String mountPoint) {
+    public CgroupV1SubsystemController(String root, String mountPoint, boolean isContainerized) {
         this.root = root;
         this.mountPoint = mountPoint;
+        this.isContainerized = isContainerized;
     }
 
     public void setPath(String cgroupPath) {
         if (root != null && cgroupPath != null) {
-            if (root.equals("/")) {
+            String path = mountPoint;
+            if (root.equals("/") || !isContainerized) {
+                // host processes / containers w/private cgroup namespace
                 if (!cgroupPath.equals("/")) {
-                    path = mountPoint + cgroupPath;
-                }
-                else {
-                    path = mountPoint;
-                }
-            }
-            else {
-                if (root.equals(cgroupPath)) {
-                    path = mountPoint;
-                }
-                else {
-                    if (cgroupPath.startsWith(root)) {
-                        if (cgroupPath.length() > root.length()) {
-                            String cgroupSubstr = cgroupPath.substring(root.length());
-                            path = mountPoint + cgroupSubstr;
-                        }
-                    }
+                    // hosts only
+                    path += cgroupPath;
                 }
+            } else if (!root.equals(cgroupPath)) {
+                // containers only, warn if doesn't match
+                System.getLogger("jdk.internal.platform").log(Level.WARNING, String.format(
+                        "Cgroup v1 controller (%s) mounting root [%s] doesn't match cgroup [%s].",
+                        mountPoint, root, cgroupPath));
             }
+            this.path = path;
         }
     }