From 4edf79780a8215d686e074371885716aa8e7b09b Mon Sep 17 00:00:00 2001
From: Kir Kolyshkin <kolyshkin@gmail.com>
Date: Wed, 8 Apr 2020 03:37:47 -0700
Subject: [PATCH] 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]
[v3: document neededControllers()]

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
---
 libcontainer/cgroups/fs2/cpu.go     |  8 +++
 libcontainer/cgroups/fs2/cpuset.go  |  8 +++
 libcontainer/cgroups/fs2/create.go  | 77 ++++++++++++++++++++++++-----
 libcontainer/cgroups/fs2/fs2.go     |  2 +-
 libcontainer/cgroups/fs2/hugetlb.go |  7 +++
 libcontainer/cgroups/fs2/io.go      | 12 +++++
 libcontainer/cgroups/fs2/memory.go  |  8 +++
 libcontainer/cgroups/fs2/pids.go    |  7 +++
 libcontainer/cgroups/systemd/v2.go  |  2 +-
 9 files changed, 118 insertions(+), 13 deletions(-)

diff --git a/libcontainer/cgroups/fs2/cpu.go b/libcontainer/cgroups/fs2/cpu.go
index 8ec855714f6..7129cf79c4d 100644
--- 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
index 6492ac93c58..fb4456b43c1 100644
--- 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
index 05407938d78..fa08e87cd18 100644
--- a/libcontainer/cgroups/fs2/create.go
+++ b/libcontainer/cgroups/fs2/create.go
@@ -1,29 +1,77 @@
 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)
+// neededControllers returns the string to write to cgroup.subtree_control,
+// containing the list of controllers to enable (for example, "+cpu +pids"),
+// based on (1) controllers available and (2) resources that are being set.
+//
+// The resulting string does not include "pseudo" controllers such as
+// "freezer" and "devices".
+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 +92,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
index f32f647d805..d845a1b5ebe 100644
--- 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
index c7610c0a414..4a399aaecd5 100644
--- 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
index 677c8d286cf..bbe3ac064b3 100644
--- 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
index 09a92d468b9..681bedd111e 100644
--- 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
index 0cf860f9c7c..b8153d28398 100644
--- 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
index 6cb09044567..42b2782d538 100644
--- 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