From 1b71c0862413c529c24059f2ce425c5674956a50 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 6 Apr 2023 13:20:04 -0700 Subject: [PATCH] cgroup: workaround cpu quota/period issue with v1 Sometimes setting CPU quota period fails when a new period is lower, and a parent cgroup has CPU quota limit set. This happens as in cgroup v1 the quota and the period can not be set together (this is fixed in v2), and since the period is being set first, new_limit = old_quota/new_period may be higher than the parent cgroup limit. The fix is to retry setting the period after the quota, to cover all possible scenarios. Tested via runc integration tests. Before the commit, it fails: root@ubu2004:~/git/runc# RUNC=`pwd`/../crun/crun.before bats -f "pod cgroup" -t tests/integration/update.bats 1..1 not ok 1 update cpu period in a pod cgroup with pod limit set # (in test file tests/integration/update.bats, line 424) # `[ "$status" -eq 0 ]' failed # crun.before spec (status=0): # # crun.before run -d --console-socket /tmp/bats-run-30428-dYkMDC/runc.4FdCtn/tty/sock test_update (status=0): # # crun.before update --cpu-quota 600000 test_update (status=1): # writing file `cpu.cfs_quota_us`: Invalid argument # crun.before update --cpu-period 10000 --cpu-quota 3000 test_update (status=1): # writing file `cpu.cfs_period_us`: Invalid argument With the fix, the test passes. Originally reported for runc in https://github.com/opencontainers/runc/issues/3084 Signed-off-by: Kir Kolyshkin --- src/libcrun/cgroup-resources.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/src/libcrun/cgroup-resources.c b/src/libcrun/cgroup-resources.c index 1a69a0e53b..b554dc3e03 100644 --- a/src/libcrun/cgroup-resources.c +++ b/src/libcrun/cgroup-resources.c @@ -908,11 +908,12 @@ static int write_cpu_resources (int dirfd_cpu, bool cgroup2, runtime_spec_schema_config_linux_resources_cpu *cpu, libcrun_error_t *err) { - size_t len; + size_t len, period_len; int ret; char fmt_buf[64]; int64_t period = -1; int64_t quota = -1; + cleanup_free char *period_str = NULL; if (cpu->shares) { @@ -937,7 +938,17 @@ write_cpu_resources (int dirfd_cpu, bool cgroup2, runtime_spec_schema_config_lin len = sprintf (fmt_buf, "%" PRIu64, cpu->period); ret = write_cgroup_file (dirfd_cpu, "cpu.cfs_period_us", fmt_buf, len, err); if (UNLIKELY (ret < 0)) - return ret; + { + if (errno != EINVAL) + return ret; + /* Sometimes when the period to be set is smaller than the current one, + * it is rejected by the kernel (EINVAL) as old_quota/new_period exceeds + * the parent cgroup quota limit. If this happens and the quota is going + * to be set, ignore the error for now and retry after setting the quota. + */ + period_str = xstrdup (fmt_buf); + period_len = len; + } } } if (cpu->quota) @@ -950,6 +961,12 @@ write_cpu_resources (int dirfd_cpu, bool cgroup2, runtime_spec_schema_config_lin ret = write_cgroup_file (dirfd_cpu, "cpu.cfs_quota_us", fmt_buf, len, err); if (UNLIKELY (ret < 0)) return ret; + if (period_str != NULL) + { + ret = write_cgroup_file (dirfd_cpu, "cpu.cfs_period_us", period_str, period_len, err); + if (UNLIKELY (ret < 0)) + return ret; + } } } if (cpu->realtime_period)