Support docker cgroup limits (#13488)
authorTom Deseyn <tom.deseyn@gmail.com>
Mon, 21 Aug 2017 18:56:32 +0000 (20:56 +0200)
committerJan Vorlicek <janvorli@microsoft.com>
Mon, 21 Aug 2017 18:56:32 +0000 (20:56 +0200)
* Fix cgroup mountinfo parsing

The parsing would find the wrong '-' in lines like this:
354 347 0:28 /system.slice/docker-654dd7b6b8bbfe1739ae3309b471e95ccc82b3a3f56b7879f0a811d68b5c4e1d.scope /sys/fs/cgroup/cpuacct,cpu ro,nosuid,nodev,noexec,relatime - cgroup cgroup rw,cpuacct,cpu

* cgroup: don't append cgroup relative path for reading docker limits

src/gc/unix/cgroup.cpp
src/pal/src/misc/cgroup.cpp

index 4721f09f86b2001b5a06138bd3899d9baa29048a..992678b634f87a63e8cc216e66fcc3994f88eb6b 100644 (file)
@@ -36,8 +36,8 @@ class CGroup
 public:
     CGroup()
     {
-        m_memory_cgroup_path = FindMemoryCgroupPath();
-        m_cpu_cgroup_path = FindCpuCgroupPath();
+        m_memory_cgroup_path = FindCgroupPath(&IsMemorySubsystem);
+        m_cpu_cgroup_path = FindCgroupPath(&IsCpuSubsystem);
     }
 
     ~CGroup()
@@ -110,65 +110,45 @@ private:
         return strcmp("cpu", strTok) == 0;
     }
 
-    static char* FindMemoryCgroupPath(){
-        char *memory_cgroup_path = nullptr;
-        char *memory_hierarchy_mount = nullptr;
-        char *mem_cgroup_path_relative_to_mount = nullptr;
-
-        memory_hierarchy_mount = FindHierarchyMount(&IsMemorySubsystem);
-        if (memory_hierarchy_mount == nullptr)
-            goto done;
-
-        mem_cgroup_path_relative_to_mount = FindCGroupPathForSubsystem(&IsMemorySubsystem);
-        if (mem_cgroup_path_relative_to_mount == nullptr)
-            goto done;
-
-        memory_cgroup_path = (char*)malloc(strlen(memory_hierarchy_mount) + strlen(mem_cgroup_path_relative_to_mount) + 1);
-        if (memory_cgroup_path == nullptr)
-           goto done;
-
-        strcpy(memory_cgroup_path, memory_hierarchy_mount);
-        strcat(memory_cgroup_path, mem_cgroup_path_relative_to_mount);        
-
-    done:
-        free(memory_hierarchy_mount);
-        free(mem_cgroup_path_relative_to_mount);        
-        return memory_cgroup_path;
-    }
-
-    static char* FindCpuCgroupPath(){
-        char *cpu_cgroup_path = nullptr;
-        char *cpu_hierarchy_mount = nullptr;     
-        char *cpu_cgroup_path_relative_to_mount = nullptr;
+    static char* FindCgroupPath(bool (*is_subsystem)(const char *)){
+        char *cgroup_path = nullptr;
+        char *hierarchy_mount = nullptr;
+        char *hierarchy_root = nullptr;
+        char *cgroup_path_relative_to_mount = nullptr;
 
-        cpu_hierarchy_mount = FindHierarchyMount(&IsCpuSubsystem);
-        if (cpu_hierarchy_mount == nullptr)
+        FindHierarchyMount(is_subsystem, &hierarchy_mount, &hierarchy_root);
+        if (hierarchy_mount == nullptr || hierarchy_root == nullptr)
             goto done;
 
-        cpu_cgroup_path_relative_to_mount = FindCGroupPathForSubsystem(&IsCpuSubsystem);
-        if (cpu_cgroup_path_relative_to_mount == nullptr)
+        cgroup_path_relative_to_mount = FindCGroupPathForSubsystem(is_subsystem);
+        if (cgroup_path_relative_to_mount == nullptr)
             goto done;
 
-        cpu_cgroup_path = (char*)malloc(strlen(cpu_hierarchy_mount) + strlen(cpu_cgroup_path_relative_to_mount) + 1);
-        if (cpu_cgroup_path == nullptr)
+        cgroup_path = (char*)malloc(strlen(hierarchy_mount) + strlen(cgroup_path_relative_to_mount) + 1);
+        if (cgroup_path == nullptr)
            goto done;
 
-        strcpy(cpu_cgroup_path, cpu_hierarchy_mount);
-        strcat(cpu_cgroup_path, cpu_cgroup_path_relative_to_mount);
+        strcpy(cgroup_path, hierarchy_mount);
+        // For a host cgroup, we need to append the relative path.
+        // In a docker container, the root and relative path are the same and we don't need to append.
+        if (strcmp(hierarchy_root, cgroup_path_relative_to_mount) != 0)
+            strcat(cgroup_path, cgroup_path_relative_to_mount);
 
     done:
-        free(cpu_hierarchy_mount);
-        free(cpu_cgroup_path_relative_to_mount);
-        return cpu_cgroup_path;
+        free(hierarchy_mount);
+        free(hierarchy_root);
+        free(cgroup_path_relative_to_mount);
+        return cgroup_path;
     }
 
-    static char* FindHierarchyMount(bool (*is_subsystem)(const char *))
+    static void FindHierarchyMount(bool (*is_subsystem)(const char *), char** pmountpath, char** pmountroot)
     {
         char *line = nullptr;
         size_t lineLen = 0, maxLineLen = 0;
         char *filesystemType = nullptr;
         char *options = nullptr;
         char *mountpath = nullptr;
+        char *mountroot = nullptr;
 
         FILE *mountinfofile = fopen(PROC_MOUNTINFO_FILENAME, "r");
         if (mountinfofile == nullptr)
@@ -189,11 +169,11 @@ private:
                 maxLineLen = lineLen;
             }
 
-            char* separatorChar = strchr(line, '-');
+            char* separatorChar = strstr(line, " - ");
 
             // See man page of proc to get format for /proc/self/mountinfo file
             int sscanfRet = sscanf(separatorChar, 
-                                   "- %s %*s %s",
+                                   " - %s %*s %s",
                                    filesystemType,
                                    options);
             if (sscanfRet != 2)
@@ -213,16 +193,21 @@ private:
                         mountpath = (char*)malloc(lineLen+1);
                         if (mountpath == nullptr)
                             goto done;
+                        mountroot = (char*)malloc(lineLen+1);
+                        if (mountroot == nullptr)
+                            goto done;
     
                         sscanfRet = sscanf(line,
-                                           "%*s %*s %*s %*s %s ",
+                                           "%*s %*s %*s %s %s ",
+                                           mountroot,
                                            mountpath);
-                        if (sscanfRet != 1)
-                        {
-                            free(mountpath);
-                            mountpath = nullptr;
+                        if (sscanfRet != 2)
                             assert(!"Failed to parse mount info file contents with sscanf.");
-                        }
+
+                        // assign the output arguments and clear the locals so we don't free them.
+                        *pmountpath = mountpath;
+                        *pmountroot = mountroot;
+                        mountpath = mountroot = nullptr;
                         goto done;
                     }
                     strTok = strtok_r(nullptr, ",", &context);
