From 9f107489b0742825cd32f6d9328b30174a00aef1 Mon Sep 17 00:00:00 2001 From: Cory Snider Date: Tue, 26 Jul 2022 15:36:47 -0400 Subject: [PATCH] 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 --- 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)