Add cgroup v2 support to CoreCLR #25906

omajid · 2019-07-26T21:32:23Z

Upstream cgroup v2 documentation is available at: https://www.kernel.org/doc/Documentation/cgroup-v2.txt

Some notable differences between cgroup v1 and v2, from a CoreCLR point of view, include:

cgroup v2 has a single hierarchy, so we just look for a single cgroup2 entry in /proc/self/mountinfo (without looking for a subsystem match).
Since cgroup v2 has a single hierarchy, /proc/self/cgroup generally has a single line 0::/path. There's no need to match subsystems or hierarchy ids here.
memory.limit_in_bytes is now memory.max. It can contain the literal max to indicate no limit.
memory.usage_in_bytes is now memory.current
cpu.cfs_quota_us and cpu.cfs_period_us have been combined into a single cpu.max file with the format $MAX $PERIOD. The max value can be a literal max to indicate a limit is not active.

~~AFAIK no container runtimes support cgroupv2 yet. I have tested this on a cgroup2 using host, outside of containers, only.~~ Tested on Fedora 31 with podman.

Fixes #25821

omajid · 2019-07-26T21:36:11Z

cc @tmds

omajid · 2019-07-30T21:53:33Z

@jerboaa (thanks!) pointed out that crun can be used to run containers with memory limits:

Linking the code below with coreclr lets me verify the memory limits, at least:

$ sudo podman run --runtime=/usr/bin/crun --volume $(pwd):/clrcg2 --memory=30M --rm -ti fedora:30 /clrcg2/test
initialized cgroup
31457280
1 724992
0 0
cleaned up cgroups

Unfortunately, cpus isn't supported by podman/crun with cgroup v2 yet.

Code:

#include <stddef.h>
#include <stdint.h>
#include <stdio.h>

#include "cgroup.h"

size_t GetRestrictedPhysicalMemoryLimit();
bool GetPhysicalMemoryUsed(size_t* used);
bool GetCpuLimit(uint32_t* cpu);

int main(int, char *[])
{
    InitializeCGroup();
    printf("initialized cgroup\n");

    size_t physical_memory_limit = GetRestrictedPhysicalMemoryLimit();
    printf("%lu\n", physical_memory_limit);

    size_t used_memory = 0;
    bool okay_memory_used = GetPhysicalMemoryUsed(&used_memory);
    printf("%d %lu\n", okay_memory_used, used_memory);

    uint32_t cpu_limit = 0;
    bool okay_cpu_limit = GetCpuLimit(&cpu_limit);
    printf("%d %u\n", okay_cpu_limit, cpu_limit);

    CleanupCGroup();
    printf("cleaned up cgroups\n");
    return 0;
}

jerboaa · 2019-08-16T09:48:10Z

@jerboaa (thanks!) pointed out that crun can be used to run containers with memory limits:
[...]
Unfortunately, cpus isn't supported by podman/crun with cgroup v2 yet.

That's not quite accurate. One can run with the cpu controller enabled on cgroups v2 only using podman and crun:

$ cat /sys/fs/cgroup/user.slice/cgroup.subtree_control
cpu io memory pids

$ podman --runtime=/usr/bin/crun run --memory=300M --cpu-quota=10000 --cpu-period=10000 -ti fedora:30 /bin/bash
[root@c6561b50c547 /]# cat /sys/fs/cgroup$(cat /proc/self/cgroup | cut -d':' -f3)/cpu.max
10000 10000

Or using --cpu-shares:

$ podman --runtime=/usr/bin/crun run --memory=300M --cpu-shares=1024 -ti fedora:30 /bin/bash
[root@e18f01fca65a /]# cat /sys/fs/cgroup$(cat /proc/self/cgroup | cut -d':' -f3)/cpu.weight
39

src/gc/unix/cgroup.cpp

Upstream cgroup v2 documentation is available at: https://www.kernel.org/doc/Documentation/cgroup-v2.txt Some notable differences between cgroup v1 and v2, from a CoreCLR point of view, include: - cgroup v2 has a single hierarchy, so we just look for a single "cgroup2" entry in /proc/self/mountinfo (without looking for a subsystem match). - Since cgroup v2 has a single hierarchy, /proc/self/cgroup generally has a single line "0::/path". There's no need to match subsystems or hierarchy ids here. - "memory.limit_in_bytes" is now "memory.max". It can contain the literal "max" to indicate no limit. - "memory.usage_in_bytes" is now "memory.current" - "cpu.cfs_quota_us" and "cpu.cfs_period_us" have been combined into a single "cpu.max" file with the format "$MAX $PERIOD". The max value can be a literal "max" to indicate a limit is not active.

