Skip to content

Commit

Permalink
libct/cg/v1: workaround CPU quota period set failure
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
kolyshkin committed Aug 26, 2021
1 parent 654f331 commit 1b2adcf
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 2 deletions.
24 changes: 22 additions & 2 deletions libcontainer/cgroups/fs/cpu.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@ package fs

import (
"bufio"
"errors"
"fmt"
"os"
"strconv"

"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{}
Expand Down Expand Up @@ -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)
}
Expand Down
9 changes: 9 additions & 0 deletions tests/integration/update.bats
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit 1b2adcf

Please sign in to comment.