diff --git a/CHANGELOG.md b/CHANGELOG.md index ad7e4d390e5..65ebce91093 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased 1.1.z] +### Changed + + * Sum `anon` and `file` from `memory.stat` for cgroupv2 root usage, + as the root does not have `memory.current` for cgroupv2. + This aligns cgroupv2 root usage more closely with cgroupv1 reporting. + Additionally, report root swap usage as sum of swap and memory usage, + aligned with v1 and existing non-root v2 reporting. (#3933) + ## [1.1.8] - 2023-07-20 > 海纳百川 有容乃大 diff --git a/libcontainer/cgroups/fs2/memory.go b/libcontainer/cgroups/fs2/memory.go index adbc4b23086..9cca98c4c0a 100644 --- a/libcontainer/cgroups/fs2/memory.go +++ b/libcontainer/cgroups/fs2/memory.go @@ -101,8 +101,9 @@ func statMemory(dirPath string, stats *cgroups.Stats) error { if err != nil { if errors.Is(err, unix.ENOENT) && dirPath == UnifiedMountpoint { // The root cgroup does not have memory.{current,max} - // so emulate those using data from /proc/meminfo. - return statsFromMeminfo(stats) + // so emulate those using data from /proc/meminfo and + // /sys/fs/cgroup/memory.stat + return rootStatsFromMeminfo(stats) } return err } @@ -154,7 +155,7 @@ func getMemoryDataV2(path, name string) (cgroups.MemoryData, error) { return memoryData, nil } -func statsFromMeminfo(stats *cgroups.Stats) error { +func rootStatsFromMeminfo(stats *cgroups.Stats) error { const file = "/proc/meminfo" f, err := os.Open(file) if err != nil { @@ -166,14 +167,10 @@ func statsFromMeminfo(stats *cgroups.Stats) error { var ( swap_free uint64 swap_total uint64 - main_total uint64 - main_free uint64 ) mem := map[string]*uint64{ "SwapFree": &swap_free, "SwapTotal": &swap_total, - "MemTotal": &main_total, - "MemFree": &main_free, } found := 0 @@ -206,11 +203,18 @@ func statsFromMeminfo(stats *cgroups.Stats) error { return &parseError{Path: "", File: file, Err: err} } + // cgroup v1 `usage_in_bytes` reports memory usage as the sum of + // - rss (NR_ANON_MAPPED) + // - cache (NR_FILE_PAGES) + // cgroup v1 reports SwapUsage values as mem+swap combined + // cgroup v2 reports rss and cache as anon and file. + // sum `anon` + `file` to report the same value as `usage_in_bytes` in v1. + // sum swap usage as combined mem+swap usage for consistency as well. + stats.MemoryStats.Usage.Usage = stats.MemoryStats.Stats["anon"] + stats.MemoryStats.Stats["file"] + stats.MemoryStats.Usage.Limit = math.MaxUint64 stats.MemoryStats.SwapUsage.Usage = (swap_total - swap_free) * 1024 stats.MemoryStats.SwapUsage.Limit = math.MaxUint64 - - stats.MemoryStats.Usage.Usage = (main_total - main_free) * 1024 - stats.MemoryStats.Usage.Limit = math.MaxUint64 + stats.MemoryStats.SwapUsage.Usage += stats.MemoryStats.Usage.Usage return nil } diff --git a/libcontainer/cgroups/fs2/memory_test.go b/libcontainer/cgroups/fs2/memory_test.go new file mode 100644 index 00000000000..2e2713c29ae --- /dev/null +++ b/libcontainer/cgroups/fs2/memory_test.go @@ -0,0 +1,139 @@ +package fs2 + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/opencontainers/runc/libcontainer/cgroups" +) + +const exampleMemoryStatData = `anon 790425600 +file 6502666240 +kernel_stack 7012352 +pagetables 8867840 +percpu 2445520 +sock 40960 +shmem 6721536 +file_mapped 656187392 +file_dirty 1122304 +file_writeback 0 +swapcached 10 +anon_thp 438304768 +file_thp 0 +shmem_thp 0 +inactive_anon 892223488 +active_anon 2973696 +inactive_file 5307346944 +active_file 1179316224 +unevictable 31477760 +slab_reclaimable 348866240 +slab_unreclaimable 10099808 +slab 358966048 +workingset_refault_anon 0 +workingset_refault_file 0 +workingset_activate_anon 0 +workingset_activate_file 0 +workingset_restore_anon 0 +workingset_restore_file 0 +workingset_nodereclaim 0 +pgfault 103216687 +pgmajfault 6879 +pgrefill 0 +pgscan 0 +pgsteal 0 +pgactivate 1110217 +pgdeactivate 292 +pglazyfree 267 +pglazyfreed 0 +thp_fault_alloc 57411 +thp_collapse_alloc 443` + +func TestStatMemoryPodCgroupNotFound(t *testing.T) { + // We're using a fake cgroupfs. + cgroups.TestMode = true + fakeCgroupDir := t.TempDir() + + // only write memory.stat to ensure pod cgroup usage + // still reads memory.current. + statPath := filepath.Join(fakeCgroupDir, "memory.stat") + if err := os.WriteFile(statPath, []byte(exampleMemoryStatData), 0o644); err != nil { + t.Fatal(err) + } + + gotStats := cgroups.NewStats() + + // use a fake root path to mismatch the file we wrote. + // this triggers the non-root path which should fail to find memory.current. + err := statMemory(fakeCgroupDir, gotStats) + if err == nil { + t.Errorf("expected error when statting memory for cgroupv2 root, but was nil") + } + + if !strings.Contains(err.Error(), "memory.current: no such file or directory") { + t.Errorf("expected error to contain 'memory.current: no such file or directory', but was %s", err.Error()) + } +} + +func TestStatMemoryPodCgroup(t *testing.T) { + // We're using a fake cgroupfs. + cgroups.TestMode = true + fakeCgroupDir := t.TempDir() + + statPath := filepath.Join(fakeCgroupDir, "memory.stat") + if err := os.WriteFile(statPath, []byte(exampleMemoryStatData), 0o644); err != nil { + t.Fatal(err) + } + + if err := os.WriteFile(filepath.Join(fakeCgroupDir, "memory.current"), []byte("123456789"), 0o644); err != nil { + t.Fatal(err) + } + + if err := os.WriteFile(filepath.Join(fakeCgroupDir, "memory.max"), []byte("999999999"), 0o644); err != nil { + t.Fatal(err) + } + + gotStats := cgroups.NewStats() + + // use a fake root path to trigger the pod cgroup lookup. + err := statMemory(fakeCgroupDir, gotStats) + if err != nil { + t.Errorf("expected no error when statting memory for cgroupv2 root, but got %#+v", err) + } + + // result should be "memory.current" + var expectedUsageBytes uint64 = 123456789 + if gotStats.MemoryStats.Usage.Usage != expectedUsageBytes { + t.Errorf("parsed cgroupv2 memory.stat doesn't match expected result: \ngot %#v\nexpected %#v\n", gotStats.MemoryStats.Usage.Usage, expectedUsageBytes) + } +} + +func TestRootStatsFromMeminfo(t *testing.T) { + stats := &cgroups.Stats{ + MemoryStats: cgroups.MemoryStats{ + Stats: map[string]uint64{ + "anon": 790425600, + "file": 6502666240, + }, + }, + } + + if err := rootStatsFromMeminfo(stats); err != nil { + t.Fatal(err) + } + + // result is anon + file + var expectedUsageBytes uint64 = 7293091840 + if stats.MemoryStats.Usage.Usage != expectedUsageBytes { + t.Errorf("parsed cgroupv2 memory.stat doesn't match expected result: \ngot %d\nexpected %d\n", stats.MemoryStats.Usage.Usage, expectedUsageBytes) + } + + // swap is adjusted to mem+swap + if stats.MemoryStats.SwapUsage.Usage < stats.MemoryStats.Usage.Usage { + t.Errorf("swap usage %d should be at least mem usage %d", stats.MemoryStats.SwapUsage.Usage, stats.MemoryStats.Usage.Usage) + } + if stats.MemoryStats.SwapUsage.Limit < stats.MemoryStats.Usage.Limit { + t.Errorf("swap limit %d should be at least mem limit %d", stats.MemoryStats.SwapUsage.Limit, stats.MemoryStats.Usage.Limit) + } +}