RFR: 8349988: Change cgroup version detection logic to not depend on /proc/cgroups [v4]

Thomas Fitzsimmons duke at openjdk.org
Tue Apr 1 11:38:19 UTC 2025


On Tue, 1 Apr 2025 08:41:53 GMT, Severin Gehwolf <sgehwolf at openjdk.org> wrote:

>> One option is to pass an argument to `determine_type` to indicate it is being called from the test suite and skip the call to `statfs` in such case.
>
> If we really must, I'd rather have a function pointer for the statfs call which we can replace in test code. It doesn't seem worth the extra complexity in my opinion though.

This is what I had so far (not yet fully tested), but it adds a null check to the non-testing code path...; I can try an approach with a function pointer too if necessary; let me know how to proceed:


diff --git a/src/hotspot/os/linux/cgroupSubsystem_linux.cpp b/src/hotspot/os/linux/cgroupSubsystem_linux.cpp
index 612cb9a9302..6479853ac04 100644
--- a/src/hotspot/os/linux/cgroupSubsystem_linux.cpp
+++ b/src/hotspot/os/linux/cgroupSubsystem_linux.cpp
@@ -66,26 +66,10 @@ CgroupSubsystem* CgroupSubsystemFactory::create() {
   CgroupV1Controller* pids = nullptr;
   CgroupInfo cg_infos[CG_INFO_LENGTH];
   u1 cg_type_flags = INVALID_CGROUPS_GENERIC;
-  const char* proc_cgroups = "/proc/cgroups";
-  const char* sys_fs_cgroup_cgroup_controllers = "/sys/fs/cgroup/cgroup.controllers";
-  const char* controllers_file = proc_cgroups;
   const char* proc_self_cgroup = "/proc/self/cgroup";
   const char* proc_self_mountinfo = "/proc/self/mountinfo";
-  const char* sys_fs_cgroup = "/sys/fs/cgroup";
-  struct statfs fsstat = {};
-  bool cgroups_v2_enabled = false;
 
-  // Assume cgroups v2 is usable by the JDK iff /sys/fs/cgroup has the cgroup v2
-  // file system magic.  If it does not then heuristics are required to determine
-  // if cgroups v1 is usable or not.
-  if (statfs(sys_fs_cgroup, &fsstat) != -1) {
-    cgroups_v2_enabled = (fsstat.f_type == CGROUP2_SUPER_MAGIC);
-    if (cgroups_v2_enabled) {
-      controllers_file = sys_fs_cgroup_cgroup_controllers;
-    }
-  }
-
-  bool valid_cgroup = determine_type(cg_infos, cgroups_v2_enabled, controllers_file, proc_self_cgroup, proc_self_mountinfo, &cg_type_flags);
+  bool valid_cgroup = determine_type(cg_infos, true, NULL, proc_self_cgroup, proc_self_mountinfo, &cg_type_flags);
 
   if (!valid_cgroup) {
     // Could not detect cgroup type
@@ -249,9 +233,16 @@ static inline bool match_mount_info_line(char* line,
                tmpcgroups) == 5;
 }
 
+/*
+ * If controllers_file_mock is non-NULL use it as the controllers file
+ * and respect cgroups_v2_enabled_mock.  This is used by WhiteBox to
+ * mock the statfs call.  If controllers_file_mock is NULL, ignore
+ * cgroups_v2_enabled_mock and determine using statfs what to use as
+ * the controllers file.
+ */
 bool CgroupSubsystemFactory::determine_type(CgroupInfo* cg_infos,
-                                            bool cgroups_v2_enabled,
-                                            const char* controllers_file,
+                                            bool cgroups_v2_enabled_mock,
+                                            const char* controllers_file_mock,
                                             const char* proc_self_cgroup,
                                             const char* proc_self_mountinfo,
                                             u1* flags) {
@@ -265,6 +256,28 @@ bool CgroupSubsystemFactory::determine_type(CgroupInfo* cg_infos,
   // pids might not be enabled on older Linux distros (SLES 12.1, RHEL 7.1)
   // cpuset might not be enabled on newer Linux distros (Fedora 41)
   bool all_required_controllers_enabled = true;
+  bool cgroups_v2_enabled = false;
+  const char* controllers_file = controllers_file_mock;
+
+  if (controllers_file) {
+    cgroups_v2_enabled = cgroups_v2_enabled_mock;
+  } else {
+    const char* proc_cgroups = "/proc/cgroups";
+    const char* sys_fs_cgroup_cgroup_controllers = "/sys/fs/cgroup/cgroup.controllers";
+    const char* sys_fs_cgroup = "/sys/fs/cgroup";
+    struct statfs fsstat = {};
+
+    controllers_file = proc_cgroups;
+    // Assume cgroups v2 is usable by the JDK iff /sys/fs/cgroup has the cgroup v2
+    // file system magic.  If it does not then heuristics are required to determine
+    // if cgroups v1 is usable or not.
+    if (statfs(sys_fs_cgroup, &fsstat) != -1) {
+      cgroups_v2_enabled = (fsstat.f_type == CGROUP2_SUPER_MAGIC);
+      if (cgroups_v2_enabled) {
+        controllers_file = sys_fs_cgroup_cgroup_controllers;
+      }
+    }
+  }
 
   // If cgroups v2 is enabled, open /sys/fs/cgroup/cgroup.controllers.  If not, open /proc/cgroups.
   controllers = os::fopen(controllers_file, "r");

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23811#discussion_r2022679495


More information about the hotspot-dev mailing list