From 1b2adcfe5620cac8ce3e52ea066161c72431f74c Mon Sep 17 00:00:00 2001
From: Kir Kolyshkin <kolyshkin@gmail.com>
Date: Wed, 28 Jul 2021 10:34:26 -0700
Subject: [PATCH] libct/cg/v1: workaround CPU quota period set failure

As reported in issue 3084, 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.

Add a test case to cover a regression caused by an earlier version of
this patch (ignoring a failure of setting invalid period when quota is
not set).

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
---
 libcontainer/cgroups/fs/cpu.go | 24 ++++++++++++++++++++++--
 tests/integration/update.bats  |  9 +++++++++
 2 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/libcontainer/cgroups/fs/cpu.go b/libcontainer/cgroups/fs/cpu.go
index 04bd5bc9f..657570a27 100644
--- a/libcontainer/cgroups/fs/cpu.go
+++ b/libcontainer/cgroups/fs/cpu.go
@@ -4,6 +4,7 @@ package fs
 
 import (
 	"bufio"
+	"errors"
 	"fmt"
 	"os"
 	"strconv"
@@ -11,6 +12,7 @@ import (
 	"github.com/opencontainers/runc/libcontainer/cgroups"
 	"github.com/opencontainers/runc/libcontainer/cgroups/fscommon"
 	"github.com/opencontainers/runc/libcontainer/configs"
+	"golang.org/x/sys/unix"
 )
 
 type CpuGroup struct{}
@@ -71,15 +73,33 @@ func (s *CpuGroup) Set(path string, r *configs.Resources) error {
 			return fmt.Errorf("the minimum allowed cpu-shares is %d", sharesRead)
 		}
 	}
+
+	var period string
 	if r.CpuPeriod != 0 {
-		if err := cgroups.WriteFile(path, "cpu.cfs_period_us", strconv.FormatUint(r.CpuPeriod, 10)); err != nil {
-			return err
+		period = strconv.FormatUint(r.CpuPeriod, 10)
+		if err := cgroups.WriteFile(path, "cpu.cfs_period_us", period); err != nil {
+			// 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.
+			if !errors.Is(err, unix.EINVAL) || r.CpuQuota == 0 {
+				return err
+			}
+		} else {
+			period = ""
 		}
 	}
 	if r.CpuQuota != 0 {
 		if err := cgroups.WriteFile(path, "cpu.cfs_quota_us", strconv.FormatInt(r.CpuQuota, 10)); err != nil {
 			return err
 		}
+		if period != "" {
+			if err := cgroups.WriteFile(path, "cpu.cfs_period_us", period); err != nil {
+				return err
+			}
+		}
 	}
 	return s.SetRtSched(path, r)
 }
diff --git a/tests/integration/update.bats b/tests/integration/update.bats
index 2004d9efc..b9a5b602c 100644
--- a/tests/integration/update.bats
+++ b/tests/integration/update.bats
@@ -345,6 +345,15 @@ EOF
 	check_cpu_quota -1 1000000 "infinity"
 }
 
+@test "set cpu period with no quota (invalid period)" {
+	[[ "$ROOTLESS" -ne 0 ]] && requires rootless_cgroup
+
+	update_config '.linux.resources.cpu |= { "period": 100 }'
+
+	runc run -d --console-socket "$CONSOLE_SOCKET" test_update
+	[ "$status" -eq 1 ]
+}
+
 @test "set cpu quota with no period" {
 	[[ "$ROOTLESS" -ne 0 ]] && requires rootless_cgroup