[PATCH] 8217766: Container Support doesn't work for some Join Controllers combinations
Benjamin Pineau
ben.pineau at gmail.com
Wed Feb 27 06:58:01 UTC 2019
Hi,
Here is a patch for the cgroup identification issue reported
in https://bugs.openjdk.java.net/browse/JDK-8217766
I do not have a sponsor, is someone here willing to help?
Thanks,
# HG changeset patch
# User Benjamin Pineau <ben.pineau at gmail.com>
# Date 1551124966 -3600
# Mon Feb 25 21:02:46 2019 +0100
# Node ID 1217b6d4cc5fbe07596ad131cf628b50d091f5f2
# Parent ee4488381c78135bbb93ad93d6883ff5b8d0d721
[PATCH] 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 runing
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.
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, "\n");
- 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);
}
}
}
More information about the hotspot-runtime-dev
mailing list