Skip to content

Commit 2bbf8a2

Browse files
jerboaajerboaa
committedOct 9, 2020
8245543: Cgroups: Incorrect detection logic on some systems (still reproducible)
Reviewed-by: bobv, shade
1 parent aaa0a2a commit 2bbf8a2

File tree

4 files changed

+96
-6
lines changed

4 files changed

+96
-6
lines changed
 

‎src/hotspot/os/linux/cgroupSubsystem_linux.cpp

+5-2
Original file line numberDiff line numberDiff line change
@@ -294,14 +294,15 @@ bool CgroupSubsystemFactory::determine_type(CgroupInfo* cg_infos,
294294
// Skip cgroup2 fs lines on hybrid or unified hierarchy.
295295
continue;
296296
}
297-
any_cgroup_mounts_found = true;
298297
while ((token = strsep(&cptr, ",")) != NULL) {
299298
if (strcmp(token, "memory") == 0) {
299+
any_cgroup_mounts_found = true;
300300
assert(cg_infos[MEMORY_IDX]._mount_path == NULL, "stomping of _mount_path");
301301
cg_infos[MEMORY_IDX]._mount_path = os::strdup(tmpmount);
302302
cg_infos[MEMORY_IDX]._root_mount_path = os::strdup(tmproot);
303303
cg_infos[MEMORY_IDX]._data_complete = true;
304304
} else if (strcmp(token, "cpuset") == 0) {
305+
any_cgroup_mounts_found = true;
305306
if (cg_infos[CPUSET_IDX]._mount_path != NULL) {
306307
// On some systems duplicate cpuset controllers get mounted in addition to
307308
// the main cgroup controllers most likely under /sys/fs/cgroup. In that
@@ -321,11 +322,13 @@ bool CgroupSubsystemFactory::determine_type(CgroupInfo* cg_infos,
321322
cg_infos[CPUSET_IDX]._root_mount_path = os::strdup(tmproot);
322323
cg_infos[CPUSET_IDX]._data_complete = true;
323324
} else if (strcmp(token, "cpu") == 0) {
325+
any_cgroup_mounts_found = true;
324326
assert(cg_infos[CPU_IDX]._mount_path == NULL, "stomping of _mount_path");
325327
cg_infos[CPU_IDX]._mount_path = os::strdup(tmpmount);
326328
cg_infos[CPU_IDX]._root_mount_path = os::strdup(tmproot);
327329
cg_infos[CPU_IDX]._data_complete = true;
328330
} else if (strcmp(token, "cpuacct") == 0) {
331+
any_cgroup_mounts_found = true;
329332
assert(cg_infos[CPUACCT_IDX]._mount_path == NULL, "stomping of _mount_path");
330333
cg_infos[CPUACCT_IDX]._mount_path = os::strdup(tmpmount);
331334
cg_infos[CPUACCT_IDX]._root_mount_path = os::strdup(tmproot);
@@ -339,7 +342,7 @@ bool CgroupSubsystemFactory::determine_type(CgroupInfo* cg_infos,
339342
// Neither cgroup2 nor cgroup filesystems mounted via /proc/self/mountinfo
340343
// No point in continuing.
341344
if (!any_cgroup_mounts_found) {
342-
log_trace(os, container)("No cgroup controllers mounted.");
345+
log_trace(os, container)("No relevant cgroup controllers mounted.");
343346
cleanup(cg_infos);
344347
*flags = INVALID_CGROUPS_NO_MOUNT;
345348
return false;

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

+57-4
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@
3333
import java.util.List;
3434
import java.util.Map;
3535
import java.util.Optional;
36+
import java.util.regex.Matcher;
37+
import java.util.regex.Pattern;
3638
import java.util.stream.Stream;
3739

3840
import jdk.internal.platform.cgroupv1.CgroupV1Subsystem;
@@ -46,6 +48,31 @@ public class CgroupSubsystemFactory {
4648
private static final String BLKIO_CTRL = "blkio";
4749
private static final String MEMORY_CTRL = "memory";
4850

51+
/*
52+
* From https://www.kernel.org/doc/Documentation/filesystems/proc.txt
53+
*
54+
* 36 35 98:0 /mnt1 /mnt2 rw,noatime master:1 - ext3 /dev/root rw,errors=continue
55+
* (1)(2)(3) (4) (5) (6) (7) (8) (9) (10) (11)
56+
*
57+
* (1) mount ID: unique identifier of the mount (may be reused after umount)
58+
* (2) parent ID: ID of parent (or of self for the top of the mount tree)
59+
* (3) major:minor: value of st_dev for files on filesystem
60+
* (4) root: root of the mount within the filesystem
61+
* (5) mount point: mount point relative to the process's root
62+
* (6) mount options: per mount options
63+
* (7) optional fields: zero or more fields of the form "tag[:value]"
64+
* (8) separator: marks the end of the optional fields
65+
* (9) filesystem type: name of filesystem of the form "type[.subtype]"
66+
* (10) mount source: filesystem specific information or "none"
67+
* (11) super options: per super block options
68+
*/
69+
private static final Pattern MOUNTINFO_PATTERN = Pattern.compile(
70+
"^[^\\s]+\\s+[^\\s]+\\s+[^\\s]+\\s+" + // (1), (2), (3)
71+
"[^\\s]+\\s+([^\\s]+)\\s+" + // (4), (5) - group 1: mount point
72+
"[^-]+-\\s+" + // (6), (7), (8)
73+
"([^\\s]+)\\s+" + // (9) - group 2: filesystem type
74+
".*$"); // (10), (11)
75+
4976
static CgroupMetrics create() {
5077
Optional<CgroupTypeResult> optResult = null;
5178
try {
@@ -114,12 +141,13 @@ public static Optional<CgroupTypeResult> determineType(String mountInfo, String
114141
anyControllersEnabled = anyControllersEnabled || info.isEnabled();
115142
}
116143

117-
// If there are no mounted controllers in mountinfo, but we've only
118-
// seen 0 hierarchy IDs in /proc/cgroups, we are on a cgroups v1 system.
144+
// If there are no mounted, relevant cgroup controllers in mountinfo and only
145+
// 0 hierarchy IDs in /proc/cgroups have been seen, we are on a cgroups v1 system.
119146
// However, continuing in that case does not make sense as we'd need
120-
// information from mountinfo for the mounted controller paths anyway.
147+
// information from mountinfo for the mounted controller paths which we wouldn't
148+
// find anyway in that case.
121149
try (Stream<String> mntInfo = CgroupUtil.readFilePrivileged(Paths.get(mountInfo))) {
122-
boolean anyCgroupMounted = mntInfo.anyMatch(line -> line.contains("cgroup"));
150+
boolean anyCgroupMounted = mntInfo.anyMatch(CgroupSubsystemFactory::isRelevantControllerMount);
123151
if (!anyCgroupMounted && isCgroupsV2) {
124152
return Optional.empty();
125153
}
@@ -128,6 +156,31 @@ public static Optional<CgroupTypeResult> determineType(String mountInfo, String
128156
return Optional.of(result);
129157
}
130158

159+
private static boolean isRelevantControllerMount(String line) {
160+
Matcher lineMatcher = MOUNTINFO_PATTERN.matcher(line.trim());
161+
if (lineMatcher.matches()) {
162+
String mountPoint = lineMatcher.group(1);
163+
String fsType = lineMatcher.group(2);
164+
if (fsType.equals("cgroup")) {
165+
String filename = Paths.get(mountPoint).getFileName().toString();
166+
for (String fn: filename.split(",")) {
167+
switch (fn) {
168+
case MEMORY_CTRL: // fall through
169+
case CPU_CTRL:
170+
case CPUSET_CTRL:
171+
case CPUACCT_CTRL:
172+
case BLKIO_CTRL:
173+
return true;
174+
default: break; // ignore not recognized controllers
175+
}
176+
}
177+
} else if (fsType.equals("cgroup2")) {
178+
return true;
179+
}
180+
}
181+
return false;
182+
}
183+
131184
public static final class CgroupTypeResult {
132185
private final boolean isCgroupV2;
133186
private final boolean anyControllersEnabled;

‎test/hotspot/jtreg/containers/cgroup/CgroupSubsystemFactory.java

+18
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ public class CgroupSubsystemFactory {
6262
private Path cgroupv1MntInfoNonZeroHierarchy;
6363
private Path cgroupv1MntInfoDoubleCpuset;
6464
private Path cgroupv1MntInfoDoubleCpuset2;
65+
private Path cgroupv1MntInfoSystemdOnly;
6566
private String mntInfoEmpty = "";
6667
private Path cgroupV1SelfCgroup;
6768
private Path cgroupV2SelfCgroup;
@@ -128,6 +129,9 @@ public class CgroupSubsystemFactory {
128129
"pids 3 80 1";
129130
private String mntInfoCgroupsV2Only =
130131
"28 21 0:25 / /sys/fs/cgroup rw,nosuid,nodev,noexec,relatime shared:4 - cgroup2 none rw,seclabel,nsdelegate";
132+
private String mntInfoCgroupsV1SystemdOnly =
133+
"35 26 0:26 / /sys/fs/cgroup/systemd rw,nosuid,nodev,noexec,relatime - cgroup systemd rw,name=systemd\n" +
134+
"26 18 0:19 / /sys/fs/cgroup rw,relatime - tmpfs none rw,size=4k,mode=755\n";
131135

132136
private void setup() {
133137
try {
@@ -168,6 +172,9 @@ private void setup() {
168172

169173
cgroupv1MntInfoDoubleCpuset2 = Paths.get(existingDirectory.toString(), "mnt_info_cgroupv1_double_cpuset2");
170174
Files.writeString(cgroupv1MntInfoDoubleCpuset2, mntInfoCgroupv1DoubleCpuset2);
175+
176+
cgroupv1MntInfoSystemdOnly = Paths.get(existingDirectory.toString(), "mnt_info_cgroupv1_systemd_only");
177+
Files.writeString(cgroupv1MntInfoSystemdOnly, mntInfoCgroupsV1SystemdOnly);
171178
} catch (IOException e) {
172179
throw new RuntimeException(e);
173180
}
@@ -195,6 +202,16 @@ public void testCgroupv1MultipleCpusetMounts(WhiteBox wb, Path mountInfo) {
195202
System.out.println("testCgroupv1MultipleCpusetMounts PASSED!");
196203
}
197204

205+
public void testCgroupv1SystemdOnly(WhiteBox wb) {
206+
String procCgroups = cgroupv1CgInfoZeroHierarchy.toString();
207+
String procSelfCgroup = cgroupV1SelfCgroup.toString();
208+
String procSelfMountinfo = cgroupv1MntInfoSystemdOnly.toString();
209+
int retval = wb.validateCgroup(procCgroups, procSelfCgroup, procSelfMountinfo);
210+
Asserts.assertEQ(INVALID_CGROUPS_NO_MOUNT, retval, "Only systemd mounted. Invalid");
211+
Asserts.assertFalse(isValidCgroup(retval));
212+
System.out.println("testCgroupv1SystemdOnly PASSED!");
213+
}
214+
198215
public void testCgroupv1NoMounts(WhiteBox wb) {
199216
String procCgroups = cgroupv1CgInfoZeroHierarchy.toString();
200217
String procSelfCgroup = cgroupV1SelfCgroup.toString();
@@ -261,6 +278,7 @@ public static void main(String[] args) throws Exception {
261278
CgroupSubsystemFactory test = new CgroupSubsystemFactory();
262279
test.setup();
263280
try {
281+
test.testCgroupv1SystemdOnly(wb);
264282
test.testCgroupv1NoMounts(wb);
265283
test.testCgroupv2(wb);
266284
test.testCgroupV1Hybrid(wb);

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

+16
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ public class TestCgroupSubsystemFactory {
5656
private Path cgroupv2MntInfoZeroHierarchy;
5757
private Path cgroupv1CgInfoNonZeroHierarchy;
5858
private Path cgroupv1MntInfoNonZeroHierarchy;
59+
private Path cgroupv1MntInfoSystemdOnly;
5960
private String mntInfoEmpty = "";
6061
private String cgroupsZeroHierarchy =
6162
"#subsys_name hierarchy num_cgroups enabled\n" +
@@ -98,6 +99,9 @@ public class TestCgroupSubsystemFactory {
9899
"pids 3 80 1";
99100
private String mntInfoCgroupsV2Only =
100101
"28 21 0:25 / /sys/fs/cgroup rw,nosuid,nodev,noexec,relatime shared:4 - cgroup2 cgroup2 rw,seclabel,nsdelegate";
102+
private String mntInfoCgroupsV1SystemdOnly =
103+
"35 26 0:26 / /sys/fs/cgroup/systemd rw,nosuid,nodev,noexec,relatime - cgroup systemd rw,name=systemd\n" +
104+
"26 18 0:19 / /sys/fs/cgroup rw,relatime - tmpfs none rw,size=4k,mode=755\n";
101105

102106
@Before
103107
public void setup() {
@@ -118,6 +122,9 @@ public void setup() {
118122

119123
cgroupv1MntInfoNonZeroHierarchy = Paths.get(existingDirectory.toString(), "mountinfo_non_zero");
120124
Files.writeString(cgroupv1MntInfoNonZeroHierarchy, mntInfoHybrid);
125+
126+
cgroupv1MntInfoSystemdOnly = Paths.get(existingDirectory.toString(), "mountinfo_cgroupv1_systemd_only");
127+
Files.writeString(cgroupv1MntInfoSystemdOnly, mntInfoCgroupsV1SystemdOnly);
121128
} catch (IOException e) {
122129
throw new RuntimeException(e);
123130
}
@@ -132,6 +139,15 @@ public void teardown() {
132139
}
133140
}
134141

142+
@Test
143+
public void testCgroupv1SystemdOnly() throws IOException {
144+
String cgroups = cgroupv1CgInfoZeroHierarchy.toString();
145+
String mountInfo = cgroupv1MntInfoSystemdOnly.toString();
146+
Optional<CgroupTypeResult> result = CgroupSubsystemFactory.determineType(mountInfo, cgroups);
147+
148+
assertTrue("zero hierarchy ids with no *relevant* controllers mounted", result.isEmpty());
149+
}
150+
135151
@Test
136152
public void testHybridCgroupsV1() throws IOException {
137153
String cgroups = cgroupv1CgInfoNonZeroHierarchy.toString();

0 commit comments

Comments
 (0)
Please sign in to comment.