From dc8d0cc17207f657dc12ef72252cd02f00d09e12 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 2 Dec 2021 11:53:04 -0800 Subject: [PATCH 01/11] libct/intelrdt: wrap Root in sync.Once In case resctrl filesystem can not be found in /proc/self/mountinfo (which is pretty common on non-server or non-x86 hardware), subsequent calls to Root() will result in parsing it again and again. Use sync.Once to avoid it. Make unit tests call it so that Root() won't actually parse mountinfo in tests. Signed-off-by: Kir Kolyshkin (cherry picked from commit 02e961bcf9423dbaa8134eac0b77e16f0550e40e) Signed-off-by: Kir Kolyshkin --- libcontainer/intelrdt/intelrdt.go | 41 +++++++++++++----------------- libcontainer/intelrdt/util_test.go | 5 ++++ 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/libcontainer/intelrdt/intelrdt.go b/libcontainer/intelrdt/intelrdt.go index 8b6bf3ef09d..07e4e1fd96b 100644 --- a/libcontainer/intelrdt/intelrdt.go +++ b/libcontainer/intelrdt/intelrdt.go @@ -276,35 +276,30 @@ func findIntelRdtMountpointDir(f io.Reader) (string, error) { // For Root() use only. var ( - intelRdtRoot string - rootMu sync.Mutex + intelRdtRoot string + intelRdtRootErr error + rootOnce sync.Once ) // Root returns the Intel RDT "resource control" filesystem mount point. func Root() (string, error) { - rootMu.Lock() - defer rootMu.Unlock() - - if intelRdtRoot != "" { - return intelRdtRoot, nil - } - - f, err := os.Open("/proc/self/mountinfo") - if err != nil { - return "", err - } - root, err := findIntelRdtMountpointDir(f) - f.Close() - if err != nil { - return "", err - } + rootOnce.Do(func() { + f, err := os.Open("/proc/self/mountinfo") + if err != nil { + intelRdtRootErr = err + return + } + root, err := findIntelRdtMountpointDir(f) + f.Close() + if err != nil { + intelRdtRootErr = err + return + } - if _, err := os.Stat(root); err != nil { - return "", err - } + intelRdtRoot = root + }) - intelRdtRoot = root - return intelRdtRoot, nil + return intelRdtRoot, intelRdtRootErr } type cpuInfoFlags struct { diff --git a/libcontainer/intelrdt/util_test.go b/libcontainer/intelrdt/util_test.go index f1b4244e245..b29d685e981 100644 --- a/libcontainer/intelrdt/util_test.go +++ b/libcontainer/intelrdt/util_test.go @@ -26,7 +26,12 @@ func NewIntelRdtTestUtil(t *testing.T) *intelRdtTestUtil { config := &configs.Config{ IntelRdt: &configs.IntelRdt{}, } + + // Assign fake intelRtdRoot value, returned by Root(). intelRdtRoot = t.TempDir() + // Make sure Root() won't even try to parse mountinfo. + rootOnce.Do(func() {}) + testIntelRdtPath := filepath.Join(intelRdtRoot, "resctrl") // Ensure the full mock Intel RDT "resource control" filesystem path exists From a5407b9a364b436560c02a22a18a377ab9955fa3 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 2 Dec 2021 12:09:33 -0800 Subject: [PATCH 02/11] libct/intelrdt: remove findMountpointDir test This test was written back in the day when findIntelRdtMountpointDir was using its own mountinfo parser. Commit f1c1fdf911ef changed that, and thus this test is actually testing moby/sys/mountinfo parser, which does not make much sense. Remove the test, and drop the io.Reader argument since we no longer need to parse a custom file. Signed-off-by: Kir Kolyshkin (cherry picked from commit 6c6b14e07585b5a94c103f0c13b2cf4895184c33) Signed-off-by: Kir Kolyshkin --- libcontainer/intelrdt/intelrdt.go | 15 +-- libcontainer/intelrdt/intelrdt_test.go | 140 ------------------------- 2 files changed, 4 insertions(+), 151 deletions(-) diff --git a/libcontainer/intelrdt/intelrdt.go b/libcontainer/intelrdt/intelrdt.go index 07e4e1fd96b..0797c82f2b0 100644 --- a/libcontainer/intelrdt/intelrdt.go +++ b/libcontainer/intelrdt/intelrdt.go @@ -5,7 +5,6 @@ import ( "bytes" "errors" "fmt" - "io" "os" "path/filepath" "strconv" @@ -250,9 +249,9 @@ func featuresInit() { }) } -// Return the mount point path of Intel RDT "resource control" filesysem -func findIntelRdtMountpointDir(f io.Reader) (string, error) { - mi, err := mountinfo.GetMountsFromReader(f, func(m *mountinfo.Info) (bool, bool) { +// Return the mount point path of Intel RDT "resource control" filesystem. +func findIntelRdtMountpointDir() (string, error) { + mi, err := mountinfo.GetMounts(func(m *mountinfo.Info) (bool, bool) { // similar to mountinfo.FSTypeFilter but stops after the first match if m.FSType == "resctrl" { return false, true // don't skip, stop @@ -284,13 +283,7 @@ var ( // Root returns the Intel RDT "resource control" filesystem mount point. func Root() (string, error) { rootOnce.Do(func() { - f, err := os.Open("/proc/self/mountinfo") - if err != nil { - intelRdtRootErr = err - return - } - root, err := findIntelRdtMountpointDir(f) - f.Close() + root, err := findIntelRdtMountpointDir() if err != nil { intelRdtRootErr = err return diff --git a/libcontainer/intelrdt/intelrdt_test.go b/libcontainer/intelrdt/intelrdt_test.go index 3534b438ce7..2184a1468df 100644 --- a/libcontainer/intelrdt/intelrdt_test.go +++ b/libcontainer/intelrdt/intelrdt_test.go @@ -1,8 +1,6 @@ package intelrdt import ( - "errors" - "io" "os" "path/filepath" "strings" @@ -127,141 +125,3 @@ func TestApply(t *testing.T) { t.Fatalf("unexpected tasks file, expected '1235', got %q", pids) } } - -const ( - mountinfoValid = `18 40 0:18 / /sys rw,nosuid,nodev,noexec,relatime shared:6 - sysfs sysfs rw -19 40 0:3 / /proc rw,nosuid,nodev,noexec,relatime shared:5 - proc proc rw -20 40 0:5 / /dev rw,nosuid shared:2 - devtmpfs devtmpfs rw,size=131927256k,nr_inodes=32981814,mode=755 -21 18 0:17 / /sys/kernel/security rw,nosuid,nodev,noexec,relatime shared:7 - securityfs securityfs rw -22 20 0:19 / /dev/shm rw,nosuid,nodev shared:3 - tmpfs tmpfs rw -23 20 0:12 / /dev/pts rw,nosuid,noexec,relatime shared:4 - devpts devpts rw,gid=5,mode=620,ptmxmode=000 -24 40 0:20 / /run rw,nosuid,nodev shared:22 - tmpfs tmpfs rw,mode=755 -25 18 0:21 / /sys/fs/cgroup ro,nosuid,nodev,noexec shared:8 - tmpfs tmpfs ro,mode=755 -26 25 0:22 / /sys/fs/cgroup/systemd rw,nosuid,nodev,noexec,relatime shared:9 - cgroup cgroup rw,xattr,release_agent=/usr/lib/systemd/systemd-cgroups-agent,name=systemd -27 18 0:23 / /sys/fs/pstore rw,nosuid,nodev,noexec,relatime shared:20 - pstore pstore rw -28 25 0:24 / /sys/fs/cgroup/perf_event rw,nosuid,nodev,noexec,relatime shared:10 - cgroup cgroup rw,perf_event -29 25 0:25 / /sys/fs/cgroup/cpu,cpuacct rw,nosuid,nodev,noexec,relatime shared:11 - cgroup cgroup rw,cpuacct,cpu -30 25 0:26 / /sys/fs/cgroup/memory rw,nosuid,nodev,noexec,relatime shared:12 - cgroup cgroup rw,memory -31 25 0:27 / /sys/fs/cgroup/devices rw,nosuid,nodev,noexec,relatime shared:13 - cgroup cgroup rw,devices -32 25 0:28 / /sys/fs/cgroup/hugetlb rw,nosuid,nodev,noexec,relatime shared:14 - cgroup cgroup rw,hugetlb -33 25 0:29 / /sys/fs/cgroup/blkio rw,nosuid,nodev,noexec,relatime shared:15 - cgroup cgroup rw,blkio -34 25 0:30 / /sys/fs/cgroup/pids rw,nosuid,nodev,noexec,relatime shared:16 - cgroup cgroup rw,pids -35 25 0:31 / /sys/fs/cgroup/cpuset rw,nosuid,nodev,noexec,relatime shared:17 - cgroup cgroup rw,cpuset -36 25 0:32 / /sys/fs/cgroup/freezer rw,nosuid,nodev,noexec,relatime shared:18 - cgroup cgroup rw,freezer -37 25 0:33 / /sys/fs/cgroup/net_cls,net_prio rw,nosuid,nodev,noexec,relatime shared:19 - cgroup cgroup rw,net_prio,net_cls -38 18 0:34 / /sys/kernel/config rw,relatime shared:21 - configfs configfs rw -40 0 253:0 / / rw,relatime shared:1 - ext4 /dev/mapper/vvrg-vvrg rw,data=ordered -16 18 0:6 / /sys/kernel/debug rw,relatime shared:23 - debugfs debugfs rw -41 18 0:16 / /sys/fs/resctrl rw,relatime shared:24 - resctrl resctrl rw -42 20 0:36 / /dev/hugepages rw,relatime shared:25 - hugetlbfs hugetlbfs rw -43 19 0:37 / /proc/sys/fs/binfmt_misc rw,relatime shared:26 - autofs systemd-1 rw,fd=32,pgrp=1,timeout=0,minproto=5,maxproto=5,direct,pipe_ino=35492 -44 20 0:15 / /dev/mqueue rw,relatime shared:27 - mqueue mqueue rw -45 40 8:1 / /boot rw,relatime shared:28 - ext4 /dev/sda1 rw,stripe=4,data=ordered -46 40 253:1 / /home rw,relatime shared:29 - ext4 /dev/mapper/vvhg-vvhg rw,data=ordered -47 40 0:38 / /var/lib/nfs/rpc_pipefs rw,relatime shared:30 - rpc_pipefs sunrpc rw -125 24 0:20 /mesos/containers /run/mesos/containers rw,nosuid shared:22 - tmpfs tmpfs rw,mode=755 -123 40 253:0 /var/lib/docker/containers /var/lib/docker/containers rw,relatime - ext4 /dev/mapper/vvrg-vvrg rw,data=ordered -129 40 253:0 /var/lib/docker/overlay2 /var/lib/docker/overlay2 rw,relatime - ext4 /dev/mapper/vvrg-vvrg rw,data=ordered -119 24 0:39 / /run/user/1009 rw,nosuid,nodev,relatime shared:100 - tmpfs tmpfs rw,size=26387788k,mode=700,uid=1009,gid=1009` - - mountinfoMbaSc = `18 40 0:18 / /sys rw,nosuid,nodev,noexec,relatime shared:6 - sysfs sysfs rw -19 40 0:3 / /proc rw,nosuid,nodev,noexec,relatime shared:5 - proc proc rw -20 40 0:5 / /dev rw,nosuid shared:2 - devtmpfs devtmpfs rw,size=131927256k,nr_inodes=32981814,mode=755 -21 18 0:17 / /sys/kernel/security rw,nosuid,nodev,noexec,relatime shared:7 - securityfs securityfs rw -22 20 0:19 / /dev/shm rw,nosuid,nodev shared:3 - tmpfs tmpfs rw -23 20 0:12 / /dev/pts rw,nosuid,noexec,relatime shared:4 - devpts devpts rw,gid=5,mode=620,ptmxmode=000 -24 40 0:20 / /run rw,nosuid,nodev shared:22 - tmpfs tmpfs rw,mode=755 -25 18 0:21 / /sys/fs/cgroup ro,nosuid,nodev,noexec shared:8 - tmpfs tmpfs ro,mode=755 -26 25 0:22 / /sys/fs/cgroup/systemd rw,nosuid,nodev,noexec,relatime shared:9 - cgroup cgroup rw,xattr,release_agent=/usr/lib/systemd/systemd-cgroups-agent,name=systemd -27 18 0:23 / /sys/fs/pstore rw,nosuid,nodev,noexec,relatime shared:20 - pstore pstore rw -28 25 0:24 / /sys/fs/cgroup/perf_event rw,nosuid,nodev,noexec,relatime shared:10 - cgroup cgroup rw,perf_event -29 25 0:25 / /sys/fs/cgroup/cpu,cpuacct rw,nosuid,nodev,noexec,relatime shared:11 - cgroup cgroup rw,cpuacct,cpu -30 25 0:26 / /sys/fs/cgroup/memory rw,nosuid,nodev,noexec,relatime shared:12 - cgroup cgroup rw,memory -31 25 0:27 / /sys/fs/cgroup/devices rw,nosuid,nodev,noexec,relatime shared:13 - cgroup cgroup rw,devices -32 25 0:28 / /sys/fs/cgroup/hugetlb rw,nosuid,nodev,noexec,relatime shared:14 - cgroup cgroup rw,hugetlb -33 25 0:29 / /sys/fs/cgroup/blkio rw,nosuid,nodev,noexec,relatime shared:15 - cgroup cgroup rw,blkio -34 25 0:30 / /sys/fs/cgroup/pids rw,nosuid,nodev,noexec,relatime shared:16 - cgroup cgroup rw,pids -35 25 0:31 / /sys/fs/cgroup/cpuset rw,nosuid,nodev,noexec,relatime shared:17 - cgroup cgroup rw,cpuset -36 25 0:32 / /sys/fs/cgroup/freezer rw,nosuid,nodev,noexec,relatime shared:18 - cgroup cgroup rw,freezer -37 25 0:33 / /sys/fs/cgroup/net_cls,net_prio rw,nosuid,nodev,noexec,relatime shared:19 - cgroup cgroup rw,net_prio,net_cls -38 18 0:34 / /sys/kernel/config rw,relatime shared:21 - configfs configfs rw -40 0 253:0 / / rw,relatime shared:1 - ext4 /dev/mapper/vvrg-vvrg rw,data=ordered -16 18 0:6 / /sys/kernel/debug rw,relatime shared:23 - debugfs debugfs rw -41 18 0:16 / /sys/fs/resctrl rw,relatime shared:24 - resctrl resctrl rw,mba_MBps -42 20 0:36 / /dev/hugepages rw,relatime shared:25 - hugetlbfs hugetlbfs rw -43 19 0:37 / /proc/sys/fs/binfmt_misc rw,relatime shared:26 - autofs systemd-1 rw,fd=32,pgrp=1,timeout=0,minproto=5,maxproto=5,direct,pipe_ino=35492 -44 20 0:15 / /dev/mqueue rw,relatime shared:27 - mqueue mqueue rw -45 40 8:1 / /boot rw,relatime shared:28 - ext4 /dev/sda1 rw,stripe=4,data=ordered -46 40 253:1 / /home rw,relatime shared:29 - ext4 /dev/mapper/vvhg-vvhg rw,data=ordered -47 40 0:38 / /var/lib/nfs/rpc_pipefs rw,relatime shared:30 - rpc_pipefs sunrpc rw -125 24 0:20 /mesos/containers /run/mesos/containers rw,nosuid shared:22 - tmpfs tmpfs rw,mode=755 -123 40 253:0 /var/lib/docker/containers /var/lib/docker/containers rw,relatime - ext4 /dev/mapper/vvrg-vvrg rw,data=ordered -129 40 253:0 /var/lib/docker/overlay2 /var/lib/docker/overlay2 rw,relatime - ext4 /dev/mapper/vvrg-vvrg rw,data=ordered -119 24 0:39 / /run/user/1009 rw,nosuid,nodev,relatime shared:100 - tmpfs tmpfs rw,size=26387788k,mode=700,uid=1009,gid=1009` -) - -func TestFindIntelRdtMountpointDir(t *testing.T) { - testCases := []struct { - name string - input io.Reader - isNotFoundError bool - isError bool - mbaScEnabled bool - mountpoint string - }{ - { - name: "Valid mountinfo with MBA Software Controller disabled", - input: strings.NewReader(mountinfoValid), - mountpoint: "/sys/fs/resctrl", - }, - { - name: "Valid mountinfo with MBA Software Controller enabled", - input: strings.NewReader(mountinfoMbaSc), - mbaScEnabled: true, - mountpoint: "/sys/fs/resctrl", - }, - { - name: "Empty mountinfo", - input: strings.NewReader(""), - isNotFoundError: true, - }, - { - name: "Broken mountinfo", - input: strings.NewReader("baa"), - isError: true, - }, - } - - for _, tc := range testCases { - tc := tc - t.Run(tc.name, func(t *testing.T) { - mbaScEnabled = false - mp, err := findIntelRdtMountpointDir(tc.input) - if tc.isNotFoundError { - if !errors.Is(err, errNotFound) { - t.Errorf("expected errNotFound error, got %+v", err) - } - return - } - if tc.isError { - if err == nil { - t.Error("expected error, got nil") - } - return - } - if err != nil { - t.Errorf("expected nil, got %+v", err) - return - } - // no errors, check the results - if tc.mbaScEnabled != mbaScEnabled { - t.Errorf("expected mbaScEnabled=%v, got %v", - tc.mbaScEnabled, mbaScEnabled) - } - if tc.mountpoint != mp { - t.Errorf("expected mountpoint=%q, got %q", - tc.mountpoint, mp) - } - }) - } -} From 5ba1b8ec436c193c1935d752cc9d5367182ff241 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 2 Dec 2021 20:51:44 -0800 Subject: [PATCH 03/11] libct/intelrdt: faster init if rdt is unsupported In a (quite common) case RDT is not supported by the kernel/hardware, it does not make sense to parse /proc/cpuinfo and /proc/self/mountinfo, and yet the current code does it (on every runc exec, for example). Fortunately, there is a quick way to check whether RDT is available -- if so, kernel creates /sys/fs/resctrl directory. Check its existence, and skip all the other initialization if it's not present. Signed-off-by: Kir Kolyshkin (cherry picked from commit edeb3b376c5cc763d7afd4c6a63d6e70f8f9992c) Signed-off-by: Kir Kolyshkin --- libcontainer/intelrdt/intelrdt.go | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/libcontainer/intelrdt/intelrdt.go b/libcontainer/intelrdt/intelrdt.go index 0797c82f2b0..27cc18f688d 100644 --- a/libcontainer/intelrdt/intelrdt.go +++ b/libcontainer/intelrdt/intelrdt.go @@ -12,6 +12,8 @@ import ( "sync" "github.com/moby/sys/mountinfo" + "golang.org/x/sys/unix" + "github.com/opencontainers/runc/libcontainer/cgroups/fscommon" "github.com/opencontainers/runc/libcontainer/configs" ) @@ -193,21 +195,21 @@ var ( // For Intel RDT initialization initOnce sync.Once - errNotFound = errors.New("Intel RDT resctrl mount point not found") + errNotFound = errors.New("Intel RDT not available") ) // Check if Intel RDT sub-features are enabled in featuresInit() func featuresInit() { initOnce.Do(func() { - // 1. Check if hardware and kernel support Intel RDT sub-features - flagsSet, err := parseCpuInfoFile("/proc/cpuinfo") + // 1. Check if Intel RDT "resource control" filesystem is available. + // The user guarantees to mount the filesystem. + root, err := Root() if err != nil { return } - // 2. Check if Intel RDT "resource control" filesystem is available. - // The user guarantees to mount the filesystem. - root, err := Root() + // 2. Check if hardware and kernel support Intel RDT sub-features. + flagsSet, err := parseCpuInfoFile("/proc/cpuinfo") if err != nil { return } @@ -283,6 +285,12 @@ var ( // Root returns the Intel RDT "resource control" filesystem mount point. func Root() (string, error) { rootOnce.Do(func() { + // If resctrl is available, kernel creates this directory. + if unix.Access("/sys/fs/resctrl", unix.F_OK) != nil { + intelRdtRootErr = errNotFound + return + } + root, err := findIntelRdtMountpointDir() if err != nil { intelRdtRootErr = err From dfdc7d078f694a1a8c9342900cb0a936e587be37 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 18 Jan 2022 17:41:47 -0800 Subject: [PATCH 04/11] libct/intelrdt: explain why mountinfo is required For the Nth time I wanted to replace parsing mountinfo with statfs and the check for superblock magic, but it is not possible since some code relies of mount options check which can only be obtained via mountinfo. Add a note about it. Signed-off-by: Kir Kolyshkin (cherry picked from commit 5e201e7ce2c48ce76010d4ab056c7a9a0c9429e2) Signed-off-by: Kir Kolyshkin --- libcontainer/intelrdt/intelrdt.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libcontainer/intelrdt/intelrdt.go b/libcontainer/intelrdt/intelrdt.go index 27cc18f688d..1fe1ec3e86c 100644 --- a/libcontainer/intelrdt/intelrdt.go +++ b/libcontainer/intelrdt/intelrdt.go @@ -291,6 +291,8 @@ func Root() (string, error) { return } + // NB: ideally, we could just do statfs and RDTGROUP_SUPER_MAGIC check, but + // we have to parse mountinfo since we're also interested in mount options. root, err := findIntelRdtMountpointDir() if err != nil { intelRdtRootErr = err From 69473d0af9bedb48677facc4540160c4c481a5de Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 9 Dec 2021 11:47:04 -0800 Subject: [PATCH 05/11] libct: rm TestGetContainerStats, mockIntelRdtManager TestGetContainerStats test a function that is smaller than the test itself, and only calls a couple of other functions (which are represented by mocks). It does not make sense to have it. mockIntelRdtManager is only needed for TestGetContainerStats and TestGetContainerState, which basically tests that Path is called. Also, it does not make much sense to have it. Signed-off-by: Kir Kolyshkin (cherry picked from commit 85932850ec4062da696d93a8f16b0cb3a0ad50b6) Signed-off-by: Kir Kolyshkin --- libcontainer/container_linux_test.go | 112 +-------------------------- 1 file changed, 4 insertions(+), 108 deletions(-) diff --git a/libcontainer/container_linux_test.go b/libcontainer/container_linux_test.go index 3eb6e5affa7..27eee624c07 100644 --- a/libcontainer/container_linux_test.go +++ b/libcontainer/container_linux_test.go @@ -7,22 +7,15 @@ import ( "github.com/opencontainers/runc/libcontainer/cgroups" "github.com/opencontainers/runc/libcontainer/configs" - "github.com/opencontainers/runc/libcontainer/intelrdt" "github.com/opencontainers/runc/libcontainer/system" ) type mockCgroupManager struct { pids []int allPids []int - stats *cgroups.Stats paths map[string]string } -type mockIntelRdtManager struct { - stats *intelrdt.Stats - path string -} - func (m *mockCgroupManager) GetPids() ([]int, error) { return m.pids, nil } @@ -32,7 +25,7 @@ func (m *mockCgroupManager) GetAllPids() ([]int, error) { } func (m *mockCgroupManager) GetStats() (*cgroups.Stats, error) { - return m.stats, nil + return nil, nil } func (m *mockCgroupManager) Apply(pid int) error { @@ -76,30 +69,6 @@ func (m *mockCgroupManager) GetFreezerState() (configs.FreezerState, error) { return configs.Thawed, nil } -func (m *mockIntelRdtManager) Apply(pid int) error { - return nil -} - -func (m *mockIntelRdtManager) GetStats() (*intelrdt.Stats, error) { - return m.stats, nil -} - -func (m *mockIntelRdtManager) Destroy() error { - return nil -} - -func (m *mockIntelRdtManager) GetPath() string { - return m.path -} - -func (m *mockIntelRdtManager) Set(container *configs.Config) error { - return nil -} - -func (m *mockIntelRdtManager) GetCgroups() (*configs.Cgroup, error) { - return nil, nil -} - type mockProcess struct { _pid int started uint64 @@ -173,61 +142,11 @@ func TestGetContainerPids(t *testing.T) { } } -func TestGetContainerStats(t *testing.T) { - container := &linuxContainer{ - id: "myid", - config: &configs.Config{}, - cgroupManager: &mockCgroupManager{ - pids: []int{1, 2, 3}, - stats: &cgroups.Stats{ - MemoryStats: cgroups.MemoryStats{ - Usage: cgroups.MemoryData{ - Usage: 1024, - }, - }, - }, - }, - intelRdtManager: &mockIntelRdtManager{ - stats: &intelrdt.Stats{ - L3CacheSchema: "L3:0=f;1=f0", - MemBwSchema: "MB:0=20;1=70", - }, - }, - } - stats, err := container.Stats() - if err != nil { - t.Fatal(err) - } - if stats.CgroupStats == nil { - t.Fatal("cgroup stats are nil") - } - if stats.CgroupStats.MemoryStats.Usage.Usage != 1024 { - t.Fatalf("expected memory usage 1024 but received %d", stats.CgroupStats.MemoryStats.Usage.Usage) - } - if intelrdt.IsCATEnabled() { - if stats.IntelRdtStats == nil { - t.Fatal("intel rdt stats are nil") - } - if stats.IntelRdtStats.L3CacheSchema != "L3:0=f;1=f0" { - t.Fatalf("expected L3CacheSchema L3:0=f;1=f0 but received %s", stats.IntelRdtStats.L3CacheSchema) - } - } - if intelrdt.IsMBAEnabled() { - if stats.IntelRdtStats == nil { - t.Fatal("intel rdt stats are nil") - } - if stats.IntelRdtStats.MemBwSchema != "MB:0=20;1=70" { - t.Fatalf("expected MemBwSchema MB:0=20;1=70 but received %s", stats.IntelRdtStats.MemBwSchema) - } - } -} - func TestGetContainerState(t *testing.T) { var ( - pid = os.Getpid() - expectedMemoryPath = "/sys/fs/cgroup/memory/myid" - expectedNetworkPath = fmt.Sprintf("/proc/%d/ns/net", pid) - expectedIntelRdtPath = "/sys/fs/resctrl/myid" + pid = os.Getpid() + expectedMemoryPath = "/sys/fs/cgroup/memory/myid" + expectedNetworkPath = fmt.Sprintf("/proc/%d/ns/net", pid) ) container := &linuxContainer{ id: "myid", @@ -248,24 +167,10 @@ func TestGetContainerState(t *testing.T) { }, cgroupManager: &mockCgroupManager{ pids: []int{1, 2, 3}, - stats: &cgroups.Stats{ - MemoryStats: cgroups.MemoryStats{ - Usage: cgroups.MemoryData{ - Usage: 1024, - }, - }, - }, paths: map[string]string{ "memory": expectedMemoryPath, }, }, - intelRdtManager: &mockIntelRdtManager{ - stats: &intelrdt.Stats{ - L3CacheSchema: "L3:0=f0;1=f", - MemBwSchema: "MB:0=70;1=20", - }, - path: expectedIntelRdtPath, - }, } container.state = &createdState{c: container} state, err := container.State() @@ -285,15 +190,6 @@ func TestGetContainerState(t *testing.T) { if memPath := paths["memory"]; memPath != expectedMemoryPath { t.Fatalf("expected memory path %q but received %q", expectedMemoryPath, memPath) } - if intelrdt.IsCATEnabled() || intelrdt.IsMBAEnabled() { - intelRdtPath := state.IntelRdtPath - if intelRdtPath == "" { - t.Fatal("intel rdt path should not be empty") - } - if intelRdtPath != expectedIntelRdtPath { - t.Fatalf("expected intel rdt path %q but received %q", expectedIntelRdtPath, intelRdtPath) - } - } for _, ns := range container.config.Namespaces { path := state.NamespacePaths[ns.Type] if path == "" { From 5ebcfa62816140c2a7a8972b4074db179694275b Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 9 Aug 2023 16:14:00 -0700 Subject: [PATCH 06/11] [1.1] libct: rm intelrtd.Manager interface, NewIntelRdtManager [This is a manual port of commit dbd990d5559733bd0f33e2 to release-1.1 branch. Original description follows.] Remove intelrtd.Manager interface, since we only have a single implementation, and do not expect another one. Rename intelRdtManager to Manager, and modify its users accordingly. Remove NewIntelRdtManager from factory. Remove IntelRdtfs. Instead, make intelrdt.NewManager return nil if the feature is not available. Remove TestFactoryNewIntelRdt as it is now identical to TestFactoryNew. Add internal function newManager to be used for tests (to make sure some testing is done even when the feature is not available in kernel/hardware). Signed-off-by: Kir Kolyshkin --- libcontainer/container_linux.go | 2 +- libcontainer/factory_linux.go | 43 ++++++---------------- libcontainer/factory_linux_test.go | 24 +------------ libcontainer/intelrdt/intelrdt.go | 49 +++++++++++--------------- libcontainer/intelrdt/intelrdt_test.go | 10 +++--- libcontainer/process_linux.go | 2 +- utils_linux.go | 4 +-- 7 files changed, 41 insertions(+), 93 deletions(-) diff --git a/libcontainer/container_linux.go b/libcontainer/container_linux.go index 5f1a494b4a8..2f17e37d43d 100644 --- a/libcontainer/container_linux.go +++ b/libcontainer/container_linux.go @@ -40,7 +40,7 @@ type linuxContainer struct { root string config *configs.Config cgroupManager cgroups.Manager - intelRdtManager intelrdt.Manager + intelRdtManager *intelrdt.Manager initPath string initArgs []string initProcess parentProcess diff --git a/libcontainer/factory_linux.go b/libcontainer/factory_linux.go index a1fa7de2d24..546d0eb000c 100644 --- a/libcontainer/factory_linux.go +++ b/libcontainer/factory_linux.go @@ -48,20 +48,6 @@ func InitArgs(args ...string) func(*LinuxFactory) error { } } -// IntelRdtfs is an options func to configure a LinuxFactory to return -// containers that use the Intel RDT "resource control" filesystem to -// create and manage Intel RDT resources (e.g., L3 cache, memory bandwidth). -func IntelRdtFs(l *LinuxFactory) error { - if !intelrdt.IsCATEnabled() && !intelrdt.IsMBAEnabled() { - l.NewIntelRdtManager = nil - } else { - l.NewIntelRdtManager = func(config *configs.Config, id string, path string) intelrdt.Manager { - return intelrdt.NewManager(config, id, path) - } - } - return nil -} - // TmpfsRoot is an option func to mount LinuxFactory.Root to tmpfs. func TmpfsRoot(l *LinuxFactory) error { mounted, err := mountinfo.Mounted(l.Root) @@ -136,9 +122,6 @@ type LinuxFactory struct { // Validator provides validation to container configurations. Validator validate.Validator - - // NewIntelRdtManager returns an initialized Intel RDT manager for a single container. - NewIntelRdtManager func(config *configs.Config, id string, path string) intelrdt.Manager } func (l *LinuxFactory) Create(id string, config *configs.Config) (Container, error) { @@ -208,18 +191,16 @@ func (l *LinuxFactory) Create(id string, config *configs.Config) (Container, err return nil, err } c := &linuxContainer{ - id: id, - root: containerRoot, - config: config, - initPath: l.InitPath, - initArgs: l.InitArgs, - criuPath: l.CriuPath, - newuidmapPath: l.NewuidmapPath, - newgidmapPath: l.NewgidmapPath, - cgroupManager: cm, - } - if l.NewIntelRdtManager != nil { - c.intelRdtManager = l.NewIntelRdtManager(config, id, "") + id: id, + root: containerRoot, + config: config, + initPath: l.InitPath, + initArgs: l.InitArgs, + criuPath: l.CriuPath, + newuidmapPath: l.NewuidmapPath, + newgidmapPath: l.NewgidmapPath, + cgroupManager: cm, + intelRdtManager: intelrdt.NewManager(config, id, ""), } c.state = &stoppedState{c: c} return c, nil @@ -261,12 +242,10 @@ func (l *LinuxFactory) Load(id string) (Container, error) { newuidmapPath: l.NewuidmapPath, newgidmapPath: l.NewgidmapPath, cgroupManager: cm, + intelRdtManager: intelrdt.NewManager(&state.Config, id, state.IntelRdtPath), root: containerRoot, created: state.Created, } - if l.NewIntelRdtManager != nil { - c.intelRdtManager = l.NewIntelRdtManager(&state.Config, id, state.IntelRdtPath) - } c.state = &loadedState{c: c} if err := c.refreshState(); err != nil { return nil, err diff --git a/libcontainer/factory_linux_test.go b/libcontainer/factory_linux_test.go index d29c32e9dbf..47f3069953b 100644 --- a/libcontainer/factory_linux_test.go +++ b/libcontainer/factory_linux_test.go @@ -37,28 +37,6 @@ func TestFactoryNew(t *testing.T) { } } -func TestFactoryNewIntelRdt(t *testing.T) { - root := t.TempDir() - factory, err := New(root, IntelRdtFs) - if err != nil { - t.Fatal(err) - } - if factory == nil { - t.Fatal("factory should not be nil") - } - lfactory, ok := factory.(*LinuxFactory) - if !ok { - t.Fatal("expected linux factory returned on linux based systems") - } - if lfactory.Root != root { - t.Fatalf("expected factory root to be %q but received %q", root, lfactory.Root) - } - - if factory.Type() != "libcontainer" { - t.Fatalf("unexpected factory type: %q, expected %q", factory.Type(), "libcontainer") - } -} - func TestFactoryNewTmpfs(t *testing.T) { root := t.TempDir() factory, err := New(root, TmpfsRoot) @@ -157,7 +135,7 @@ func TestFactoryLoadContainer(t *testing.T) { if err := marshal(filepath.Join(root, id, stateFilename), expectedState); err != nil { t.Fatal(err) } - factory, err := New(root, IntelRdtFs) + factory, err := New(root) if err != nil { t.Fatal(err) } diff --git a/libcontainer/intelrdt/intelrdt.go b/libcontainer/intelrdt/intelrdt.go index 1fe1ec3e86c..3953f930d25 100644 --- a/libcontainer/intelrdt/intelrdt.go +++ b/libcontainer/intelrdt/intelrdt.go @@ -146,34 +146,27 @@ import ( * } */ -type Manager interface { - // Applies Intel RDT configuration to the process with the specified pid - Apply(pid int) error - - // Returns statistics for Intel RDT - GetStats() (*Stats, error) - - // Destroys the Intel RDT container-specific 'container_id' group - Destroy() error - - // Returns Intel RDT path to save in a state file and to be able to - // restore the object later - GetPath() string - - // Set Intel RDT "resource control" filesystem as configured. - Set(container *configs.Config) error -} - -// This implements interface Manager -type intelRdtManager struct { +type Manager struct { mu sync.Mutex config *configs.Config id string path string } -func NewManager(config *configs.Config, id string, path string) Manager { - return &intelRdtManager{ +// NewManager returns a new instance of Manager, or nil, if the Intel RDT +// functionality is not available from hardware or not enabled in the kernel. +func NewManager(config *configs.Config, id string, path string) *Manager { + if _, err := Root(); err != nil { + // Intel RDT is not available. + return nil + } + return newManager(config, id, path) +} + +// newManager is the same as NewManager, except it does not check if the feature +// is actually available. Used by unit tests that mock intelrdt paths. +func newManager(config *configs.Config, id string, path string) *Manager { + return &Manager{ config: config, id: id, path: path, @@ -507,7 +500,7 @@ func IsMBAScEnabled() bool { } // Get the path of the clos group in "resource control" filesystem that the container belongs to -func (m *intelRdtManager) getIntelRdtPath() (string, error) { +func (m *Manager) getIntelRdtPath() (string, error) { rootPath, err := Root() if err != nil { return "", err @@ -522,7 +515,7 @@ func (m *intelRdtManager) getIntelRdtPath() (string, error) { } // Applies Intel RDT configuration to the process with the specified pid -func (m *intelRdtManager) Apply(pid int) (err error) { +func (m *Manager) Apply(pid int) (err error) { // If intelRdt is not specified in config, we do nothing if m.config.IntelRdt == nil { return nil @@ -557,7 +550,7 @@ func (m *intelRdtManager) Apply(pid int) (err error) { } // Destroys the Intel RDT container-specific 'container_id' group -func (m *intelRdtManager) Destroy() error { +func (m *Manager) Destroy() error { // Don't remove resctrl group if closid has been explicitly specified. The // group is likely externally managed, i.e. by some other entity than us. // There are probably other containers/tasks sharing the same group. @@ -574,7 +567,7 @@ func (m *intelRdtManager) Destroy() error { // Returns Intel RDT path to save in a state file and to be able to // restore the object later -func (m *intelRdtManager) GetPath() string { +func (m *Manager) GetPath() string { if m.path == "" { m.path, _ = m.getIntelRdtPath() } @@ -582,7 +575,7 @@ func (m *intelRdtManager) GetPath() string { } // Returns statistics for Intel RDT -func (m *intelRdtManager) GetStats() (*Stats, error) { +func (m *Manager) GetStats() (*Stats, error) { // If intelRdt is not specified in config if m.config.IntelRdt == nil { return nil, nil @@ -668,7 +661,7 @@ func (m *intelRdtManager) GetStats() (*Stats, error) { } // Set Intel RDT "resource control" filesystem as configured. -func (m *intelRdtManager) Set(container *configs.Config) error { +func (m *Manager) Set(container *configs.Config) error { // About L3 cache schema: // It has allocation bitmasks/values for L3 cache on each socket, // which contains L3 cache id and capacity bitmask (CBM). diff --git a/libcontainer/intelrdt/intelrdt_test.go b/libcontainer/intelrdt/intelrdt_test.go index 2184a1468df..c127cd8f7c6 100644 --- a/libcontainer/intelrdt/intelrdt_test.go +++ b/libcontainer/intelrdt/intelrdt_test.go @@ -20,7 +20,7 @@ func TestIntelRdtSetL3CacheSchema(t *testing.T) { }) helper.config.IntelRdt.L3CacheSchema = l3CacheSchemeAfter - intelrdt := NewManager(helper.config, "", helper.IntelRdtPath) + intelrdt := newManager(helper.config, "", helper.IntelRdtPath) if err := intelrdt.Set(helper.config); err != nil { t.Fatal(err) } @@ -50,7 +50,7 @@ func TestIntelRdtSetMemBwSchema(t *testing.T) { }) helper.config.IntelRdt.MemBwSchema = memBwSchemeAfter - intelrdt := NewManager(helper.config, "", helper.IntelRdtPath) + intelrdt := newManager(helper.config, "", helper.IntelRdtPath) if err := intelrdt.Set(helper.config); err != nil { t.Fatal(err) } @@ -80,7 +80,7 @@ func TestIntelRdtSetMemBwScSchema(t *testing.T) { }) helper.config.IntelRdt.MemBwSchema = memBwScSchemeAfter - intelrdt := NewManager(helper.config, "", helper.IntelRdtPath) + intelrdt := newManager(helper.config, "", helper.IntelRdtPath) if err := intelrdt.Set(helper.config); err != nil { t.Fatal(err) } @@ -103,7 +103,7 @@ func TestApply(t *testing.T) { const closID = "test-clos" helper.config.IntelRdt.ClosID = closID - intelrdt := NewManager(helper.config, "", helper.IntelRdtPath) + intelrdt := newManager(helper.config, "", helper.IntelRdtPath) if err := intelrdt.Apply(1234); err == nil { t.Fatal("unexpected success when applying pid") } @@ -112,7 +112,7 @@ func TestApply(t *testing.T) { } // Dir should be created if some schema has been specified - intelrdt.(*intelRdtManager).config.IntelRdt.L3CacheSchema = "L3:0=f" + intelrdt.config.IntelRdt.L3CacheSchema = "L3:0=f" if err := intelrdt.Apply(1235); err != nil { t.Fatalf("Apply() failed: %v", err) } diff --git a/libcontainer/process_linux.go b/libcontainer/process_linux.go index 446649af843..0d9ceb9c98c 100644 --- a/libcontainer/process_linux.go +++ b/libcontainer/process_linux.go @@ -299,7 +299,7 @@ type initProcess struct { logFilePair filePair config *initConfig manager cgroups.Manager - intelRdtManager intelrdt.Manager + intelRdtManager *intelrdt.Manager container *linuxContainer fds []string process *Process diff --git a/utils_linux.go b/utils_linux.go index a9badf20f8b..60d534e86b1 100644 --- a/utils_linux.go +++ b/utils_linux.go @@ -32,8 +32,6 @@ func loadFactory(context *cli.Context) (libcontainer.Factory, error) { return nil, err } - intelRdtManager := libcontainer.IntelRdtFs - // We resolve the paths for {newuidmap,newgidmap} from the context of runc, // to avoid doing a path lookup in the nsexec context. TODO: The binary // names are not currently configurable. @@ -46,7 +44,7 @@ func loadFactory(context *cli.Context) (libcontainer.Factory, error) { newgidmap = "" } - return libcontainer.New(abs, intelRdtManager, + return libcontainer.New(abs, libcontainer.CriuPath(context.GlobalString("criu")), libcontainer.NewuidmapPath(newuidmap), libcontainer.NewgidmapPath(newgidmap)) From 7c83dbe6ec2cec36f1962eae9f18b47d79044827 Mon Sep 17 00:00:00 2001 From: Cory Snider Date: Tue, 26 Jul 2022 15:36:47 -0400 Subject: [PATCH 07/11] libct/intelrdt: delete IsMBAScEnabled() This function is unused, and removing it simplifies the changes which follow this commit. Signed-off-by: Cory Snider (cherry picked from commit 13674f43d3ff08a6896ecefd1dd201f948cc7e0c) Signed-off-by: Kir Kolyshkin --- libcontainer/intelrdt/intelrdt.go | 20 +------------------- 1 file changed, 1 insertion(+), 19 deletions(-) diff --git a/libcontainer/intelrdt/intelrdt.go b/libcontainer/intelrdt/intelrdt.go index 3953f930d25..6ea5b2851c9 100644 --- a/libcontainer/intelrdt/intelrdt.go +++ b/libcontainer/intelrdt/intelrdt.go @@ -182,8 +182,6 @@ var ( catEnabled bool // The flag to indicate if Intel RDT/MBA is enabled mbaEnabled bool - // The flag to indicate if Intel RDT/MBA Software Controller is enabled - mbaScEnabled bool // For Intel RDT initialization initOnce sync.Once @@ -216,12 +214,7 @@ func featuresInit() { catEnabled = true } } - if mbaScEnabled { - // We confirm MBA Software Controller is enabled in step 2, - // MBA should be enabled because MBA Software Controller - // depends on MBA - mbaEnabled = true - } else if flagsSet.MBA { + if flagsSet.MBA { if _, err := os.Stat(filepath.Join(root, "info", "MB")); err == nil { mbaEnabled = true } @@ -260,11 +253,6 @@ func findIntelRdtMountpointDir() (string, error) { return "", errNotFound } - // Check if MBA Software Controller is enabled through mount option "-o mba_MBps" - if strings.Contains(","+mi[0].VFSOptions+",", ",mba_MBps,") { - mbaScEnabled = true - } - return mi[0].Mountpoint, nil } @@ -493,12 +481,6 @@ func IsMBAEnabled() bool { return mbaEnabled } -// Check if Intel RDT/MBA Software Controller is enabled -func IsMBAScEnabled() bool { - featuresInit() - return mbaScEnabled -} - // Get the path of the clos group in "resource control" filesystem that the container belongs to func (m *Manager) getIntelRdtPath() (string, error) { rootPath, err := Root() From 6a7a6a5741137981ed97f5c06b6d36656da7c257 Mon Sep 17 00:00:00 2001 From: Cory Snider Date: Tue, 26 Jul 2022 15:36:47 -0400 Subject: [PATCH 08/11] libct/intelrdt: skip reading /proc/cpuinfo Reading /proc/cpuinfo is a surprisingly expensive operation. Since kernel version 4.12 [1], opening /proc/cpuinfo on an x86 system can block for around 20 milliseconds while the kernel samples the current CPU frequency. There is a very recent patch [2] which gets rid of the delay, but has yet to make it into the mainline kenel. Regardless, kernels for which opening /proc/cpuinfo takes 20ms will continue to be run in production for years to come. libcontainer only opens /proc/cpuinfo to read the processor feature flags so all the delays to get an accurate snapshot of the CPU frequency are just wasted time. If we wanted to, we could interrogate the CPU features directly from userspace using the `CPUID` instruction. However, Intel and AMD CPUs have flags in different positions for their analogous sub-features and there are CPU quirks [3] which would need to be accounted for. Some Haswell server CPUs support RDT/CAT but are missing the `CPUID` flags advertising their support; the kernel checks for support on that processor family by probing the the hardware using privileged RDMSR/WRMSR instructions [4]. This sort of probing could not be implemented in userspace so it would not be possible to check for RDT feature support in userspace without false negatives on some hardware configurations. It looks like libcontainer reads the CPU feature flags as a kind of optimization so that it can skip checking whether the kernel supports an RDT sub-feature if the hardware support is missing. As the kernel only exposes subtrees in the `resctrl` filesystem for RDT sub-features with hardware and kernel support, checking the CPU feature flags is redundant from a correctness point of view. Remove the /proc/cpuinfo check as it is an optimization which actually hurts performance. [1]: https://unix.stackexchange.com/a/526679 [2]: https://lore.kernel.org/all/20220415161206.875029458@linutronix.de/ [3]: https://github.com/torvalds/linux/blob/7cf6a8a17f5b134b7e783c2d45c53298faef82a7/arch/x86/kernel/cpu/resctrl/core.c#L834-L851 [4]: https://github.com/torvalds/linux/blob/a6b450573b912316ad36262bfc70e7c3870c56d1/arch/x86/kernel/cpu/resctrl/core.c#L111-L153 Signed-off-by: Cory Snider (cherry picked from commit 9f107489b0742825cd32f6d9328b30174a00aef1) Signed-off-by: Kir Kolyshkin --- libcontainer/intelrdt/intelrdt.go | 101 ++++++------------------------ 1 file changed, 18 insertions(+), 83 deletions(-) diff --git a/libcontainer/intelrdt/intelrdt.go b/libcontainer/intelrdt/intelrdt.go index 6ea5b2851c9..82bfd32859d 100644 --- a/libcontainer/intelrdt/intelrdt.go +++ b/libcontainer/intelrdt/intelrdt.go @@ -1,7 +1,6 @@ package intelrdt import ( - "bufio" "bytes" "errors" "fmt" @@ -199,40 +198,28 @@ func featuresInit() { return } - // 2. Check if hardware and kernel support Intel RDT sub-features. - flagsSet, err := parseCpuInfoFile("/proc/cpuinfo") - if err != nil { - return - } - - // 3. Double check if Intel RDT sub-features are available in - // "resource control" filesystem. Intel RDT sub-features can be + // 2. Check if Intel RDT sub-features are available in "resource + // control" filesystem. Intel RDT sub-features can be // selectively disabled or enabled by kernel command line // (e.g., rdt=!l3cat,mba) in 4.14 and newer kernel - if flagsSet.CAT { - if _, err := os.Stat(filepath.Join(root, "info", "L3")); err == nil { - catEnabled = true - } + if _, err := os.Stat(filepath.Join(root, "info", "L3")); err == nil { + catEnabled = true } - if flagsSet.MBA { - if _, err := os.Stat(filepath.Join(root, "info", "MB")); err == nil { - mbaEnabled = true - } + if _, err := os.Stat(filepath.Join(root, "info", "MB")); err == nil { + mbaEnabled = true } - if flagsSet.MBMTotal || flagsSet.MBMLocal || flagsSet.CMT { - if _, err := os.Stat(filepath.Join(root, "info", "L3_MON")); err != nil { - return - } - enabledMonFeatures, err = getMonFeatures(root) - if err != nil { - return - } - if enabledMonFeatures.mbmTotalBytes || enabledMonFeatures.mbmLocalBytes { - mbmEnabled = true - } - if enabledMonFeatures.llcOccupancy { - cmtEnabled = true - } + if _, err := os.Stat(filepath.Join(root, "info", "L3_MON")); err != nil { + return + } + enabledMonFeatures, err = getMonFeatures(root) + if err != nil { + return + } + if enabledMonFeatures.mbmTotalBytes || enabledMonFeatures.mbmLocalBytes { + mbmEnabled = true + } + if enabledMonFeatures.llcOccupancy { + cmtEnabled = true } }) } @@ -286,58 +273,6 @@ func Root() (string, error) { return intelRdtRoot, intelRdtRootErr } -type cpuInfoFlags struct { - CAT bool // Cache Allocation Technology - MBA bool // Memory Bandwidth Allocation - - // Memory Bandwidth Monitoring related. - MBMTotal bool - MBMLocal bool - - CMT bool // Cache Monitoring Technology -} - -func parseCpuInfoFile(path string) (cpuInfoFlags, error) { - infoFlags := cpuInfoFlags{} - - f, err := os.Open(path) - if err != nil { - return infoFlags, err - } - defer f.Close() - - s := bufio.NewScanner(f) - for s.Scan() { - line := s.Text() - - // Search "cat_l3" and "mba" flags in first "flags" line - if strings.HasPrefix(line, "flags") { - flags := strings.Split(line, " ") - // "cat_l3" flag for CAT and "mba" flag for MBA - for _, flag := range flags { - switch flag { - case "cat_l3": - infoFlags.CAT = true - case "mba": - infoFlags.MBA = true - case "cqm_mbm_total": - infoFlags.MBMTotal = true - case "cqm_mbm_local": - infoFlags.MBMLocal = true - case "cqm_occup_llc": - infoFlags.CMT = true - } - } - return infoFlags, nil - } - } - if err := s.Err(); err != nil { - return infoFlags, err - } - - return infoFlags, nil -} - // Gets a single uint64 value from the specified file. func getIntelRdtParamUint(path, file string) (uint64, error) { fileName := filepath.Join(path, file) From 4796f49ca31dd3fa44b9d3c06b28c123bae5b67f Mon Sep 17 00:00:00 2001 From: Cory Snider Date: Tue, 26 Jul 2022 15:36:47 -0400 Subject: [PATCH 09/11] libct/intelrdt: elide parsing mountinfo The intelrdt package only needs to parse mountinfo to find the mount point of the resctrl filesystem. Users are generally going to mount the resctrl filesystem to the pre-created /sys/fs/resctrl directory, so there is a common case where mountinfo parsing is not required. Optimize for the common case with a fast path which checks both for the existence of the /sys/fs/resctrl directory and whether the resctrl filesystem was mounted to that path using a single statfs syscall. Signed-off-by: Cory Snider (cherry picked from commit c156bde7cc9bb721f7110b987992d7f9c8b951a0) Signed-off-by: Kir Kolyshkin --- libcontainer/intelrdt/intelrdt.go | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/libcontainer/intelrdt/intelrdt.go b/libcontainer/intelrdt/intelrdt.go index 82bfd32859d..4cd9ffc108f 100644 --- a/libcontainer/intelrdt/intelrdt.go +++ b/libcontainer/intelrdt/intelrdt.go @@ -224,7 +224,7 @@ func featuresInit() { }) } -// Return the mount point path of Intel RDT "resource control" filesystem. +// findIntelRdtMountpointDir returns the mount point of the Intel RDT "resource control" filesystem. func findIntelRdtMountpointDir() (string, error) { mi, err := mountinfo.GetMounts(func(m *mountinfo.Info) (bool, bool) { // similar to mountinfo.FSTypeFilter but stops after the first match @@ -250,24 +250,32 @@ var ( rootOnce sync.Once ) +// The kernel creates this (empty) directory if resctrl is supported by the +// hardware and kernel. The user is responsible for mounting the resctrl +// filesystem, and they could mount it somewhere else if they wanted to. +const defaultResctrlMountpoint = "/sys/fs/resctrl" + // Root returns the Intel RDT "resource control" filesystem mount point. func Root() (string, error) { rootOnce.Do(func() { - // If resctrl is available, kernel creates this directory. - if unix.Access("/sys/fs/resctrl", unix.F_OK) != nil { - intelRdtRootErr = errNotFound + // Does this system support resctrl? + var statfs unix.Statfs_t + if err := unix.Statfs(defaultResctrlMountpoint, &statfs); err != nil { + if errors.Is(err, unix.ENOENT) { + err = errNotFound + } + intelRdtRootErr = err return } - // NB: ideally, we could just do statfs and RDTGROUP_SUPER_MAGIC check, but - // we have to parse mountinfo since we're also interested in mount options. - root, err := findIntelRdtMountpointDir() - if err != nil { - intelRdtRootErr = err + // Has the resctrl fs been mounted to the default mount point? + if statfs.Type == unix.RDTGROUP_SUPER_MAGIC { + intelRdtRoot = defaultResctrlMountpoint return } - intelRdtRoot = root + // The resctrl fs could have been mounted somewhere nonstandard. + intelRdtRoot, intelRdtRootErr = findIntelRdtMountpointDir() }) return intelRdtRoot, intelRdtRootErr From 6cf9ac159eaaeeb10ada5b70f2ddce2f983adfbd Mon Sep 17 00:00:00 2001 From: Cory Snider Date: Tue, 26 Jul 2022 15:36:47 -0400 Subject: [PATCH 10/11] libct/intelrdt: skip remove unless configured The OCI runtime spec mandates "[i]f intelRdt is not set, the runtime MUST NOT manipulate any resctrl pseudo-filesystems." Attempting to delete files counts as manipulating, so stop doing that when the container's RDT configuration is nil. Signed-off-by: Cory Snider (cherry picked from commit 56daf36be2c5c944f3ee7a8d37cf809c514ffcf7) Signed-off-by: Kir Kolyshkin --- libcontainer/intelrdt/intelrdt.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libcontainer/intelrdt/intelrdt.go b/libcontainer/intelrdt/intelrdt.go index 4cd9ffc108f..8eb87a3f09e 100644 --- a/libcontainer/intelrdt/intelrdt.go +++ b/libcontainer/intelrdt/intelrdt.go @@ -479,7 +479,7 @@ func (m *Manager) Destroy() error { // Don't remove resctrl group if closid has been explicitly specified. The // group is likely externally managed, i.e. by some other entity than us. // There are probably other containers/tasks sharing the same group. - if m.config.IntelRdt == nil || m.config.IntelRdt.ClosID == "" { + if m.config.IntelRdt != nil && m.config.IntelRdt.ClosID == "" { m.mu.Lock() defer m.mu.Unlock() if err := os.RemoveAll(m.GetPath()); err != nil { From f44190e0769c57d7c0081df13fbf3977d872a214 Mon Sep 17 00:00:00 2001 From: Cory Snider Date: Tue, 26 Jul 2022 15:36:47 -0400 Subject: [PATCH 11/11] libct/intelrdt: check if available iff configured Unless the container's runtime config has intelRdt configuration set, any checks for whether Intel RDT is supported or the resctrl filesystem is mounted are a waste of time as, per the OCI Runtime Spec, "the runtime MUST NOT manipulate any resctrl pseudo-filesystems." And in the likely case where Intel RDT is supported by both the hardware and kernel but the resctrl filesystem is not mounted, these checks can get expensive as the intelrdt package needs to parse mountinfo to check whether the filesystem has been mounted to a non-standard path. Optimize for the common case of containers with no intelRdt configuration by only performing the checks when the container has opted in. Signed-off-by: Cory Snider (cherry picked from commit ea0bd7826846da19d16d805cf1f8582f01448300) Signed-off-by: Kir Kolyshkin --- libcontainer/intelrdt/intelrdt.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/libcontainer/intelrdt/intelrdt.go b/libcontainer/intelrdt/intelrdt.go index 8eb87a3f09e..0f5c457b099 100644 --- a/libcontainer/intelrdt/intelrdt.go +++ b/libcontainer/intelrdt/intelrdt.go @@ -152,9 +152,13 @@ type Manager struct { path string } -// NewManager returns a new instance of Manager, or nil, if the Intel RDT -// functionality is not available from hardware or not enabled in the kernel. +// NewManager returns a new instance of Manager, or nil if the Intel RDT +// functionality is not specified in the config, available from hardware or +// enabled in the kernel. func NewManager(config *configs.Config, id string, path string) *Manager { + if config.IntelRdt == nil { + return nil + } if _, err := Root(); err != nil { // Intel RDT is not available. return nil