omajid · 2019-10-31T03:06:39Z

Anyone willing to review this?

cc @Maoni0 @janvorli @jkotas

Maoni0 · 2019-10-31T03:15:02Z

cc @lpereira too

tmds · 2019-10-31T09:42:10Z

src/pal/src/misc/cgroup.cpp

@@ -211,11 +345,15 @@ class CGroup

            if (strncmp(filesystemType, "cgroup", 6) == 0)


Though the existing code doesn't support v2, it passes this condition.
Maybe we want to make this more strict?

if (strcmp(filesystemType, "cgroup") == 0 || strcmp(filesystemType, "cgroup2") == 0)

Since we're comparing with a literal using strncmp doesn't add much benefit over strcmp. Or maybe the compiler flags don't allow it...

tmds · 2019-10-31T09:44:22Z

src/pal/src/misc/cgroup.cpp

+                if (strncmp(filesystemType, "cgroup2", 8) == 0)
+                {
+                    *is_cgroupv2 = true;
+                }


Maybe make this assignment unconditional, so is_cgroupv2 is also assigned when filesystemType is "cgroup":
*is_cgroupv2 = strcmp(filesystemType, "cgroup2") == 0;

tmds · 2019-10-31T10:00:02Z

src/gc/unix/cgroup.cpp

 public:
    static void Initialize()
    {
-        s_memory_cgroup_path = FindCgroupPath(&IsMemorySubsystem);
-        s_cpu_cgroup_path = FindCgroupPath(&IsCpuSubsystem);
+        s_memory_cgroup_path = FindCGroupPath(&IsCGroup1MemorySubsystem);


Maybe we should change this so it stores the cgroup version in a static int s_cgroup_version?
Then we can switch on the static when the limits are retrieved?
like

static bool GetCpuLimit(uint32_t *val) { if (s_cgroup_version == 1) { return GetCGroup1CpuLimit(val); } else if (s_cgroup_version == 2) { return GetCGroup2CpuLimit(val); } return false; }

This would also allow to drop the SearchForFile function and pick the filename based on the s_cgroup_version.

lpereira

Thanks! Just a first pass here.

lpereira · 2019-10-31T16:45:23Z

src/gc/unix/cgroup.cpp

+        size_t filename_len = strlen(s_cpu_cgroup_path) + strlen(CGROUP2_CPU_MAX_FILENAME);
+        filename = (char*)malloc(filename_len + 1);
+        if (filename == nullptr)
+        {
+            goto done;
+        }
+
+        strcpy(filename, s_cpu_cgroup_path);
+        strcat(filename, CGROUP2_CPU_MAX_FILENAME);


You can use asprintf() here and make this much cleaner:

if (asprintf(&filename, "%s/%s", s_cpu_group_path, CGROUP2_CPU_MAX_FILENAME) < 0) goto done;

Alternatively, just declare a char[PATH_MAX] on the stack and use snprintf() instead, especially if this is called frequently.

lpereira · 2019-10-31T16:48:53Z

src/gc/unix/cgroup.cpp

+        max_quota_string = (char*) malloc(lineLen + 1);
+        if (max_quota_string == nullptr)
+        {
+            goto done;
+        }
+        period_string = (char*) malloc(lineLen + 1);
+        if (period_string == nullptr)
+        {
+            goto done;
+        }
+
+        sscanRet = sscanf(line, "%s %s", max_quota_string, period_string);
+        if (sscanRet != 2)
+        {
+            assert(!"Unable to parse " CGROUP2_CPU_MAX_FILENAME " file contents with sscanf.");
+            goto done;
+        }


This is dangerous: you're writing an unbounded amount of characters to both max_quota_string and period_string. Consider just slicing line instead by looking for ' ' with memchr() and writing a '\0' there. Saves two malloc() calls too:

char *space = memchr(line, ' ', lineLen); if (!space) goto done; max_quota_string = line; *space = '\0'; period_string = space + 1;

lpereira · 2019-10-31T16:50:24Z

src/gc/unix/cgroup.cpp

+        errno = 0;
+        max_quota = atoll(max_quota_string);
+        if (errno != 0)
+        {
+            goto done;
+        }
+
+        errno = 0;
+        period = atoll(period_string);
+        if (errno != 0)
+        {
+            goto done;
+        }


atoll() doesn't detect errors, so it's moot to set errno here as if you were to use strtol() instead. Prefer to use strtol(), even though you're reading a file from the system.

lpereira · 2019-10-31T16:51:34Z

src/gc/unix/cgroup.cpp

+        if (getline(&line, &lineLen, file) == -1)
+        {
+            goto done;
+        }


How long these lines can be? Might be worth considering using fgets() and allocating line in the stack instead to save a malloc().

I am not sure. The format of this line is $MAX $PERIOD. The values are in microseconds, and the first value allows the special case of a max literal. A default is max 10000. For more information, grep for cpu.max here. But there doesn't seem to be any explicit hard limits on the values here.

So it's probably safe to say that you need the space for two longs in base 10 plus a space?

Unfortunately you can't use std::numeric_limits here, but you can employ a trick to calculate an approximation of the buffer size based on the integer width: 3 * sizeof(integer_type) will give the size for an integer of type integer_type, with space for the terminating NUL, when converted to string in base 10 (it may be larger, but never smaller than needed).

So 1 /* for the space */ + 2 * sizeof(long) */ should be sufficient for a line in this file (and to avoid a malloc roundtrip).

lpereira · 2019-10-31T16:52:48Z

src/gc/unix/cgroup.cpp

            if (strncmp(filesystemType, "cgroup", 6) == 0)
            {
+                if (strncmp(filesystemType, "cgroup2", 8) == 0)


This will also match "cgroup2foo". Is this OK, future-wise?

lpereira · 2019-10-31T16:53:54Z

src/gc/unix/cgroup.cpp

+                int sscanfRet = sscanf(line,
+                                       "0::%s",
+                                       cgroup_path);


Don't use scanf() with "%s" conversion specifier. It's an unbounded read. There are ways to limit the number of bytes read from it with precision modifiers; the man page mentions how it works. (Or, alternatively, check if the string starts with "0::" and advance the pointer if it does. No need to copy the string.)

@omajid, @lpereira It is not a problem here. The string storage was allocated to be sufficient (size of the whole line we are scanning). The same for the comment in the sscanf below.

lpereira · 2019-10-31T16:54:04Z

src/gc/unix/cgroup.cpp

+                int sscanfRet = sscanf(line,
+                                       "%*[^:]:%[^:]:%s",
+                                       subsystem_list,
+                                       cgroup_path);


Good catch. But I only moved this code around. Is it better to fix it up now or do that cleanup in a separate change?

It's usually a good idea to leave the code better than you found it.

lpereira · 2019-10-31T16:55:29Z

src/pal/src/misc/cgroup.cpp

+        return strcmp("cpu", strTok) == 0;
+    }
+
+    static bool GetCGroup2CpuLimit(UINT *val)


This is very similar to GetCGroup1CpuLimit()... can't you use the same function for both, maybe passing a parameter, or reading s_cgroup_version inside to differentiate between versions?

lpereira · 2019-10-31T16:56:47Z

src/pal/src/misc/cgroup.cpp

+            char* full_filename = (char*)PAL_malloc(len+1);
+            if (full_filename == nullptr)
+            {
+                return nullptr;
+            }
+            strcpy_s(full_filename, len+1, search_root);
+            strcat_s(full_filename, len+1, possible_file);


Might be better to use snprintf() on a buffer declared with size PATH_MAX, and strdup() when found. Saves a roundtrip to malloc.

maryamariyan · 2019-11-06T21:04:27Z

Thank you for your contribution. As announced in dotnet/coreclr#27549 this repository will be moving to dotnet/runtime on November 13. If you would like to continue working on this PR after this date, the easiest way to move the change to dotnet/runtime is:

In your coreclr repository clone, create patch by running git format-patch origin
In your runtime repository clone, apply the patch by running git apply --directory src/coreclr <path to the patch created in step 1>

janvorli · 2019-11-12T16:07:51Z

@omajid are you planning to update this PR to reflect some of the feedback to which you've responded with thumbs up before we switch to the dotnet/runtime repo tomorrow?

omajid · 2019-11-12T16:12:14Z

@janvorli Hi! I definitely intend to update this PR and address all the feedback. Unfortunately, I dont think I can get it finished and tested by tomorrow. Is it okay if I create a new PR in dotnet/runtime with an updated fix based on the comments here?

janvorli · 2019-11-12T16:13:25Z

Is it okay if I create a new PR in dotnet/runtime with an updated fix based on the comments here?

Of course!

janvorli · 2019-11-12T16:13:56Z

I'm closing the PR in this repo.

omajid closed this Aug 15, 2019

omajid reopened this Aug 15, 2019

omajid closed this Oct 10, 2019

omajid reopened this Oct 10, 2019

tmds mentioned this pull request Oct 24, 2019

Add cgroup v2 support to Interop.cgroups dotnet/corefx#39686

Merged

omajid commented Oct 28, 2019

View reviewed changes

src/gc/unix/cgroup.cpp Outdated Show resolved Hide resolved

omajid force-pushed the crgoupv2-initial branch from 79db113 to baeb379 Compare October 28, 2019 21:37

omajid closed this Oct 30, 2019

omajid reopened this Oct 30, 2019

tmds reviewed Oct 31, 2019

View reviewed changes

lpereira suggested changes Oct 31, 2019

View reviewed changes

jkotas added the area-PAL label Nov 2, 2019

janvorli closed this Nov 12, 2019

omajid mentioned this pull request Feb 1, 2020

Add cgroup v2 support to .NET Core dotnet/runtime#30337

Closed

4 tasks

omajid mentioned this pull request Mar 31, 2020

Add cgroup v2 support to coreclr dotnet/runtime#34334

Merged

richlander mentioned this pull request Nov 19, 2022

Update samples for user and cgroup info dotnet/dotnet-docker#4220

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add cgroup v2 support to CoreCLR #25906

Add cgroup v2 support to CoreCLR #25906

omajid commented Jul 26, 2019 •

edited

Loading

omajid commented Jul 26, 2019

omajid commented Jul 30, 2019

jerboaa commented Aug 16, 2019

omajid commented Oct 31, 2019

Maoni0 commented Oct 31, 2019

tmds Oct 31, 2019 •

edited

Loading

tmds Oct 31, 2019

tmds Oct 31, 2019

tmds Oct 31, 2019 •

edited

Loading

lpereira left a comment

lpereira Oct 31, 2019

lpereira Oct 31, 2019 •

edited

Loading

lpereira Oct 31, 2019

lpereira Oct 31, 2019

omajid Nov 4, 2019 •

edited

Loading

lpereira Nov 4, 2019

lpereira Oct 31, 2019

lpereira Oct 31, 2019 •

edited

Loading

janvorli Dec 18, 2019

lpereira Oct 31, 2019

omajid Nov 4, 2019

lpereira Nov 4, 2019

lpereira Oct 31, 2019 •

edited

Loading

lpereira Oct 31, 2019

maryamariyan commented Nov 6, 2019

janvorli commented Nov 12, 2019

omajid commented Nov 12, 2019

janvorli commented Nov 12, 2019

janvorli commented Nov 12, 2019

		@@ -211,11 +345,15 @@ class CGroup

		if (strncmp(filesystemType, "cgroup", 6) == 0)

Add cgroup v2 support to CoreCLR #25906

Add cgroup v2 support to CoreCLR #25906

Conversation

omajid commented Jul 26, 2019 • edited Loading

omajid commented Jul 26, 2019

omajid commented Jul 30, 2019

jerboaa commented Aug 16, 2019

omajid commented Oct 31, 2019

Maoni0 commented Oct 31, 2019

tmds Oct 31, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tmds Oct 31, 2019 • edited Loading

Choose a reason for hiding this comment

lpereira left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lpereira Oct 31, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

omajid Nov 4, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lpereira Oct 31, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lpereira Oct 31, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maryamariyan commented Nov 6, 2019

janvorli commented Nov 12, 2019

omajid commented Nov 12, 2019

janvorli commented Nov 12, 2019

janvorli commented Nov 12, 2019

omajid commented Jul 26, 2019 •

edited

Loading

tmds Oct 31, 2019 •

edited

Loading

tmds Oct 31, 2019 •

edited

Loading

lpereira Oct 31, 2019 •

edited

Loading

omajid Nov 4, 2019 •

edited

Loading

lpereira Oct 31, 2019 •

edited

Loading

lpereira Oct 31, 2019 •

edited

Loading