Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Another nail to mountinfo coffin #2599

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 19 additions & 42 deletions libcontainer/cgroups/fs/cpuset.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,10 @@ import (
"os"
"path/filepath"

"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 @@ -43,46 +41,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 @@ -111,22 +82,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
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
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`
cyphar marked this conversation as resolved.
Show resolved Hide resolved
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