Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
8239559: Cgroups: Incorrect detection logic on some systems
Adjust heuristic with cgroup mounts according to mountinfo

Reviewed-by: sgehwolf
Backport-of: 53ee0c4963007b86db7979312b81f990e6ce271a
  • Loading branch information
Jonathan Dowland committed Dec 13, 2022
1 parent a318e00 commit b082617
Show file tree
Hide file tree
Showing 3 changed files with 295 additions and 29 deletions.
Expand Up @@ -30,11 +30,13 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.stream.Stream;

import jdk.internal.platform.cgroupv1.CgroupV1Subsystem;
import jdk.internal.platform.cgroupv2.CgroupV2Subsystem;

class CgroupSubsystemFactory {
public class CgroupSubsystemFactory {

private static final String CPU_CTRL = "cpu";
private static final String CPUACCT_CTRL = "cpuacct";
Expand All @@ -43,55 +45,114 @@ class CgroupSubsystemFactory {
private static final String MEMORY_CTRL = "memory";

static CgroupMetrics create() {
Map<String, CgroupInfo> infos = new HashMap<>();
Optional<CgroupTypeResult> optResult = null;
try {
List<String> lines = CgroupUtil.readAllLinesPrivileged(Paths.get("/proc/cgroups"));
for (String line : lines) {
if (line.startsWith("#")) {
continue;
}
CgroupInfo info = CgroupInfo.fromCgroupsLine(line);
switch (info.getName()) {
case CPU_CTRL: infos.put(CPU_CTRL, info); break;
case CPUACCT_CTRL: infos.put(CPUACCT_CTRL, info); break;
case CPUSET_CTRL: infos.put(CPUSET_CTRL, info); break;
case MEMORY_CTRL: infos.put(MEMORY_CTRL, info); break;
case BLKIO_CTRL: infos.put(BLKIO_CTRL, info); break;
}
}
optResult = determineType("/proc/self/mountinfo", "/proc/cgroups");
} catch (IOException e) {
return null;
}

// For cgroups v1 all controllers need to have non-zero hierarchy id
boolean isCgroupsV2 = true;
boolean anyControllersEnabled = false;
boolean anyCgroupsV2Controller = false;
boolean anyCgroupsV1Controller = false;
for (CgroupInfo info: infos.values()) {
anyCgroupsV1Controller = anyCgroupsV1Controller || info.getHierarchyId() != 0;
anyCgroupsV2Controller = anyCgroupsV2Controller || info.getHierarchyId() == 0;
isCgroupsV2 = isCgroupsV2 && info.getHierarchyId() == 0;
anyControllersEnabled = anyControllersEnabled || info.isEnabled();
if (optResult.equals(Optional.empty())) {
return null;
}
CgroupTypeResult result = optResult.get();

// If no controller is enabled, return no metrics.
if (!anyControllersEnabled) {
if (!result.isAnyControllersEnabled()) {
return null;
}
// The code is not ready to deal with mixed cgroups v1 and cgroups v2
// controllers on a per-controller basis. Return no metrics in that
// case
if (anyCgroupsV1Controller && anyCgroupsV2Controller) {
if (result.isAnyCgroupV1Controllers() && result.isAnyCgroupV2Controllers()) {
return null;
}

if (isCgroupsV2) {
if (result.isCgroupV2()) {
CgroupSubsystem subsystem = CgroupV2Subsystem.getInstance();
return subsystem != null ? new CgroupMetrics(subsystem) : null;
} else {
CgroupV1Subsystem subsystem = CgroupV1Subsystem.getInstance();
return subsystem != null ? new CgroupV1MetricsImpl(subsystem) : null;
}
}

public static Optional<CgroupTypeResult> determineType(String mountInfo, String cgroups) throws IOException {
Map<String, CgroupInfo> infos = new HashMap<>();
List<String> lines = CgroupUtil.readAllLinesPrivileged(Paths.get(cgroups));
for (String line : lines) {
if (line.startsWith("#")) {
continue;
}
CgroupInfo info = CgroupInfo.fromCgroupsLine(line);
switch (info.getName()) {
case CPU_CTRL: infos.put(CPU_CTRL, info); break;
case CPUACCT_CTRL: infos.put(CPUACCT_CTRL, info); break;
case CPUSET_CTRL: infos.put(CPUSET_CTRL, info); break;
case MEMORY_CTRL: infos.put(MEMORY_CTRL, info); break;
case BLKIO_CTRL: infos.put(BLKIO_CTRL, info); break;
}
}

// For cgroups v2 all controllers need to have zero hierarchy id
// and /proc/self/mountinfo needs to have at least one cgroup filesystem
// mounted. Note that hybrid hierarchy has controllers mounted via
// cgroup v1. In that case hierarchy id's will be non-zero.
boolean isCgroupsV2 = true;
boolean anyControllersEnabled = false;
boolean anyCgroupsV2Controller = false;
boolean anyCgroupsV1Controller = false;
for (CgroupInfo info: infos.values()) {
anyCgroupsV1Controller = anyCgroupsV1Controller || info.getHierarchyId() != 0;
anyCgroupsV2Controller = anyCgroupsV2Controller || info.getHierarchyId() == 0;
isCgroupsV2 = isCgroupsV2 && info.getHierarchyId() == 0;
anyControllersEnabled = anyControllersEnabled || info.isEnabled();
}

// If there are no mounted controllers in mountinfo, but we've only
// seen 0 hierarchy IDs in /proc/cgroups, we are on a cgroups v1 system.
// However, continuing in that case does not make sense as we'd need
// information from mountinfo for the mounted controller paths anyway.
try (Stream<String> mntInfo = CgroupUtil.readFilePrivileged(Paths.get(mountInfo))) {
boolean anyCgroupMounted = mntInfo.anyMatch(line -> line.contains("cgroup"));
if (!anyCgroupMounted && isCgroupsV2) {
return Optional.empty();
}
}
CgroupTypeResult result = new CgroupTypeResult(isCgroupsV2, anyControllersEnabled, anyCgroupsV2Controller, anyCgroupsV1Controller);
return Optional.of(result);
}

public static final class CgroupTypeResult {
private final boolean isCgroupV2;
private final boolean anyControllersEnabled;
private final boolean anyCgroupV2Controllers;
private final boolean anyCgroupV1Controllers;

private CgroupTypeResult(boolean isCgroupV2,
boolean anyControllersEnabled,
boolean anyCgroupV2Controllers,
boolean anyCgroupV1Controllers) {
this.isCgroupV2 = isCgroupV2;
this.anyControllersEnabled = anyControllersEnabled;
this.anyCgroupV1Controllers = anyCgroupV1Controllers;
this.anyCgroupV2Controllers = anyCgroupV2Controllers;
}

public boolean isCgroupV2() {
return isCgroupV2;
}

public boolean isAnyControllersEnabled() {
return anyControllersEnabled;
}

public boolean isAnyCgroupV2Controllers() {
return anyCgroupV2Controllers;
}

public boolean isAnyCgroupV1Controllers() {
return anyCgroupV1Controllers;
}
}
}
181 changes: 181 additions & 0 deletions jdk/test/jdk/internal/platform/cgroup/TestCgroupSubsystemFactory.java
@@ -0,0 +1,181 @@
/*
* Copyright (c) 2020, 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.
*/
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;

import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Optional;

import org.junit.After;
import org.junit.Before;
import org.junit.Test;

import jdk.internal.platform.CgroupSubsystemFactory;
import jdk.internal.platform.CgroupSubsystemFactory.CgroupTypeResult;
import jdk.testlibrary.Utils;
import jdk.testlibrary.FileUtils;


/*
* @test
* @requires os.family == "linux"
* @modules java.base/jdk.internal.platform
* @library /lib/testlibrary
* @run junit/othervm TestCgroupSubsystemFactory
*/
public class TestCgroupSubsystemFactory {

private Path existingDirectory;
private Path cgroupv1CgInfoZeroHierarchy;
private Path cgroupv1MntInfoZeroHierarchy;
private Path cgroupv2CgInfoZeroHierarchy;
private Path cgroupv2MntInfoZeroHierarchy;
private Path cgroupv1CgInfoNonZeroHierarchy;
private Path cgroupv1MntInfoNonZeroHierarchy;
private String mntInfoEmpty = "";
private String cgroupsZeroHierarchy =
"#subsys_name hierarchy num_cgroups enabled\n" +
"cpuset 0 1 1\n" +
"cpu 0 1 1\n" +
"cpuacct 0 1 1\n" +
"memory 0 1 1\n" +
"devices 0 1 1\n" +
"freezer 0 1 1\n" +
"net_cls 0 1 1\n" +
"blkio 0 1 1\n" +
"perf_event 0 1 1 ";
private String mntInfoHybrid =
"30 23 0:26 / /sys/fs/cgroup ro,nosuid,nodev,noexec shared:4 - tmpfs tmpfs ro,seclabel,mode=755\n" +
"31 30 0:27 / /sys/fs/cgroup/unified rw,nosuid,nodev,noexec,relatime shared:5 - cgroup2 cgroup2 rw,seclabel,nsdelegate\n" +
"32 30 0:28 / /sys/fs/cgroup/systemd rw,nosuid,nodev,noexec,relatime shared:6 - cgroup cgroup rw,seclabel,xattr,name=systemd\n" +
"35 30 0:31 / /sys/fs/cgroup/memory rw,nosuid,nodev,noexec,relatime shared:7 - cgroup cgroup rw,seclabel,memory\n" +
"36 30 0:32 / /sys/fs/cgroup/pids rw,nosuid,nodev,noexec,relatime shared:8 - cgroup cgroup rw,seclabel,pids\n" +
"37 30 0:33 / /sys/fs/cgroup/perf_event rw,nosuid,nodev,noexec,relatime shared:9 - cgroup cgroup rw,seclabel,perf_event\n" +
"38 30 0:34 / /sys/fs/cgroup/net_cls,net_prio rw,nosuid,nodev,noexec,relatime shared:10 - cgroup cgroup rw,seclabel,net_cls,net_prio\n" +
"39 30 0:35 / /sys/fs/cgroup/hugetlb rw,nosuid,nodev,noexec,relatime shared:11 - cgroup cgroup rw,seclabel,hugetlb\n" +
"40 30 0:36 / /sys/fs/cgroup/cpu,cpuacct rw,nosuid,nodev,noexec,relatime shared:12 - cgroup cgroup rw,seclabel,cpu,cpuacct\n" +
"41 30 0:37 / /sys/fs/cgroup/devices rw,nosuid,nodev,noexec,relatime shared:13 - cgroup cgroup rw,seclabel,devices\n" +
"42 30 0:38 / /sys/fs/cgroup/cpuset rw,nosuid,nodev,noexec,relatime shared:14 - cgroup cgroup rw,seclabel,cpuset\n" +
"43 30 0:39 / /sys/fs/cgroup/blkio rw,nosuid,nodev,noexec,relatime shared:15 - cgroup cgroup rw,seclabel,blkio\n" +
"44 30 0:40 / /sys/fs/cgroup/freezer rw,nosuid,nodev,noexec,relatime shared:16 - cgroup cgroup rw,seclabel,freezer";
private String cgroupsNonZeroHierarchy =
"#subsys_name hierarchy num_cgroups enabled\n" +
"cpuset 9 1 1\n" +
"cpu 7 1 1\n" +
"cpuacct 7 1 1\n" +
"blkio 10 1 1\n" +
"memory 2 90 1\n" +
"devices 8 74 1\n" +
"freezer 11 1 1\n" +
"net_cls 5 1 1\n" +
"perf_event 4 1 1\n" +
"net_prio 5 1 1\n" +
"hugetlb 6 1 1\n" +
"pids 3 80 1";
private String mntInfoCgroupsV2Only =
"28 21 0:25 / /sys/fs/cgroup rw,nosuid,nodev,noexec,relatime shared:4 - cgroup2 cgroup2 rw,seclabel,nsdelegate";

@Before
public void setup() {
try {
existingDirectory = Utils.createTempDirectory(TestCgroupSubsystemFactory.class.getSimpleName());
Path cgroupsZero = Paths.get(existingDirectory.toString(), "cgroups_zero");
Files.write(cgroupsZero, cgroupsZeroHierarchy.getBytes(StandardCharsets.UTF_8));
cgroupv1CgInfoZeroHierarchy = cgroupsZero;
cgroupv2CgInfoZeroHierarchy = cgroupsZero;
cgroupv1MntInfoZeroHierarchy = Paths.get(existingDirectory.toString(), "mountinfo_empty");
Files.write(cgroupv1MntInfoZeroHierarchy, mntInfoEmpty.getBytes());

cgroupv2MntInfoZeroHierarchy = Paths.get(existingDirectory.toString(), "mountinfo_cgroupv2");
Files.write(cgroupv2MntInfoZeroHierarchy, mntInfoCgroupsV2Only.getBytes());

cgroupv1CgInfoNonZeroHierarchy = Paths.get(existingDirectory.toString(), "cgroups_non_zero");
Files.write(cgroupv1CgInfoNonZeroHierarchy, cgroupsNonZeroHierarchy.getBytes());

cgroupv1MntInfoNonZeroHierarchy = Paths.get(existingDirectory.toString(), "mountinfo_non_zero");
Files.write(cgroupv1MntInfoNonZeroHierarchy, mntInfoHybrid.getBytes());
} catch (IOException e) {
throw new RuntimeException(e);
}
}

@After
public void teardown() {
try {
FileUtils.deleteFileTreeWithRetry(existingDirectory);
} catch (IOException e) {
System.err.println("Teardown failed. " + e.getMessage());
}
}

@Test
public void testHybridCgroupsV1() throws IOException {
String cgroups = cgroupv1CgInfoNonZeroHierarchy.toString();
String mountInfo = cgroupv1MntInfoNonZeroHierarchy.toString();
Optional<CgroupTypeResult> result = CgroupSubsystemFactory.determineType(mountInfo, cgroups);

assertTrue("Expected non-empty cgroup result", result.isPresent());
CgroupTypeResult res = result.get();
assertFalse("hybrid hierarchy expected as cgroups v1", res.isCgroupV2());
}

@Test
public void testZeroHierarchyCgroupsV1() throws IOException {
String cgroups = cgroupv1CgInfoZeroHierarchy.toString();
String mountInfo = cgroupv1MntInfoZeroHierarchy.toString();
Optional<CgroupTypeResult> result = CgroupSubsystemFactory.determineType(mountInfo, cgroups);

assertTrue("zero hierarchy ids with no mounted controllers => empty result", Optional.empty().equals(result));
}

@Test
public void testZeroHierarchyCgroupsV2() throws IOException {
String cgroups = cgroupv2CgInfoZeroHierarchy.toString();
String mountInfo = cgroupv2MntInfoZeroHierarchy.toString();
Optional<CgroupTypeResult> result = CgroupSubsystemFactory.determineType(mountInfo, cgroups);

assertTrue("Expected non-empty cgroup result", result.isPresent());
CgroupTypeResult res = result.get();

assertTrue("zero hierarchy ids with mounted controllers expected cgroups v2", res.isCgroupV2());
}

@Test(expected = IOException.class)
public void mountInfoFileNotFound() throws IOException {
String cgroups = cgroupv1CgInfoZeroHierarchy.toString(); // any existing file
String mountInfo = Paths.get(existingDirectory.toString(), "not-existing-mountinfo").toString();

CgroupSubsystemFactory.determineType(mountInfo, cgroups);
}

@Test(expected = IOException.class)
public void cgroupsFileNotFound() throws IOException {
String cgroups = Paths.get(existingDirectory.toString(), "not-existing-cgroups").toString();
String mountInfo = cgroupv2MntInfoZeroHierarchy.toString(); // any existing file
CgroupSubsystemFactory.determineType(mountInfo, cgroups);
}
}
24 changes: 24 additions & 0 deletions jdk/test/lib/testlibrary/jdk/testlibrary/Utils.java
Expand Up @@ -38,7 +38,11 @@
import java.util.List;
import java.util.Arrays;
import java.util.LinkedList;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.attribute.FileAttribute;

import java.util.Collections;
import java.util.Objects;
import java.util.regex.Pattern;
Expand Down Expand Up @@ -440,4 +444,24 @@ public static List<Integer> parseBreakpoints(String filePath, int id) {
public static String getTestSourcePath(String fileName) {
return Paths.get(System.getProperty("test.src")).resolve(fileName).toString();
}

/**
* Creates an empty directory in "user.dir" or "."
* <p>
* This method is meant as a replacement for {@code Files#createTempDirectory(String, String, FileAttribute...)}
* that doesn't leave files behind in /tmp directory of the test machine
* <p>
* If the property "user.dir" is not set, "." will be used.
*
* @param prefix
* @param attrs
* @return the path to the newly created directory
* @throws IOException
*
* @see {@link Files#createTempDirectory(String, String, FileAttribute...)}
*/
public static Path createTempDirectory(String prefix, FileAttribute<?>... attrs) throws IOException {
Path dir = Paths.get(System.getProperty("user.dir", "."));
return Files.createTempDirectory(dir, prefix);
}
}

1 comment on commit b082617

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.