[PATCHv2] 8217766: Container Support doesn't work for some Join Controllers combinations
Benjamin Pineau
benjamin.pineau at datadoghq.com
Sun Mar 10 18:40:37 UTC 2019
Hi Bob,
Thanks for the quick review and sponsorship offer!
Here's the updated version, with containers metrics.
# HG changeset patch
# User Benjamin Pineau <benjamin.pineau at datadoghq.com>
# Date 1552235922 -3600
# Sun Mar 10 17:38:42 2019 +0100
# Node ID c30c43d29314ac087d3e6cec1afef6faf7233103
# Parent 0324b3756aa261672d0f745b2a9a651fee69d262
[PATCHv2] 8217766: Container Support doesn't work for some Join Controllers combinations
The cgroup identification -implemented by parsing /proc/self/mountinfo
and /proc/self/cgroup- assumed each cgroup controller was mounted
disjoint from the others (except for "cpu" and "cpuacct" controllers).
Which means, we expected one single controller per mountinfo line.
This matches the way most Linux distributions currently configure
cgroupsv1 by default. Yet controllers can be grouped arbitrarily.
For instance, using the JoinControllers systemd directive.
One use case for that is to let Kubernetes' kubelet discover his own
dedicated and reserved cgroup hierarchy. In that situation, the JVM
fails to discover the expected cgroup controllers set, and, when running
containerized, default to a suboptimal understanding of available resources.
Supporting arbitrarily controllers groups per mountpoint actually
allows for simpler and easier to follow code, as we don't need nested
if/else for every controller.
v2: also update Containers Metrics, to support joint controllers.
diff --git a/src/hotspot/os/linux/osContainer_linux.cpp b/src/hotspot/os/linux/osContainer_linux.cpp
--- a/src/hotspot/os/linux/osContainer_linux.cpp
+++ b/src/hotspot/os/linux/osContainer_linux.cpp
@@ -217,16 +217,11 @@
* we are running under cgroup control.
*/
void OSContainer::init() {
- int mountid;
- int parentid;
- int major;
- int minor;
FILE *mntinfo = NULL;
FILE *cgroup = NULL;
char buf[MAXPATHLEN+1];
char tmproot[MAXPATHLEN+1];
char tmpmount[MAXPATHLEN+1];
- char tmpbase[MAXPATHLEN+1];
char *p;
jlong mem_limit;
@@ -260,87 +255,27 @@
return;
}
- while ( (p = fgets(buf, MAXPATHLEN, mntinfo)) != NULL) {
- // Look for the filesystem type and see if it's cgroup
- char fstype[MAXPATHLEN+1];
- fstype[0] = '\0';
- char *s = strstr(p, " - ");
- if (s != NULL &&
- sscanf(s, " - %s", fstype) == 1 &&
- strcmp(fstype, "cgroup") == 0) {
+ while ((p = fgets(buf, MAXPATHLEN, mntinfo)) != NULL) {
+ char tmpcgroups[MAXPATHLEN+1];
+ char *cptr = tmpcgroups;
+ char *token;
+
+ // mountinfo format is documented at https://www.kernel.org/doc/Documentation/filesystems/proc.txt
+ if (sscanf(p, "%*d %*d %*d:%*d %s %s %*[^-]- cgroup %*s %s", tmproot, tmpmount, tmpcgroups) != 3) {
+ continue;
+ }
- if (strstr(p, "memory") != NULL) {
- int matched = sscanf(p, "%d %d %d:%d %s %s",
- &mountid,
- &parentid,
- &major,
- &minor,
- tmproot,
- tmpmount);
- if (matched == 6) {
- memory = new CgroupSubsystem(tmproot, tmpmount);
- }
- else
- log_debug(os, container)("Incompatible str containing cgroup and memory: %s", p);
- } else if (strstr(p, "cpuset") != NULL) {
- int matched = sscanf(p, "%d %d %d:%d %s %s",
- &mountid,
- &parentid,
- &major,
- &minor,
- tmproot,
- tmpmount);
- if (matched == 6) {
- cpuset = new CgroupSubsystem(tmproot, tmpmount);
- }
- else {
- log_debug(os, container)("Incompatible str containing cgroup and cpuset: %s", p);
- }
- } else if (strstr(p, "cpu,cpuacct") != NULL || strstr(p, "cpuacct,cpu") != NULL) {
- int matched = sscanf(p, "%d %d %d:%d %s %s",
- &mountid,
- &parentid,
- &major,
- &minor,
- tmproot,
- tmpmount);
- if (matched == 6) {
- cpu = new CgroupSubsystem(tmproot, tmpmount);
- cpuacct = new CgroupSubsystem(tmproot, tmpmount);
- }
- else {
- log_debug(os, container)("Incompatible str containing cgroup and cpu,cpuacct: %s", p);
- }
- } else if (strstr(p, "cpuacct") != NULL) {
- int matched = sscanf(p, "%d %d %d:%d %s %s",
- &mountid,
- &parentid,
- &major,
- &minor,
- tmproot,
- tmpmount);
- if (matched == 6) {
- cpuacct = new CgroupSubsystem(tmproot, tmpmount);
- }
- else {
- log_debug(os, container)("Incompatible str containing cgroup and cpuacct: %s", p);
- }
- } else if (strstr(p, "cpu") != NULL) {
- int matched = sscanf(p, "%d %d %d:%d %s %s",
- &mountid,
- &parentid,
- &major,
- &minor,
- tmproot,
- tmpmount);
- if (matched == 6) {
- cpu = new CgroupSubsystem(tmproot, tmpmount);
- }
- else {
- log_debug(os, container)("Incompatible str containing cgroup and cpu: %s", p);
- }
- }
- }
+ while ((token = strsep(&cptr, ",")) != NULL) {
+ if (strcmp(token, "memory") == 0) {
+ memory = new CgroupSubsystem(tmproot, tmpmount);
+ } else if (strcmp(token, "cpuset") == 0) {
+ cpuset = new CgroupSubsystem(tmproot, tmpmount);
+ } else if (strcmp(token, "cpu") == 0) {
+ cpu = new CgroupSubsystem(tmproot, tmpmount);
+ } else if (strcmp(token, "cpuacct") == 0) {
+ cpuacct= new CgroupSubsystem(tmproot, tmpmount);
+ }
+ }
}
fclose(mntinfo);
@@ -392,30 +327,30 @@
return;
}
- while ( (p = fgets(buf, MAXPATHLEN, cgroup)) != NULL) {
- int cgno;
- int matched;
- char *controller;
+ while ((p = fgets(buf, MAXPATHLEN, cgroup)) != NULL) {
+ char *controllers;
+ char *token;
char *base;
/* Skip cgroup number */
strsep(&p, ":");
- /* Get controller and base */
- controller = strsep(&p, ":");
+ /* Get controllers and base */
+ controllers = strsep(&p, ":");
base = strsep(&p, "
");
- if (controller != NULL) {
- if (strstr(controller, "memory") != NULL) {
+ if (controllers == NULL) {
+ continue;
+ }
+
+ while ((token = strsep(&controllers, ",")) != NULL) {
+ if (strcmp(token, "memory") == 0) {
memory->set_subsystem_path(base);
- } else if (strstr(controller, "cpuset") != NULL) {
+ } else if (strcmp(token, "cpuset") == 0) {
cpuset->set_subsystem_path(base);
- } else if (strstr(controller, "cpu,cpuacct") != NULL || strstr(controller, "cpuacct,cpu") != NULL) {
+ } else if (strcmp(token, "cpu") == 0) {
cpu->set_subsystem_path(base);
+ } else if (strcmp(token, "cpuacct") == 0) {
cpuacct->set_subsystem_path(base);
- } else if (strstr(controller, "cpuacct") != NULL) {
- cpuacct->set_subsystem_path(base);
- } else if (strstr(controller, "cpu") != NULL) {
- cpu->set_subsystem_path(base);
}
}
}
diff --git a/src/java.base/linux/classes/jdk/internal/platform/cgroupv1/Metrics.java b/src/java.base/linux/classes/jdk/internal/platform/cgroupv1/Metrics.java
--- a/src/java.base/linux/classes/jdk/internal/platform/cgroupv1/Metrics.java
+++ b/src/java.base/linux/classes/jdk/internal/platform/cgroupv1/Metrics.java
@@ -128,9 +128,9 @@
if (mountentry.length < 5) return;
Path p = Paths.get(mountentry[4]);
- String subsystemName = p.getFileName().toString();
+ String []subsystemNames = p.getFileName().toString().split(",");
- if (subsystemName != null) {
+ for (String subsystemName: subsystemNames) {
switch (subsystemName) {
case "memory":
metric.setMemorySubSystem(new SubSystem(mountentry[3], mountentry[4]));
@@ -138,11 +138,6 @@
case "cpuset":
metric.setCpuSetSubSystem(new SubSystem(mountentry[3], mountentry[4]));
break;
- case "cpu,cpuacct":
- case "cpuacct,cpu":
- metric.setCpuSubSystem(new SubSystem(mountentry[3], mountentry[4]));
- metric.setCpuAcctSubSystem(new SubSystem(mountentry[3], mountentry[4]));
- break;
case "cpuacct":
metric.setCpuAcctSubSystem(new SubSystem(mountentry[3], mountentry[4]));
break;
More information about the hotspot-runtime-dev
mailing list