From 6905b72154642539be0efed279796321ac50d558 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 26 Mar 2020 10:02:39 -0700 Subject: [PATCH] cgroupv2: use "max" for negative values Cgroup v1 kernel doc [1] says: > We can write "-1" to reset the ``*.limit_in_bytes(unlimited)``. and cgroup v2 kernel documentation [2] says: > - If a controller implements an absolute resource guarantee and/or > limit, the interface files should be named "min" and "max" > respectively. If a controller implements best effort resource > guarantee and/or limit, the interface files should be named "low" > and "high" respectively. > > In the above four control files, the special token "max" should be > used to represent upward infinity for both reading and writing. Allow -1 value to still be used for v2, converting it to "max" where it makes sense to do so. This fixes the following issue: > runc update test_update --memory-swap -1: > error while setting cgroup v2: [write /sys/fs/cgroup/machine.slice/runc-cgroups-integration-test.scope/memory.swap.max: invalid argument > failed to write "-1" to "/sys/fs/cgroup/machine.slice/runc-cgroups-integration-test.scope/memory.swap.max" > github.com/opencontainers/runc/libcontainer/cgroups/fscommon.WriteFile > /home/kir/go/src/github.com/opencontainers/runc/libcontainer/cgroups/fscommon/fscommon.go:21 > github.com/opencontainers/runc/libcontainer/cgroups/fs2.setMemory > /home/kir/go/src/github.com/opencontainers/runc/libcontainer/cgroups/fs2/memory.go:20 > github.com/opencontainers/runc/libcontainer/cgroups/fs2.(*manager).Set > /home/kir/go/src/github.com/opencontainers/runc/libcontainer/cgroups/fs2/fs2.go:175 > github.com/opencontainers/runc/libcontainer/cgroups/systemd.(*UnifiedManager).Set > /home/kir/go/src/github.com/opencontainers/runc/libcontainer/cgroups/systemd/unified_hierarchy.go:290 > github.com/opencontainers/runc/libcontainer.(*linuxContainer).Set > /home/kir/go/src/github.com/opencontainers/runc/libcontainer/container_linux.go:211 [1] linux/Documentation/admin-guide/cgroup-v1/memory.rst [2] linux/Documentation/admin-guide/cgroup-v2.rst Signed-off-by: Kir Kolyshkin --- libcontainer/cgroups/fs2/memory.go | 28 ++++++++++++++++++++++------ libcontainer/cgroups/fs2/pids.go | 12 ++---------- 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/libcontainer/cgroups/fs2/memory.go b/libcontainer/cgroups/fs2/memory.go index 23eccbec40d..f1db078613e 100644 --- a/libcontainer/cgroups/fs2/memory.go +++ b/libcontainer/cgroups/fs2/memory.go @@ -15,22 +15,38 @@ import ( "github.com/pkg/errors" ) +// numToStr converts an int64 value to a string for writing to a +// cgroupv2 files with .min, .max, .low, or .high suffix. +// Negative values are converted to "max" for cgroupv1 compatibility +// (which used to write -1 to remove the limit). +func numToStr(value int64) (ret string) { + if value > 0 { + ret = strconv.FormatInt(value, 10) + } else if value < 0 { + ret = "max" + } else { + ret = "" + } + + return ret +} + func setMemory(dirPath string, cgroup *configs.Cgroup) error { - if cgroup.Resources.MemorySwap != 0 { - if err := fscommon.WriteFile(dirPath, "memory.swap.max", strconv.FormatInt(cgroup.Resources.MemorySwap, 10)); err != nil { + if val := numToStr(cgroup.Resources.MemorySwap); val != "" { + if err := fscommon.WriteFile(dirPath, "memory.swap.max", val); err != nil { return err } } - if cgroup.Resources.Memory != 0 { - if err := fscommon.WriteFile(dirPath, "memory.max", strconv.FormatInt(cgroup.Resources.Memory, 10)); err != nil { + if val := numToStr(cgroup.Resources.Memory); val != "" { + if err := fscommon.WriteFile(dirPath, "memory.max", val); err != nil { return err } } // cgroup.Resources.KernelMemory is ignored - if cgroup.Resources.MemoryReservation != 0 { - if err := fscommon.WriteFile(dirPath, "memory.low", strconv.FormatInt(cgroup.Resources.MemoryReservation, 10)); err != nil { + if val := numToStr(cgroup.Resources.MemoryReservation); val != "" { + if err := fscommon.WriteFile(dirPath, "memory.low", val); err != nil { return err } } diff --git a/libcontainer/cgroups/fs2/pids.go b/libcontainer/cgroups/fs2/pids.go index db2d7ac90da..99d912cf2de 100644 --- a/libcontainer/cgroups/fs2/pids.go +++ b/libcontainer/cgroups/fs2/pids.go @@ -6,7 +6,6 @@ import ( "io/ioutil" "os" "path/filepath" - "strconv" "strings" "github.com/opencontainers/runc/libcontainer/cgroups" @@ -17,15 +16,8 @@ import ( ) func setPids(dirPath string, cgroup *configs.Cgroup) error { - if cgroup.Resources.PidsLimit != 0 { - // "max" is the fallback value. - limit := "max" - - if cgroup.Resources.PidsLimit > 0 { - limit = strconv.FormatInt(cgroup.Resources.PidsLimit, 10) - } - - if err := fscommon.WriteFile(dirPath, "pids.max", limit); err != nil { + if val := numToStr(cgroup.Resources.PidsLimit); val != "" { + if err := fscommon.WriteFile(dirPath, "pids.max", val); err != nil { return err } }