8293540: [Metrics] Incorrectly detected resource limits with additional cgroup fs mounts

Reviewed-by: iklam

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

@@ -318,30 +318,11 @@ public class CgroupSubsystemFactory {
                         case MEMORY_CTRL: // fall-through
                         case CPU_CTRL:
                         case CPUACCT_CTRL:
+                        case CPUSET_CTRL:
                         case PIDS_CTRL:
                         case BLKIO_CTRL: {
                             CgroupInfo info = infos.get(controllerName);
-                            assert info.getMountPoint() == null;
+                            setMountPoints(info, mountPath, mountRoot);
-                            assert info.getMountRoot() == null;
-                            info.setMountPoint(mountPath);
-                            info.setMountRoot(mountRoot);
-                            cgroupv1ControllerFound = true;
-                            break;
-                        }
-                        case CPUSET_CTRL: {
-                            CgroupInfo info = infos.get(controllerName);
-                            if (info.getMountPoint() != null) {
-                                // On some systems duplicate cpuset controllers get mounted in addition to
-                                // the main cgroup controllers most likely under /sys/fs/cgroup. In that
-                                // case pick the one under /sys/fs/cgroup and discard others.
-                                if (!info.getMountPoint().startsWith("/sys/fs/cgroup")) {
-                                    info.setMountPoint(mountPath);
-                                    info.setMountRoot(mountRoot);
-                                }
-                            } else {
-                                info.setMountPoint(mountPath);
-                                info.setMountRoot(mountRoot);
-                            }
                             cgroupv1ControllerFound = true;
                             break;
                         }
@@ -355,10 +336,7 @@ public class CgroupSubsystemFactory {
                     // All controllers have the same mount point and root mount
                     // for unified hierarchy.
                     for (CgroupInfo info: infos.values()) {
-                        assert info.getMountPoint() == null;
+                        setMountPoints(info, mountPath, mountRoot);
-                        assert info.getMountRoot() == null;
-                        info.setMountPoint(mountPath);
-                        info.setMountRoot(mountRoot);
                     }
                 }
                 cgroupv2ControllerFound = true;
@@ -367,6 +345,22 @@ public class CgroupSubsystemFactory {
         return cgroupv1ControllerFound || cgroupv2ControllerFound;
     }
 
+    private static void setMountPoints(CgroupInfo info, String mountPath, String mountRoot) {
+        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
+            // case pick the main one and discard others as the limits
+            // are associated with the ones in /sys/fs/cgroup.
+            if (!info.getMountPoint().startsWith("/sys/fs/cgroup")) {
+                info.setMountPoint(mountPath);
+                info.setMountRoot(mountRoot);
+            }
+        } else {
+            info.setMountPoint(mountPath);
+            info.setMountRoot(mountRoot);
+        }
+    }
+
     public static final class CgroupTypeResult {
         private final boolean isCgroupV2;
         private final boolean anyControllersEnabled;

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

@@ -87,6 +87,11 @@ public class TestMemoryAwareness {
                 "1G", Integer.toString(((int) Math.pow(2, 20)) * 1024),
                 "1500M", Integer.toString(((int) Math.pow(2, 20)) * (1500 - 1024))
             );
+            testOperatingSystemMXBeanAwareness(
+                "100M", Integer.toString(((int) Math.pow(2, 20)) * 100),
+                "200M", Integer.toString(((int) Math.pow(2, 20)) * (200 - 100)),
+                true /* additional cgroup fs mounts */
+            );
             final String hostMaxMem = getHostMaxMemory();
             testOperatingSystemMXBeanIgnoresMemLimitExceedingPhysicalMemory(hostMaxMem);
             testMetricsIgnoresMemLimitExceedingPhysicalMemory(hostMaxMem);
@@ -170,6 +175,12 @@ public class TestMemoryAwareness {
 
     private static void testOperatingSystemMXBeanAwareness(String memoryAllocation, String expectedMemory,
             String swapAllocation, String expectedSwap) throws Exception {
+        testOperatingSystemMXBeanAwareness(memoryAllocation, expectedMemory, swapAllocation, expectedSwap, false);
+    }
+
+    private static void testOperatingSystemMXBeanAwareness(String memoryAllocation, String expectedMemory,
+            String swapAllocation, String expectedSwap, boolean addCgroupMounts) throws Exception {
+
         Common.logNewTestCase("Check OperatingSystemMXBean");
 
         DockerRunOptions opts = Common.newOpts(imageName, "CheckOperatingSystemMXBean")
@@ -181,6 +192,10 @@ public class TestMemoryAwareness {
             // diagnostics
             .addJavaOpts("--add-exports")
             .addJavaOpts("java.base/jdk.internal.platform=ALL-UNNAMED");
+        if (addCgroupMounts) {
+            // Extra cgroup mount should be ignored by product code
+            opts.addDockerOpts("--volume", "/sys/fs/cgroup:/cgroup-in:ro");
+        }
 
         OutputAnalyzer out = DockerTestUtils.dockerRunJava(opts);
         out.shouldHaveExitValue(0)

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

@@ -51,7 +51,7 @@ import jdk.test.lib.util.FileUtils;
 
 /*
  * @test
- * @bug 8287107 8287073
+ * @bug 8287107 8287073 8293540
  * @key cgroups
  * @requires os.family == "linux"
  * @modules java.base/jdk.internal.platform
@@ -72,8 +72,8 @@ public class TestCgroupSubsystemFactory {
     private Path cgroupv1CgInfoNonZeroHierarchy;
     private Path cgroupv1MntInfoNonZeroHierarchy;
     private Path cgroupv1MntInfoSystemdOnly;
-    private Path cgroupv1MntInfoDoubleCpusets;
+    private Path cgroupv1MntInfoDoubleControllers;
-    private Path cgroupv1MntInfoDoubleCpusets2;
+    private Path cgroupv1MntInfoDoubleControllers2;
     private Path cgroupv1MntInfoColonsHierarchy;
     private Path cgroupv1SelfCgroup;
     private Path cgroupv1SelfColons;
@@ -194,9 +194,13 @@ public class TestCgroupSubsystemFactory {
     private String mntInfoCgroupsV1SystemdOnly =
             "35 26 0:26 / /sys/fs/cgroup/systemd rw,nosuid,nodev,noexec,relatime - cgroup systemd rw,name=systemd\n" +
             "26 18 0:19 / /sys/fs/cgroup rw,relatime - tmpfs none rw,size=4k,mode=755\n";
-    private String mntInfoCgroupv1MoreCpusetLine = "121 32 0:37 / /cpuset rw,relatime shared:69 - cgroup none rw,cpuset\n";
+    private String mntInfoCgroupv1MoreControllers = "121 32 0:37 / /cpuset rw,relatime shared:69 - cgroup none rw,cpuset\n" +
-    private String mntInfoCgroupsV1DoubleCpuset = mntInfoHybrid + mntInfoCgroupv1MoreCpusetLine;
+            "35 30 0:31 / /cgroup-in/memory rw,nosuid,nodev,noexec,relatime shared:7 - cgroup none rw,seclabel,memory\n" +
-    private String mntInfoCgroupsV1DoubleCpuset2 = mntInfoCgroupv1MoreCpusetLine + mntInfoHybrid;
+            "36 30 0:32 / /cgroup-in/pids rw,nosuid,nodev,noexec,relatime shared:8 - cgroup none rw,seclabel,pids\n" +
+            "40 30 0:36 / /cgroup-in/cpu,cpuacct rw,nosuid,nodev,noexec,relatime shared:12 - cgroup none rw,seclabel,cpu,cpuacct\n" +
+            "40 30 0:36 / /cgroup-in/blkio rw,nosuid,nodev,noexec,relatime shared:12 - cgroup none rw,seclabel,blkio\n";
+    private String mntInfoCgroupsV1DoubleControllers = mntInfoHybrid + mntInfoCgroupv1MoreControllers;
+    private String mntInfoCgroupsV1DoubleControllers2 = mntInfoCgroupv1MoreControllers + mntInfoHybrid;
     private String cgroupv1SelfCgroupContent = "11:memory:/user.slice/user-1000.slice/user@1000.service\n" +
             "10:hugetlb:/\n" +
             "9:cpuset:/\n" +
@@ -275,11 +279,11 @@ public class TestCgroupSubsystemFactory {
             cgroupv1MntInfoSystemdOnly = Paths.get(existingDirectory.toString(), "mountinfo_cgroupv1_systemd_only");
             Files.writeString(cgroupv1MntInfoSystemdOnly, mntInfoCgroupsV1SystemdOnly);
 
-            cgroupv1MntInfoDoubleCpusets = Paths.get(existingDirectory.toString(), "mountinfo_cgroupv1_double_cpuset");
+            cgroupv1MntInfoDoubleControllers = Paths.get(existingDirectory.toString(), "mountinfo_cgroupv1_double_controllers");
-            Files.writeString(cgroupv1MntInfoDoubleCpusets, mntInfoCgroupsV1DoubleCpuset);
+            Files.writeString(cgroupv1MntInfoDoubleControllers, mntInfoCgroupsV1DoubleControllers);
 
-            cgroupv1MntInfoDoubleCpusets2 = Paths.get(existingDirectory.toString(), "mountinfo_cgroupv1_double_cpuset2");
+            cgroupv1MntInfoDoubleControllers2 = Paths.get(existingDirectory.toString(), "mountinfo_cgroupv1_double_controllers2");
-            Files.writeString(cgroupv1MntInfoDoubleCpusets2, mntInfoCgroupsV1DoubleCpuset2);
+            Files.writeString(cgroupv1MntInfoDoubleControllers2, mntInfoCgroupsV1DoubleControllers2);
 
             cgroupv1CgroupsJoinControllers = Paths.get(existingDirectory.toString(), "cgroups_cgv1_join_controllers");
             Files.writeString(cgroupv1CgroupsJoinControllers, cgroupsNonZeroJoinControllers);
@@ -390,11 +394,11 @@ public class TestCgroupSubsystemFactory {
 
     @Test
     public void testCgroupv1MultipleCpusetMounts() throws IOException {
-        doMultipleCpusetMountsTest(cgroupv1MntInfoDoubleCpusets);
+        doMultipleMountsTest(cgroupv1MntInfoDoubleControllers);
-        doMultipleCpusetMountsTest(cgroupv1MntInfoDoubleCpusets2);
+        doMultipleMountsTest(cgroupv1MntInfoDoubleControllers2);
     }
 
-    private void doMultipleCpusetMountsTest(Path info) throws IOException {
+    private void doMultipleMountsTest(Path info) throws IOException {
         String cgroups = cgroupv1CgInfoNonZeroHierarchy.toString();
         String mountInfo = info.toString();
         String selfCgroup = cgroupv1SelfCgroup.toString();
@@ -406,6 +410,13 @@ public class TestCgroupSubsystemFactory {
         CgroupInfo cpuSetInfo = res.getInfos().get("cpuset");
         assertEquals("/sys/fs/cgroup/cpuset", cpuSetInfo.getMountPoint());
         assertEquals("/", cpuSetInfo.getMountRoot());
+        // Ensure controllers at /sys/fs/cgroup will be used
+        String[] ctrlNames = new String[] { "memory", "cpu", "cpuacct", "blkio", "pids" };
+        for (int i = 0; i < ctrlNames.length; i++) {
+            CgroupInfo cinfo = res.getInfos().get(ctrlNames[i]);
+            assertTrue(cinfo.getMountPoint().startsWith("/sys/fs/cgroup/"));
+            assertEquals("/", cinfo.getMountRoot());
+        }
     }
 
     @Test

test/jdk/jdk/internal/platform/docker/TestDockerBasic.java

@@ -0,0 +1,72 @@
+/*
+ * Copyright (c) 2022, 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
+ * 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.
+ */
+
+/*
+ * @test
+ * @bug 8293540
+ * @summary Verify that -XshowSettings:system works
+ * @requires docker.support
+ * @library /test/lib
+ * @run main/timeout=360 TestDockerBasic
+ */
+
+import jdk.test.lib.Utils;
+import jdk.test.lib.containers.docker.Common;
+import jdk.test.lib.containers.docker.DockerRunOptions;
+import jdk.test.lib.containers.docker.DockerTestUtils;
+
+public class TestDockerBasic {
+    private static final String imageName = Common.imageName("javaDockerBasic");
+
+    public static void main(String[] args) throws Exception {
+        if (!DockerTestUtils.canTestDocker()) {
+            return;
+        }
+
+        DockerTestUtils.buildJdkContainerImage(imageName);
+
+        try {
+            testXshowSettingsSystem(true);
+            testXshowSettingsSystem(false);
+        } finally {
+            DockerTestUtils.removeDockerImage(imageName);
+        }
+    }
+
+    private static void testXshowSettingsSystem(boolean addCgroupMounts) throws Exception {
+        String testMsg = (addCgroupMounts ? " with " : " without ") + " additional cgroup FS mounts in /cgroup-in";
+        Common.logNewTestCase("Test TestDockerBasic " + testMsg);
+        DockerRunOptions opts =
+                new DockerRunOptions(imageName, "/jdk/bin/java", "-version");
+        opts.addJavaOpts("-esa");
+        opts.addJavaOpts("-XshowSettings:system");
+        opts.addDockerOpts("--memory", "300m");
+        if (addCgroupMounts) {
+            // Extra cgroup mount should be ignored by product code
+            opts.addDockerOpts("--volume", "/sys/fs/cgroup:/cgroup-in:ro");
+        }
+        DockerTestUtils.dockerRunJava(opts).shouldHaveExitValue(0)
+            .shouldNotContain("AssertionError")
+            .shouldContain("Memory Limit: 300.00M");
+    }
+}

test/jdk/jdk/internal/platform/docker/TestDockerCpuMetrics.java

@@ -65,11 +65,13 @@ public class TestDockerCpuMetrics {
                 testCpuSet((((numCpus - 1) / 2) + 1) + "-" + (numCpus - 1));
             }
             testCpuSet(IntStream.range(0, numCpus).mapToObj(a -> Integer.toString(a)).collect(Collectors.joining(",")));
+            testCpuSet("0", true /* additional cgroup fs mount */);
 
             testCpuQuota(50 * 1000, 100 * 1000);
             testCpuQuota(100 * 1000, 100 * 1000);
             testCpuQuota(150 * 1000, 100 * 1000);
             testCpuQuota(400 * 1000, 100 * 1000);
+            testCpuQuota(200 * 1000, 100 * 1000, true /* additional cgroup fs mount */);
 
             testCpuShares(256);
             testCpuShares(2048);
@@ -108,10 +110,18 @@ public class TestDockerCpuMetrics {
     }
 
     private static void testCpuSet(String value) throws Exception {
+        testCpuSet(value, false);
+    }
+
+    private static void testCpuSet(String value, boolean addCgroupMount) throws Exception {
         Common.logNewTestCase("testCpuSet, value = " + value);
         DockerRunOptions opts =
                 new DockerRunOptions(imageName, "/jdk/bin/java", "MetricsCpuTester");
         opts.addDockerOpts("--volume", Utils.TEST_CLASSES + ":/test-classes/");
+        if (addCgroupMount) {
+            // Extra cgroup mount should be ignored by product code
+            opts.addDockerOpts("--volume", "/sys/fs/cgroup:/cgroup-in:ro");
+        }
         opts.addJavaOpts("-cp", "/test-classes/");
         opts.addJavaOpts("--add-exports", "java.base/jdk.internal.platform=ALL-UNNAMED");
         opts.addClassOptions("cpusets", value);
@@ -120,10 +130,18 @@ public class TestDockerCpuMetrics {
     }
 
     private static void testCpuQuota(long quota, long period) throws Exception {
+        testCpuQuota(quota, period, false);
+    }
+
+    private static void testCpuQuota(long quota, long period, boolean addCgroupMount) throws Exception {
         Common.logNewTestCase("testCpuQuota, quota = " + quota + ", period = " + period);
         DockerRunOptions opts =
                 new DockerRunOptions(imageName, "/jdk/bin/java", "MetricsCpuTester");
         opts.addDockerOpts("--volume", Utils.TEST_CLASSES + ":/test-classes/");
+        if (addCgroupMount) {
+            // Extra cgroup mount should be ignored by product code
+            opts.addDockerOpts("--volume", "/sys/fs/cgroup:/cgroup-in:ro");
+        }
         opts.addDockerOpts("--cpu-period=" + period).addDockerOpts("--cpu-quota=" + quota);
         opts.addJavaOpts("-cp", "/test-classes/").addJavaOpts("--add-exports", "java.base/jdk.internal.platform=ALL-UNNAMED");
         opts.addClassOptions("cpuquota", quota + "", period + "");

test/jdk/jdk/internal/platform/docker/TestDockerMemoryMetrics.java

@@ -56,6 +56,8 @@ public class TestDockerMemoryMetrics {
         try {
             testMemoryLimit("200m");
             testMemoryLimit("1g");
+            // Memory limit test with additional cgroup fs mounted
+            testMemoryLimit("500m", true /* cgroup fs mount */);
 
             testMemoryAndSwapLimit("200m", "1g");
             testMemoryAndSwapLimit("100m", "200m");
@@ -85,6 +87,10 @@ public class TestDockerMemoryMetrics {
     }
 
     private static void testMemoryLimit(String value) throws Exception {
+        testMemoryLimit(value, false);
+    }
+
+    private static void testMemoryLimit(String value, boolean addCgroupMount) throws Exception {
         Common.logNewTestCase("testMemoryLimit, value = " + value);
         DockerRunOptions opts =
                 new DockerRunOptions(imageName, "/jdk/bin/java", "MetricsMemoryTester");
@@ -93,6 +99,10 @@ public class TestDockerMemoryMetrics {
                 .addJavaOpts("-cp", "/test-classes/")
                 .addJavaOpts("--add-exports", "java.base/jdk.internal.platform=ALL-UNNAMED")
                 .addClassOptions("memory", value);
+        if (addCgroupMount) {
+            // Extra cgroup mount should be ignored by product code
+            opts.addDockerOpts("--volume", "/sys/fs/cgroup:/cgroup-in:ro");
+        }
         DockerTestUtils.dockerRunJava(opts).shouldHaveExitValue(0).shouldContain("TEST PASSED!!!");
     }