@@ -230,12 +215,13 @@ private:
             }
         }
     done:
+        free(mountpath);
+        free(mountroot);
         free(filesystemType);
         free(options);
         free(line);
         if (mountinfofile)
             fclose(mountinfofile);
-        return mountpath;
     }
     
     static char* FindCGroupPathForSubsystem(bool (*is_subsystem)(const char *))
index 46d83842defaa05ffa8b883fd33dc159adb8ad63..ec0a0bd5fb2bdd7fdb703dc0a855ee9a0124132e 100644 (file)
@@ -31,8 +31,8 @@ class CGroup
 public:
     CGroup()
     {
-       m_memory_cgroup_path = FindMemoryCgroupPath();
-       m_cpu_cgroup_path = FindCpuCgroupPath();
+       m_memory_cgroup_path = FindCgroupPath(&IsMemorySubsystem);
+       m_cpu_cgroup_path = FindCgroupPath(&IsCpuSubsystem);
     }
 
     ~CGroup()
@@ -105,71 +105,48 @@ private:
         return strcmp("cpu", strTok) == 0;
     }
 
-    static char* FindMemoryCgroupPath(){
-        char *memory_cgroup_path = nullptr;
-        char *memory_hierarchy_mount = nullptr;
-        char *mem_cgroup_path_relative_to_mount = nullptr;
-        size_t len;
-
-        memory_hierarchy_mount = FindHierarchyMount(&IsMemorySubsystem);
-        if (memory_hierarchy_mount == nullptr)
-            goto done;
-
-        mem_cgroup_path_relative_to_mount = FindCGroupPathForSubsystem(&IsMemorySubsystem);
-        if (mem_cgroup_path_relative_to_mount == nullptr)
-            goto done;
-
-        len = strlen(memory_hierarchy_mount);
-        len += strlen(mem_cgroup_path_relative_to_mount);
-        memory_cgroup_path = (char*)PAL_malloc(len+1);
-        if (memory_cgroup_path == nullptr)
-           goto done;
-        
-        strcpy_s(memory_cgroup_path, len+1, memory_hierarchy_mount);
-        strcat_s(memory_cgroup_path, len+1, mem_cgroup_path_relative_to_mount);
-
-    done:
-        PAL_free(memory_hierarchy_mount);
-        PAL_free(mem_cgroup_path_relative_to_mount);
-        return memory_cgroup_path;
-    }
-
-    static char* FindCpuCgroupPath(){
-        char *cpu_cgroup_path = nullptr;
-        char *cpu_hierarchy_mount = nullptr;     
-        char *cpu_cgroup_path_relative_to_mount = nullptr;
+    static char* FindCgroupPath(bool (*is_subsystem)(const char *)){
+        char *cgroup_path = nullptr;
+        char *hierarchy_mount = nullptr;
+        char *hierarchy_root = nullptr;
+        char *cgroup_path_relative_to_mount = nullptr;
         size_t len;
 
-        cpu_hierarchy_mount = FindHierarchyMount(&IsCpuSubsystem);
-        if (cpu_hierarchy_mount == nullptr)
+        FindHierarchyMount(is_subsystem, &hierarchy_mount, &hierarchy_root);
+        if (hierarchy_mount == nullptr || hierarchy_root == nullptr)
             goto done;
 
-        cpu_cgroup_path_relative_to_mount = FindCGroupPathForSubsystem(&IsCpuSubsystem);
-        if (cpu_cgroup_path_relative_to_mount == nullptr)
+        cgroup_path_relative_to_mount = FindCGroupPathForSubsystem(is_subsystem);
+        if (cgroup_path_relative_to_mount == nullptr)
             goto done;
 
-        len = strlen(cpu_hierarchy_mount);
-        len += strlen(cpu_cgroup_path_relative_to_mount);
-        cpu_cgroup_path = (char*)PAL_malloc(len+1);
-        if (cpu_cgroup_path == nullptr)
+        len = strlen(hierarchy_mount);
+        len += strlen(cgroup_path_relative_to_mount);
+        cgroup_path = (char*)PAL_malloc(len+1);
+        if (cgroup_path == nullptr)
            goto done;
 
-        strcpy_s(cpu_cgroup_path, len+1, cpu_hierarchy_mount);
-        strcat_s(cpu_cgroup_path, len+1, cpu_cgroup_path_relative_to_mount);
+        strcpy_s(cgroup_path, len+1, hierarchy_mount);
+        // For a host cgroup, we need to append the relative path.
+        // In a docker container, the root and relative path are the same and we don't need to append.
+        if (strcmp(hierarchy_root, cgroup_path_relative_to_mount) != 0)
+            strcat_s(cgroup_path, len+1, cgroup_path_relative_to_mount);
 
     done:
-        PAL_free(cpu_hierarchy_mount);
-        PAL_free(cpu_cgroup_path_relative_to_mount);
-        return cpu_cgroup_path;
+        PAL_free(hierarchy_mount);
+        PAL_free(hierarchy_root);
+        PAL_free(cgroup_path_relative_to_mount);
+        return cgroup_path;
     }
 
-    static char* FindHierarchyMount(bool (*is_subsystem)(const char *))
+    static void FindHierarchyMount(bool (*is_subsystem)(const char *), char** pmountpath, char** pmountroot)
     {
         char *line = nullptr;
         size_t lineLen = 0, maxLineLen = 0;
         char *filesystemType = nullptr;
         char *options = nullptr;
         char *mountpath = nullptr;
+        char *mountroot = nullptr;
 
         FILE *mountinfofile = fopen(PROC_MOUNTINFO_FILENAME, "r");
         if (mountinfofile == nullptr)
@@ -189,11 +166,11 @@ private:
                     goto done;
                 maxLineLen = lineLen;
             }
-            char* separatorChar = strchr(line, '-');
+            char* separatorChar = strstr(line, " - ");;
 
             // See man page of proc to get format for /proc/self/mountinfo file
             int sscanfRet = sscanf_s(separatorChar, 
-                                     "- %s %*s %s",
+                                     " - %s %*s %s",
                                      filesystemType, lineLen+1,
                                      options, lineLen+1);
             if (sscanfRet != 2)
@@ -213,16 +190,21 @@ private:
                         mountpath = (char*)PAL_malloc(lineLen+1);
                         if (mountpath == nullptr)
                             goto done;
+                        mountroot = (char*)PAL_malloc(lineLen+1);
+                        if (mountroot == nullptr)
+                            goto done;
 
                         sscanfRet = sscanf_s(line,
-                                             "%*s %*s %*s %*s %s ",
+                                             "%*s %*s %*s %s %s ",
+                                             mountroot, lineLen+1,
                                              mountpath, lineLen+1);
-                        if (sscanfRet != 1)
-                        {
-                            PAL_free(mountpath);
-                            mountpath = nullptr;
+                        if (sscanfRet != 2)
                             _ASSERTE(!"Failed to parse mount info file contents with sscanf_s.");
-                        }
+
+                        // assign the output arguments and clear the locals so we don't free them.
+                        *pmountpath = mountpath;
+                        *pmountroot = mountroot;
+                        mountpath = mountroot = nullptr;
                         goto done;
                     }
                     strTok = strtok_s(nullptr, ",", &context);
@@ -230,12 +212,13 @@ private:
             }
         }
     done:
+        PAL_free(mountpath);
+        PAL_free(mountroot);
         PAL_free(filesystemType);
         PAL_free(options);
         free(line);
         if (mountinfofile)
             fclose(mountinfofile);
-        return mountpath;
     }
 
     static char* FindCGroupPathForSubsystem(bool (*is_subsystem)(const char *))