From 1359772433825e100eab926b919d3d5f01856555 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 31 Dec 2024 18:58:57 +0100 Subject: [PATCH] pkg/sysinfo: parse cpuset.cpus/mems once and memoize Preserve the result instead of parsing these for each container that specifies cpuset options, Signed-off-by: Sebastiaan van Stijn --- integration-cli/docker_cli_run_unix_test.go | 13 +++----- pkg/sysinfo/cgroup2_linux.go | 14 ++++++++ pkg/sysinfo/sysinfo.go | 12 +++++-- pkg/sysinfo/sysinfo_linux.go | 37 ++++++++++++--------- pkg/sysinfo/sysinfo_linux_test.go | 27 +++++++++------ pkg/sysinfo/sysinfo_other.go | 2 +- 6 files changed, 67 insertions(+), 38 deletions(-) diff --git a/integration-cli/docker_cli_run_unix_test.go b/integration-cli/docker_cli_run_unix_test.go index 9e94473898..59ad86ed1c 100644 --- a/integration-cli/docker_cli_run_unix_test.go +++ b/integration-cli/docker_cli_run_unix_test.go @@ -20,7 +20,6 @@ import ( "github.com/docker/docker/client" "github.com/docker/docker/integration-cli/cli" "github.com/docker/docker/integration-cli/cli/build" - "github.com/docker/docker/pkg/parsers" "github.com/docker/docker/pkg/sysinfo" "github.com/docker/docker/testutil" "github.com/moby/sys/mount" @@ -694,12 +693,10 @@ func (s *DockerCLIRunSuite) TestRunSwapLessThanMemoryLimit(c *testing.T) { func (s *DockerCLIRunSuite) TestRunInvalidCpusetCpusFlagValue(c *testing.T) { testRequires(c, cgroupCpuset, testEnv.IsLocalDaemon) - sysInfo := sysinfo.New() - cpus, err := parsers.ParseUintList(sysInfo.Cpus) - assert.NilError(c, err) + cpus := sysinfo.New().CPUSets var invalid int for i := 0; i <= len(cpus)+1; i++ { - if !cpus[i] { + if _, ok := cpus[i]; !ok { invalid = i break } @@ -713,12 +710,10 @@ func (s *DockerCLIRunSuite) TestRunInvalidCpusetCpusFlagValue(c *testing.T) { func (s *DockerCLIRunSuite) TestRunInvalidCpusetMemsFlagValue(c *testing.T) { testRequires(c, cgroupCpuset) - sysInfo := sysinfo.New() - mems, err := parsers.ParseUintList(sysInfo.Mems) - assert.NilError(c, err) + mems := sysinfo.New().MemSets var invalid int for i := 0; i <= len(mems)+1; i++ { - if !mems[i] { + if _, ok := mems[i]; !ok { invalid = i break } diff --git a/pkg/sysinfo/cgroup2_linux.go b/pkg/sysinfo/cgroup2_linux.go index 698e23602f..0762e1a8f6 100644 --- a/pkg/sysinfo/cgroup2_linux.go +++ b/pkg/sysinfo/cgroup2_linux.go @@ -125,11 +125,25 @@ func applyCPUSetCgroupInfoV2(info *SysInfo) { } info.Cpus = strings.TrimSpace(string(cpus)) + cpuSets, err := parseUintList(info.Cpus, 0) + if err != nil { + info.Warnings = append(info.Warnings, "Unable to parse cpuset cpus: "+err.Error()) + return + } + info.CPUSets = cpuSets + mems, err := os.ReadFile(path.Join("/sys/fs/cgroup", info.cg2GroupPath, "cpuset.mems.effective")) if err != nil { return } info.Mems = strings.TrimSpace(string(mems)) + + memSets, err := parseUintList(info.Cpus, 0) + if err != nil { + info.Warnings = append(info.Warnings, "Unable to parse cpuset mems: "+err.Error()) + return + } + info.MemSets = memSets } func applyPIDSCgroupInfoV2(info *SysInfo) { diff --git a/pkg/sysinfo/sysinfo.go b/pkg/sysinfo/sysinfo.go index c6cb0b8d19..9ab8519cfc 100644 --- a/pkg/sysinfo/sysinfo.go +++ b/pkg/sysinfo/sysinfo.go @@ -124,9 +124,17 @@ type cgroupCpusetInfo struct { // or "cpuset.cpus" (cgroups v1). Cpus string + // CPUSets holds the list of available cpusets parsed from "cpuset.cpus.effective" (cgroups v2) + // or "cpuset.cpus" (cgroups v1). + CPUSets map[int]struct{} + // Available Cpuset's memory nodes as read from "cpuset.mems.effective" (cgroups v2) // or "cpuset.mems" (cgroups v1). Mems string + + // MemSets holds the list of available cpusets parsed from "cpuset.mems.effective" (cgroups v2) + // or "cpuset.mems" (cgroups v1). + MemSets map[int]struct{} } type cgroupPids struct { @@ -138,12 +146,12 @@ type cgroupPids struct { // in cgroup's cpuset.cpus set, `false` otherwise. // If error is not nil a parsing error occurred. func (c cgroupCpusetInfo) IsCpusetCpusAvailable(requested string) (bool, error) { - return isCpusetListAvailable(requested, c.Cpus) + return isCpusetListAvailable(requested, c.CPUSets) } // IsCpusetMemsAvailable returns `true` if the provided string set is contained // in cgroup's cpuset.mems set, `false` otherwise. // If error is not nil a parsing error occurred. func (c cgroupCpusetInfo) IsCpusetMemsAvailable(requested string) (bool, error) { - return isCpusetListAvailable(requested, c.Mems) + return isCpusetListAvailable(requested, c.MemSets) } diff --git a/pkg/sysinfo/sysinfo_linux.go b/pkg/sysinfo/sysinfo_linux.go index eb59f621ab..b8a22682b7 100644 --- a/pkg/sysinfo/sysinfo_linux.go +++ b/pkg/sysinfo/sysinfo_linux.go @@ -241,12 +241,24 @@ func applyCPUSetCgroupInfo(info *SysInfo) { return } info.Cpus = strings.TrimSpace(string(cpus)) + cpuSets, err := parseUintList(info.Cpus, 0) + if err != nil { + info.Warnings = append(info.Warnings, "Unable to parse cpuset cpus: "+err.Error()) + return + } + info.CPUSets = cpuSets mems, err := os.ReadFile(path.Join(mountPoint, "cpuset.mems")) if err != nil { return } info.Mems = strings.TrimSpace(string(mems)) + memSets, err := parseUintList(info.Cpus, 0) + if err != nil { + info.Warnings = append(info.Warnings, "Unable to parse cpuset mems: "+err.Error()) + return + } + info.MemSets = memSets } // applyPIDSCgroupInfo adds whether the pids cgroup controller is available to the info. @@ -319,11 +331,7 @@ func readProcBool(path string) bool { // defaultMaxCPUs is the normal maximum number of CPUs on Linux. const defaultMaxCPUs = 8192 -func isCpusetListAvailable(requested, available string) (bool, error) { - parsedAvailable, err := parseUintList(available, 0) - if err != nil { - return false, err - } +func isCpusetListAvailable(requested string, available map[int]struct{}) (bool, error) { // Start with the normal maximum number of CPUs on Linux, but accept // more if we actually have more CPUs available. // @@ -336,7 +344,7 @@ func isCpusetListAvailable(requested, available string) (bool, error) { // // More details in https://github.com/docker-archive/engine/pull/70#issuecomment-458458288 maxCPUs := defaultMaxCPUs - for m := range parsedAvailable { + for m := range available { if m > maxCPUs { maxCPUs = m } @@ -346,7 +354,7 @@ func isCpusetListAvailable(requested, available string) (bool, error) { return false, err } for k := range parsedRequested { - if !parsedAvailable[k] { + if _, ok := available[k]; !ok { return false, nil } } @@ -357,7 +365,7 @@ func isCpusetListAvailable(requested, available string) (bool, error) { // found in some cgroup file (e.g. `cpuset.cpus`, `cpuset.mems`), which could be // one of the formats below. Note that duplicates are actually allowed in the // input string. It returns a `map[int]bool` with available elements from `val` -// set to `true`. Values larger than `maximum` cause an error if max is non zero, +// set to `true`. Values larger than `maximum` cause an error if max is non-zero, // in order to stop the map becoming excessively large. // Supported formats: // @@ -368,16 +376,15 @@ func isCpusetListAvailable(requested, available string) (bool, error) { // 03,1-3 <- this is gonna get parsed as [1,2,3] // 3,2,1 // 0-2,3,1 -func parseUintList(val string, maximum int) (map[int]bool, error) { +func parseUintList(val string, maximum int) (map[int]struct{}, error) { if val == "" { - return map[int]bool{}, nil + return map[int]struct{}{}, nil } - availableInts := make(map[int]bool) - split := strings.Split(val, ",") + availableInts := make(map[int]struct{}) errInvalidFormat := fmt.Errorf("invalid format: %s", val) - for _, r := range split { + for _, r := range strings.Split(val, ",") { if !strings.Contains(r, "-") { v, err := strconv.Atoi(r) if err != nil { @@ -386,7 +393,7 @@ func parseUintList(val string, maximum int) (map[int]bool, error) { if maximum != 0 && v > maximum { return nil, fmt.Errorf("value of out range, maximum is %d", maximum) } - availableInts[v] = true + availableInts[v] = struct{}{} } else { minS, maxS, _ := strings.Cut(r, "-") minAvailable, err := strconv.Atoi(minS) @@ -404,7 +411,7 @@ func parseUintList(val string, maximum int) (map[int]bool, error) { return nil, fmt.Errorf("value of out range, maximum is %d", maximum) } for i := minAvailable; i <= maxAvailable; i++ { - availableInts[i] = true + availableInts[i] = struct{}{} } } } diff --git a/pkg/sysinfo/sysinfo_linux_test.go b/pkg/sysinfo/sysinfo_linux_test.go index c7b021e6b6..f847120741 100644 --- a/pkg/sysinfo/sysinfo_linux_test.go +++ b/pkg/sysinfo/sysinfo_linux_test.go @@ -83,12 +83,16 @@ func TestIsCpusetListAvailable(t *testing.T) { {"01,3", "0-4", true, false}, {"", "0-7", true, false}, {"1--42", "0-7", false, true}, - {"1-42", "00-1,8,,9", false, true}, + {"1-42", "00-1,8,9", false, true}, {"1,41-42", "43,45", false, false}, {"0-3", "", false, false}, } for _, c := range cases { - r, err := isCpusetListAvailable(c.provided, c.available) + available, err := parseUintList(c.available, 0) + if err != nil { + t.Fatal(err) + } + r, err := isCpusetListAvailable(c.provided, available) if (c.err && err == nil) && r != c.res { t.Fatalf("Expected pair: %v, %v for %s, %s. Got %v, %v instead", c.res, c.err, c.provided, c.available, (c.err && err == nil), r) } @@ -96,16 +100,17 @@ func TestIsCpusetListAvailable(t *testing.T) { } func TestParseUintList(t *testing.T) { - valids := map[string]map[int]bool{ + yes := struct{}{} + valids := map[string]map[int]struct{}{ "": {}, - "7": {7: true}, - "1-6": {1: true, 2: true, 3: true, 4: true, 5: true, 6: true}, - "0-7": {0: true, 1: true, 2: true, 3: true, 4: true, 5: true, 6: true, 7: true}, - "0,3-4,7,8-10": {0: true, 3: true, 4: true, 7: true, 8: true, 9: true, 10: true}, - "0-0,0,1-4": {0: true, 1: true, 2: true, 3: true, 4: true}, - "03,1-3": {1: true, 2: true, 3: true}, - "3,2,1": {1: true, 2: true, 3: true}, - "0-2,3,1": {0: true, 1: true, 2: true, 3: true}, + "7": {7: yes}, + "1-6": {1: yes, 2: yes, 3: yes, 4: yes, 5: yes, 6: yes}, + "0-7": {0: yes, 1: yes, 2: yes, 3: yes, 4: yes, 5: yes, 6: yes, 7: yes}, + "0,3-4,7,8-10": {0: yes, 3: yes, 4: yes, 7: yes, 8: yes, 9: yes, 10: yes}, + "0-0,0,1-4": {0: yes, 1: yes, 2: yes, 3: yes, 4: yes}, + "03,1-3": {1: yes, 2: yes, 3: yes}, + "3,2,1": {1: yes, 2: yes, 3: yes}, + "0-2,3,1": {0: yes, 1: yes, 2: yes, 3: yes}, } for k, v := range valids { out, err := parseUintList(k, 0) diff --git a/pkg/sysinfo/sysinfo_other.go b/pkg/sysinfo/sysinfo_other.go index 00f0f4f5ce..9614f1e0ec 100644 --- a/pkg/sysinfo/sysinfo_other.go +++ b/pkg/sysinfo/sysinfo_other.go @@ -7,6 +7,6 @@ func New(options ...Opt) *SysInfo { return &SysInfo{} } -func isCpusetListAvailable(string, string) (bool, error) { +func isCpusetListAvailable(string, map[int]struct{}) (bool, error) { return false, nil }