CreateCgroupPath: only enable needed controllers

... on a best-effort basis. 1. Instead of enabling all available controllers, figure out which ones are required, and only enable those. 2. Do not return an error early if we can't enable some controllers. 3. If we fail to enable all controllers at once (usually because one of them can't be enabled), try enabling them one by one. [v2: add/use is${Controller}Set() functions] Signed-off-by: Kir Kolyshkin <[email protected]>
opencontainers · Apr 8, 2020 · 8bbe881 · 8bbe881
1 parent 430d891
commit 8bbe881
Show file tree

Hide file tree

Showing 9 changed files with 112 additions and 13 deletions.
diff --git a/libcontainer/cgroups/fs2/cpu.go b/libcontainer/cgroups/fs2/cpu.go
@@ -13,7 +13,15 @@ import (
 	"github.com/opencontainers/runc/libcontainer/configs"
 )
 
+func isCpuSet(cgroup *configs.Cgroup) bool {
+	return cgroup.Resources.CpuWeight != 0 || cgroup.Resources.CpuMax != ""
+}
+
 func setCpu(dirPath string, cgroup *configs.Cgroup) error {
+	if !isCpuSet(cgroup) {
+		return nil
+	}
+
 	// NOTE: .CpuShares is not used here. Conversion is the caller's responsibility.
 	if cgroup.Resources.CpuWeight != 0 {
 		if err := fscommon.WriteFile(dirPath, "cpu.weight", strconv.FormatUint(cgroup.Resources.CpuWeight, 10)); err != nil {

diff --git a/libcontainer/cgroups/fs2/cpuset.go b/libcontainer/cgroups/fs2/cpuset.go
@@ -7,7 +7,15 @@ import (
 	"github.com/opencontainers/runc/libcontainer/configs"
 )
 
+func isCpusetSet(cgroup *configs.Cgroup) bool {
+	return cgroup.Resources.CpusetCpus != "" || cgroup.Resources.CpusetMems != ""
+}
+
 func setCpuset(dirPath string, cgroup *configs.Cgroup) error {
+	if !isCpusetSet(cgroup) {
+		return nil
+	}
+
 	if cgroup.Resources.CpusetCpus != "" {
 		if err := fscommon.WriteFile(dirPath, "cpuset.cpus", cgroup.Resources.CpusetCpus); err != nil {
 			return err

diff --git a/libcontainer/cgroups/fs2/create.go b/libcontainer/cgroups/fs2/create.go
@@ -1,29 +1,71 @@
 package fs2
 
 import (
-	"bytes"
 	"fmt"
 	"io/ioutil"
 	"os"
 	"path/filepath"
 	"strings"
+
+	"github.com/opencontainers/runc/libcontainer/configs"
 )
 
-// CreateCgroupPath creates cgroupv2 path, enabling all the
-// available controllers in the process.
-func CreateCgroupPath(path string) (Err error) {
-	if !strings.HasPrefix(path, UnifiedMountpoint) {
-		return fmt.Errorf("invalid cgroup path %s", path)
+func neededControllers(cgroup *configs.Cgroup) []string {
+	var list []string
+
+	if cgroup == nil {
+		return list
 	}
 
+	// list of all available controllers
 	const file = UnifiedMountpoint + "/cgroup.controllers"
 	content, err := ioutil.ReadFile(file)
 	if err != nil {
-		return err
+		return list
+	}
+	avail := make(map[string]struct{})
+	for _, ctr := range strings.Split(string(content), " ") {
+		avail[ctr] = struct{}{}
 	}
 
-	ctrs := bytes.Fields(content)
-	res := append([]byte("+"), bytes.Join(ctrs, []byte(" +"))...)
+	// add the controller if available
+	add := func(controller string) {
+		if _, ok := avail[controller]; ok {
+			list = append(list, "+"+controller)
+		}
+	}
+
+	if isPidsSet(cgroup) {
+		add("pids")
+	}
+	if isMemorySet(cgroup) {
+		add("memory")
+	}
+	if isIoSet(cgroup) {
+		add("io")
+	}
+	if isCpuSet(cgroup) {
+		add("cpu")
+	}
+	if isCpusetSet(cgroup) {
+		add("cpuset")
+	}
+	if isHugeTlbSet(cgroup) {
+		add("hugetlb")
+	}
+
+	return list
+}
+
+// CreateCgroupPath creates cgroupv2 path, enabling all the
+// needed controllers in the process.
+func CreateCgroupPath(path string, c *configs.Cgroup) (Err error) {
+	if !strings.HasPrefix(path, UnifiedMountpoint) {
+		return fmt.Errorf("invalid cgroup path %s", path)
+	}
+
+	ctrs := neededControllers(c)
+	allCtrs := strings.Join(ctrs, " ")
 
 	elements := strings.Split(path, "/")
 	elements = elements[3:]
@@ -44,11 +86,18 @@ func CreateCgroupPath(path string) (Err error) {
 				}()
 			}
 		}
+		// enable needed controllers
 		if i < len(elements)-1 {
-			if err := ioutil.WriteFile(filepath.Join(current, "cgroup.subtree_control"), res, 0755); err != nil {
-				return err
+			file := filepath.Join(current, "cgroup.subtree_control")
+			if err := ioutil.WriteFile(file, []byte(allCtrs), 0755); err != nil {
+				// failed to enable all controllers at once, so let's try
+				// one by one, ignoring errors as there's nothing we can do
+				for _, ctr := range ctrs {
+					_ = ioutil.WriteFile(file, []byte(ctr), 0755)
+				}
 			}
 		}
 	}
+
 	return nil
 }
diff --git a/libcontainer/cgroups/fs2/fs2.go b/libcontainer/cgroups/fs2/fs2.go
@@ -66,7 +66,7 @@ func (m *manager) getControllers() error {
 }
 
 func (m *manager) Apply(pid int) error {
-	if err := CreateCgroupPath(m.dirPath); err != nil {
+	if err := CreateCgroupPath(m.dirPath, m.config); err != nil {
 		return err
 	}
 	if err := cgroups.WriteCgroupProc(m.dirPath, pid); err != nil && !m.rootless {

diff --git a/libcontainer/cgroups/fs2/hugetlb.go b/libcontainer/cgroups/fs2/hugetlb.go
@@ -15,7 +15,14 @@ import (
 	"github.com/opencontainers/runc/libcontainer/configs"
 )
 
+func isHugeTlbSet(cgroup *configs.Cgroup) bool {
+	return len(cgroup.Resources.HugetlbLimit) > 0
+}
+
 func setHugeTlb(dirPath string, cgroup *configs.Cgroup) error {
+	if !isHugeTlbSet(cgroup) {
+		return nil
+	}
 	for _, hugetlb := range cgroup.Resources.HugetlbLimit {
 		if err := fscommon.WriteFile(dirPath, strings.Join([]string{"hugetlb", hugetlb.Pagesize, "max"}, "."), strconv.FormatUint(hugetlb.Limit, 10)); err != nil {
 			return err

diff --git a/libcontainer/cgroups/fs2/io.go b/libcontainer/cgroups/fs2/io.go
@@ -14,7 +14,19 @@ import (
 	"github.com/opencontainers/runc/libcontainer/configs"
 )
 
+func isIoSet(cgroup *configs.Cgroup) bool {
+	return cgroup.Resources.BlkioWeight != 0 ||
+		len(cgroup.Resources.BlkioThrottleReadBpsDevice) > 0 ||
+		len(cgroup.Resources.BlkioThrottleWriteBpsDevice) > 0 ||
+		len(cgroup.Resources.BlkioThrottleReadIOPSDevice) > 0 ||
+		len(cgroup.Resources.BlkioThrottleWriteIOPSDevice) > 0
+}
+
 func setIo(dirPath string, cgroup *configs.Cgroup) error {
+	if !isIoSet(cgroup) {
+		return nil
+	}
+
 	if cgroup.Resources.BlkioWeight != 0 {
 		filename := "io.bfq.weight"
 		if err := fscommon.WriteFile(dirPath, filename,

diff --git a/libcontainer/cgroups/fs2/memory.go b/libcontainer/cgroups/fs2/memory.go
@@ -32,7 +32,15 @@ func numToStr(value int64) (ret string) {
 	return ret
 }
 
+func isMemorySet(cgroup *configs.Cgroup) bool {
+	return cgroup.Resources.MemoryReservation != 0 ||
+		cgroup.Resources.Memory != 0 || cgroup.Resources.MemorySwap != 0
+}
+
 func setMemory(dirPath string, cgroup *configs.Cgroup) error {
+	if !isMemorySet(cgroup) {
+		return nil
+	}
 	swap, err := cgroups.ConvertMemorySwapToCgroupV2Value(cgroup.Resources.MemorySwap, cgroup.Resources.Memory)
 	if err != nil {
 		return err

diff --git a/libcontainer/cgroups/fs2/pids.go b/libcontainer/cgroups/fs2/pids.go
@@ -14,7 +14,14 @@ import (
 	"golang.org/x/sys/unix"
 )
 
+func isPidsSet(cgroup *configs.Cgroup) bool {
+	return cgroup.Resources.PidsLimit != 0
+}
+
 func setPids(dirPath string, cgroup *configs.Cgroup) error {
+	if !isPidsSet(cgroup) {
+		return nil
+	}
 	if val := numToStr(cgroup.Resources.PidsLimit); val != "" {
 		if err := fscommon.WriteFile(dirPath, "pids.max", val); err != nil {
 			return err

diff --git a/libcontainer/cgroups/systemd/v2.go b/libcontainer/cgroups/systemd/v2.go
@@ -151,7 +151,7 @@ func (m *unifiedManager) Apply(pid int) error {
 	if err != nil {
 		return err
 	}
-	if err := fs2.CreateCgroupPath(m.path); err != nil {
+	if err := fs2.CreateCgroupPath(m.path, m.cgroups); err != nil {
 		return err
 	}
 	return nil