cgroups/fs2: don't always parse /proc/self/cgroup

Function defaultPath always parses /proc/self/cgroup, but
the resulting value is not always used.

Avoid unnecessary reading/parsing by moving the code
to just before its use.

Modify the test case accordingly.

[v2: test: use UnifiedMountpoint, skip test if not on v2]

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This commit is contained in:
Kir Kolyshkin 2020-04-27 14:57:08 -07:00
parent 051d6705a7
commit c3b0b13fe9
2 changed files with 36 additions and 24 deletions

View File

@ -44,19 +44,10 @@ func defaultDirPath(c *configs.Cgroup) (string, error) {
cgParent := libcontainerUtils.CleanPath(c.Parent) cgParent := libcontainerUtils.CleanPath(c.Parent)
cgName := libcontainerUtils.CleanPath(c.Name) cgName := libcontainerUtils.CleanPath(c.Name)
ownCgroup, err := parseCgroupFile("/proc/self/cgroup") return _defaultDirPath(UnifiedMountpoint, cgPath, cgParent, cgName)
if err != nil {
return "", err
}
// The current user scope most probably has tasks in it already,
// making it impossible to enable controllers for its sub-cgroup.
// A parent cgroup (with no tasks in it) is what we need.
ownCgroup = filepath.Dir(ownCgroup)
return _defaultDirPath(UnifiedMountpoint, cgPath, cgParent, cgName, ownCgroup)
} }
func _defaultDirPath(root, cgPath, cgParent, cgName, ownCgroup string) (string, error) { func _defaultDirPath(root, cgPath, cgParent, cgName string) (string, error) {
if (cgName != "" || cgParent != "") && cgPath != "" { if (cgName != "" || cgParent != "") && cgPath != "" {
return "", errors.New("cgroup: either Path or Name and Parent should be used") return "", errors.New("cgroup: either Path or Name and Parent should be used")
} }
@ -67,6 +58,16 @@ func _defaultDirPath(root, cgPath, cgParent, cgName, ownCgroup string) (string,
if filepath.IsAbs(innerPath) { if filepath.IsAbs(innerPath) {
return filepath.Join(root, innerPath), nil return filepath.Join(root, innerPath), nil
} }
ownCgroup, err := parseCgroupFile("/proc/self/cgroup")
if err != nil {
return "", err
}
// The current user scope most probably has tasks in it already,
// making it impossible to enable controllers for its sub-cgroup.
// A parent cgroup (with no tasks in it) is what we need.
ownCgroup = filepath.Dir(ownCgroup)
return filepath.Join(root, ownCgroup, innerPath), nil return filepath.Join(root, ownCgroup, innerPath), nil
} }

View File

@ -17,8 +17,11 @@
package fs2 package fs2
import ( import (
"path/filepath"
"strings" "strings"
"testing" "testing"
"github.com/opencontainers/runc/libcontainer/cgroups"
) )
func TestParseCgroupFromReader(t *testing.T) { func TestParseCgroupFromReader(t *testing.T) {
@ -45,27 +48,35 @@ func TestParseCgroupFromReader(t *testing.T) {
} }
func TestDefaultDirPath(t *testing.T) { func TestDefaultDirPath(t *testing.T) {
root := "/sys/fs/cgroup" if !cgroups.IsCgroup2UnifiedMode() {
t.Skip("need cgroupv2")
}
// same code as in defaultDirPath()
ownCgroup, err := parseCgroupFile("/proc/self/cgroup")
if err != nil {
// Not a test failure, but rather some weird
// environment so we can't run this test.
t.Skipf("can't get own cgroup: %v", err)
}
ownCgroup = filepath.Dir(ownCgroup)
cases := []struct { cases := []struct {
cgPath string cgPath string
cgParent string cgParent string
cgName string cgName string
ownCgroup string expected string
expected string
}{ }{
{ {
cgPath: "/foo/bar", cgPath: "/foo/bar",
ownCgroup: "/apple/banana", expected: "/sys/fs/cgroup/foo/bar",
expected: "/sys/fs/cgroup/foo/bar",
}, },
{ {
cgPath: "foo/bar", cgPath: "foo/bar",
ownCgroup: "/apple/banana", expected: filepath.Join(UnifiedMountpoint, ownCgroup, "foo/bar"),
expected: "/sys/fs/cgroup/apple/banana/foo/bar",
}, },
} }
for _, c := range cases { for _, c := range cases {
got, err := _defaultDirPath(root, c.cgPath, c.cgParent, c.cgName, c.ownCgroup) got, err := _defaultDirPath(UnifiedMountpoint, c.cgPath, c.cgParent, c.cgName)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }