Skip to content

Commit

Permalink
merge branch 'pr-2599'
Browse files Browse the repository at this point in the history 
Kir Kolyshkin (4):
  libct/cgroups/fs/cpuset: don't use MkdirAll
  libct/cg/fs/cpuset: don't parse mountinfo
  libct/cg/fs.getCgroupRoot: reuse (cached) cgroup mountinfo
  libct/cgroups/v1_utils: implement mountinfo cache

LGTMs: @AkihiroSuda @cyphar
Closes #2599
  • Loading branch information
@cyphar
cyphar committed Feb 1, 2021
2 parents c531a6f + 9f2153c commit 6eed6e5
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 147 deletions.
61 changes: 19 additions & 42 deletions libcontainer/cgroups/fs/cpuset.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,10 @@ import (
"strconv"
"strings"

"github.com/moby/sys/mountinfo"
"github.com/opencontainers/runc/libcontainer/cgroups"
"github.com/opencontainers/runc/libcontainer/cgroups/fscommon"
"github.com/opencontainers/runc/libcontainer/configs"
libcontainerUtils "github.com/opencontainers/runc/libcontainer/utils"
"github.com/pkg/errors"
"golang.org/x/sys/unix"
)

type CpusetGroup struct {
Expand Down Expand Up @@ -145,46 +143,19 @@ func (s *CpusetGroup) GetStats(path string, stats *cgroups.Stats) error {
return nil
}

// Get the source mount point of directory passed in as argument.
func getMount(dir string) (string, error) {
mi, err := mountinfo.GetMounts(mountinfo.ParentsFilter(dir))
if err != nil {
return "", err
}
if len(mi) < 1 {
return "", errors.Errorf("Can't find mount point of %s", dir)
}

// find the longest mount point
var idx, maxlen int
for i := range mi {
if len(mi[i].Mountpoint) > maxlen {
maxlen = len(mi[i].Mountpoint)
idx = i
}
}

return mi[idx].Mountpoint, nil
}

func (s *CpusetGroup) ApplyDir(dir string, cgroup *configs.Cgroup, pid int) error {
// This might happen if we have no cpuset cgroup mounted.
// Just do nothing and don't fail.
if dir == "" {
return nil
}
root, err := getMount(dir)
if err != nil {
return err
}
root = filepath.Dir(root)
// 'ensureParent' start with parent because we don't want to
// explicitly inherit from parent, it could conflict with
// 'cpuset.cpu_exclusive'.
if err := cpusetEnsureParent(filepath.Dir(dir), root); err != nil {
if err := cpusetEnsureParent(filepath.Dir(dir)); err != nil {
return err
}
if err := os.MkdirAll(dir, 0755); err != nil {
if err := os.Mkdir(dir, 0755); err != nil && !os.IsExist(err) {
return err
}
// We didn't inherit cpuset configs from parent, but we have
Expand Down Expand Up @@ -213,22 +184,28 @@ func getCpusetSubsystemSettings(parent string) (cpus, mems string, err error) {
return cpus, mems, nil
}

// cpusetEnsureParent makes sure that the parent directory of current is created
// and populated with the proper cpus and mems files copied from
// its parent.
func cpusetEnsureParent(current, root string) error {
// cpusetEnsureParent makes sure that the parent directories of current
// are created and populated with the proper cpus and mems files copied
// from their respective parent. It does that recursively, starting from
// the top of the cpuset hierarchy (i.e. cpuset cgroup mount point).
func cpusetEnsureParent(current string) error {
var st unix.Statfs_t

parent := filepath.Dir(current)
if libcontainerUtils.CleanPath(parent) == root {
err := unix.Statfs(parent, &st)
if err == nil && st.Type != unix.CGROUP_SUPER_MAGIC {
return nil
}
// Avoid infinite recursion.
if parent == current {
return errors.New("cpuset: cgroup parent path outside cgroup root")
// Treat non-existing directory as cgroupfs as it will be created,
// and the root cpuset directory obviously exists.
if err != nil && err != unix.ENOENT {
return &os.PathError{Op: "statfs", Path: parent, Err: err}
}
if err := cpusetEnsureParent(parent, root); err != nil {

if err := cpusetEnsureParent(parent); err != nil {
return err
}
if err := os.MkdirAll(current, 0755); err != nil {
if err := os.Mkdir(current, 0755); err != nil && !os.IsExist(err) {
return err
}
return cpusetCopyIfNeeded(current, parent)
Expand Down
43 changes: 7 additions & 36 deletions libcontainer/cgroups/fs/fs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,9 @@
package fs

import (
"bufio"
"fmt"
"os"
"path/filepath"
"strings"
"sync"

"github.com/opencontainers/runc/libcontainer/cgroups"
Expand Down Expand Up @@ -133,46 +131,19 @@ func getCgroupRoot() (string, error) {
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")
// slow path: parse mountinfo
mi, err := cgroups.GetCgroupMounts(false)
if err != nil {
return "", err
}
defer f.Close()

var root string
scanner := bufio.NewScanner(f)
for scanner.Scan() {
text := scanner.Text()
fields := strings.Split(text, " ")
// Safe as mountinfo encodes mountpoints with spaces as \040.
index := strings.Index(text, " - ")
postSeparatorFields := strings.Fields(text[index+3:])
numPostFields := len(postSeparatorFields)

// This is an error as we can't detect if the mount is for "cgroup"
if numPostFields == 0 {
return "", fmt.Errorf("mountinfo: found no fields post '-' in %q", text)
}

if postSeparatorFields[0] == "cgroup" {
// Check that the mount is properly formatted.
if numPostFields < 3 {
return "", fmt.Errorf("Error found less than 3 fields post '-' in %q", text)
}

root = filepath.Dir(fields[4])
break
}
}
if err := scanner.Err(); err != nil {
return "", err
}
if root == "" {
if len(mi) < 1 {
return "", errors.New("no cgroup mount found in mountinfo")
}

// Get the first cgroup mount (e.g. "/sys/fs/cgroup/memory"),
// use its parent directory.
root := filepath.Dir(mi[0].Mountpoint)

if _, err := os.Stat(root); err != nil {
return "", err
}
Expand Down
44 changes: 33 additions & 11 deletions libcontainer/cgroups/utils_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"strings"
"testing"

"github.com/moby/sys/mountinfo"
"github.com/sirupsen/logrus"
)

Expand Down Expand Up @@ -244,7 +245,13 @@ func TestGetCgroupMounts(t *testing.T) {
},
}
for _, td := range testTable {
mi := bytes.NewBufferString(td.mountInfo)
mi, err := mountinfo.GetMountsFromReader(
bytes.NewBufferString(td.mountInfo),
mountinfo.FSTypeFilter("cgroup"),
)
if err != nil {
t.Fatal(err)
}
cgMounts, err := getCgroupMountsHelper(td.subsystems, mi, false)
if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -281,7 +288,6 @@ func TestGetCgroupMounts(t *testing.T) {
// Test the all=true case.

// Reset the test input.
mi = bytes.NewBufferString(td.mountInfo)
for k := range td.subsystems {
td.subsystems[k] = false
}
Expand Down Expand Up @@ -317,11 +323,15 @@ func BenchmarkGetCgroupMounts(b *testing.B) {
"perf_event": false,
"hugetlb": false,
}
mi, err := mountinfo.GetMountsFromReader(
bytes.NewBufferString(fedoraMountinfo),
mountinfo.FSTypeFilter("cgroup"),
)
if err != nil {
b.Fatal(err)
}
b.ResetTimer()
for i := 0; i < b.N; i++ {
b.StopTimer()
mi := bytes.NewBufferString(fedoraMountinfo)
b.StartTimer()
if _, err := getCgroupMountsHelper(subsystems, mi, false); err != nil {
b.Fatal(err)
}
Expand Down Expand Up @@ -396,7 +406,13 @@ func TestIgnoreCgroup2Mount(t *testing.T) {
"name=systemd": false,
}

mi := bytes.NewBufferString(cgroup2Mountinfo)
mi, err := mountinfo.GetMountsFromReader(
bytes.NewBufferString(cgroup2Mountinfo),
mountinfo.FSTypeFilter("cgroup"),
)
if err != nil {
t.Fatal(err)
}
cgMounts, err := getCgroupMountsHelper(subsystems, mi, false)
if err != nil {
t.Fatal(err)
Expand All @@ -409,10 +425,8 @@ func TestIgnoreCgroup2Mount(t *testing.T) {
}

func TestFindCgroupMountpointAndRoot(t *testing.T) {
fakeMountInfo := `
35 27 0:29 / /foo rw,nosuid,nodev,noexec,relatime shared:18 - cgroup cgroup rw,devices
35 27 0:29 / /sys/fs/cgroup/devices rw,nosuid,nodev,noexec,relatime shared:18 - cgroup cgroup rw,devices
`
fakeMountInfo := `35 27 0:29 / /foo rw,nosuid,nodev,noexec,relatime shared:18 - cgroup cgroup rw,devices
35 27 0:29 / /sys/fs/cgroup/devices rw,nosuid,nodev,noexec,relatime shared:18 - cgroup cgroup rw,devices`
testCases := []struct {
cgroupPath string
output string
Expand All @@ -421,8 +435,16 @@ func TestFindCgroupMountpointAndRoot(t *testing.T) {
{cgroupPath: "", output: "/foo"},
}

mi, err := mountinfo.GetMountsFromReader(
bytes.NewBufferString(fakeMountInfo),
mountinfo.FSTypeFilter("cgroup"),
)
if err != nil {
t.Fatal(err)
}

for _, c := range testCases {
mountpoint, _, _ := findCgroupMountpointAndRootFromReader(strings.NewReader(fakeMountInfo), c.cgroupPath, "devices")
mountpoint, _, _ := findCgroupMountpointAndRootFromMI(mi, c.cgroupPath, "devices")
if mountpoint != c.output {
t.Errorf("expected %s, got %s", c.output, mountpoint)
}
Expand Down
Loading

0 comments on commit 6eed6e5

Please sign in to comment.