Repository: opencontainers/runc
Pull request #2507 (2020-07-07 04:02:57)

libct/cgroups/GetCgroupRoot: make it faster
...by checking the default path first.

Quick benchmark shows it's about 5x faster on an idle system, and the
gain should be much more on a system doing mounts etc.

This is an alternative to #2497. I don't like it much because it adds code rather than removes it.

kolyshkin: CI failure (in checkpoint --lazy-pages) looks unrelated, but the cascade of errors after it failed led me to PR #2509.
kolyshkin: CI restarted. The failure was
```
303612 checkpoint --lazy-pages and restore
3037# (from function `__runc' in file tests/integration/helpers.bash, line 57,
3038#  in test file tests/integration/checkpoint.bats, line 182)
3039#   `__runc --criu "$CRIU" restore -d --work-path ./image-dir --image-path ./image-dir --lazy-pages test_busybox_restore <&60 >&51 2>&51' failed
3040# runc list (status=0):
3041# ID          PID         STATUS      BUNDLE      CREATED     OWNER
3042# runc spec (status=0):
3043# 
3044# runc state test_busybox (status=0):
3045# {
3046#   "ociVersion": "1.0.2-dev",
3047#   "id": "test_busybox",
3048#   "pid": 3993,
3049#   "status": "running",
3050#   "bundle": "/tmp/busyboxtest",
3051#   "rootfs": "/tmp/busyboxtest/rootfs",
3052#   "created": "2020-07-07T04:09:18.881675684Z",
3053#   "owner": ""
3054# }
3055# Warn  (criu/kerndat.c:869): Can't keep kdat cache on non-tempfs
3056# runc list (status=0):
3057# ID             PID         STATUS      BUNDLE             CREATED                          OWNER
3058# test_busybox   3993        running     /tmp/busyboxtest   2020-07-07T04:09:18.881675684Z   root
3059# runc kill test_busybox KILL (status=0):
3060# 
3061# runc delete test_busybox (status=0):
3062# 
```

Patch:
diff --git a/libcontainer/cgroups/fs/fs.go b/libcontainer/cgroups/fs/fs.go
index 949fc01d..150442e0 100644
--- a/libcontainer/cgroups/fs/fs.go
+++ b/libcontainer/cgroups/fs/fs.go
@@ -68,6 +68,56 @@ func NewManager(cg *configs.Cgroup, paths map[string]string, rootless bool) cgro
 var cgroupRootLock sync.Mutex
 var cgroupRoot string
 
+const defaultCgroupRoot = "/sys/fs/cgroup"
+
+func tryDefaultCgroupRoot() string {
+	var st, pst unix.Stat_t
+
+	// (1) it should be a directory...
+	err := unix.Lstat(defaultCgroupRoot, &st)
+	if err != nil || st.Mode&unix.S_IFDIR == 0 {
+		return ""
+	}
+
+	// (2) ... and a mount point ...
+	err = unix.Lstat(filepath.Dir(defaultCgroupRoot), &pst)
+	if err != nil {
+		return ""
+	}
+
+	if st.Dev == pst.Dev {
+		// parent dir has the same dev -- not a mount point
+		return ""
+	}
+
+	// (3) ... of 'tmpfs' fs type.
+	var fst unix.Statfs_t
+	err = unix.Statfs(defaultCgroupRoot, &fst)
+	if err != nil || fst.Type != unix.TMPFS_MAGIC {
+		return ""
+	}
+
+	// (4) it should have at least 1 entry ...
+	dir, err := os.Open(defaultCgroupRoot)
+	if err != nil {
+		return ""
+	}
+	names, err := dir.Readdirnames(1)
+	if err != nil {
+		return ""
+	}
+	if len(names) < 1 {
+		return ""
+	}
+	// ... which is a cgroup mount point.
+	err = unix.Statfs(filepath.Join(defaultCgroupRoot, names[0]), &fst)
+	if err != nil || fst.Type != unix.CGROUP_SUPER_MAGIC {
+		return ""
+	}
+
+	return defaultCgroupRoot
+}
+
 // Gets the cgroupRoot.
 func getCgroupRoot() (string, error) {
 	cgroupRootLock.Lock()
@@ -77,6 +127,14 @@ func getCgroupRoot() (string, error) {
 		return cgroupRoot, nil
 	}
 
+	// fast path
+	cgroupRoot = tryDefaultCgroupRoot()
+	if cgroupRoot != "" {
+		return cgroupRoot, nil
+	}
+
+	// slow path: parse mountinfo, find the first mount where fs=cgroup
+	// (e.g. "/sys/fs/cgroup/memory"), use its parent.
 	f, err := os.Open("/proc/self/mountinfo")
 	if err != nil {
 		return "", err


Test patch:
diff --git a/libcontainer/cgroups/fs/fs_test.go b/libcontainer/cgroups/fs/fs_test.go
index 4971d547..2467dff0 100644
--- a/libcontainer/cgroups/fs/fs_test.go
+++ b/libcontainer/cgroups/fs/fs_test.go
@@ -295,3 +295,16 @@ func TestInvalidAbsoluteCgroupNameAndParent(t *testing.T) {
 		t.Errorf("SECURITY: cgroup path() is outside cgroup mountpoint!")
 	}
 }
+
+func TestTryDefaultCgroupRoot(t *testing.T) {
+	res := tryDefaultCgroupRoot()
+	exp := defaultCgroupRoot
+	if cgroups.IsCgroup2UnifiedMode() {
+		// checking that tryDefaultCgroupRoot does return ""
+		// in case /sys/fs/cgroup is not cgroup v1 root dir.
+		exp = ""
+	}
+	if res != exp {
+		t.Errorf("tryDefaultCgroupRoot: want %q, got %q", exp, res)
+	}
+}