Skip to content

Commit

Permalink
cgroup: workaround cpu quota/period issue with v1
Browse files Browse the repository at this point in the history
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
opencontainers/runc#3084

Signed-off-by: Kir Kolyshkin <[email protected]>
  • Loading branch information
kolyshkin committed Apr 6, 2023
1 parent 2fb17d0 commit 1b71c08
Showing 1 changed file with 19 additions and 2 deletions.
21 changes: 19 additions & 2 deletions src/libcrun/cgroup-resources.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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)
Expand All @@ -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)
Expand Down

0 comments on commit 1b71c08

Please sign in to comment.