Repository: opencontainers/runc
Pull request #4010 (2023-09-06 08:17:03)

feat: add `swapOnlyUsage` in `MemoryStats`
implement #4000 

lifubang: Thanks for your contribution. Please use `git rebase main` to catch up with the latest update, instead of `merge main`.
HeRaNO: > Thanks for your contribution. Please use `git rebase main` to catch up with the latest update, instead of `merge main`.

done
lifubang: And could you see `rootStatsFromMeminfo` in https://github.com/opencontainers/runc/blob/b137e99af0aea94d8bc4802c0330cb6b5476fa15/libcontainer/cgroups/fs2/memory.go#L105-L114 
1. Do you know whether `.pead` exist in `UnifiedMountpoint` or not?
2. If not, could you know how to get `max_usage_in_bytes` from `/proc/meminfo`?
HeRaNO: > And could you see `rootStatsFromMeminfo` in
> 
> https://github.com/opencontainers/runc/blob/b137e99af0aea94d8bc4802c0330cb6b5476fa15/libcontainer/cgroups/fs2/memory.go#L105-L114
> 
> 1. Do you know whether `.peak` exist in `UnifiedMountpoint` or not?
> 2. If not, could you know how to get `max_usage_in_bytes` from `/proc/meminfo`?

1. No.
2. It seems that `/proc/meminfo` doesn't export any usage of memory watermark, so It's hard to get `max_usage_in_bytes`.
AkihiroSuda: Can we have a test?

HeRaNO: > Can we have a test?

Sure, added.
HeRaNO: @haircommander @lifubang @AkihiroSuda PTAL, thx
AkihiroSuda: Please squash commits
haircommander: LGTM
HeRaNO: @kolyshkin PTAL, thx
HeRaNO: > It seems that we have accepted the ‘swapOnlyUsage’ approach, so please add some descriptions in ‘libcontainer/README.md’ to make each memory stat field more clear to the users.

I've added some explanation in `CHANGELOG.md`.

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index adcf084d..ffdc9ae2 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -31,6 +31,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
    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)
+ * Add `swapOnlyUsage` in `MemoryStats`. This field reports swap-only usage.
+   For cgroupv1, `Usage` and `Failcnt` are set by subtracting memory usage
+   from memory+swap usage. For cgroupv2, `Usage`, `Limit`, and `MaxUsage`
+   are set. (#4010)
 
 ### Fixed
 
diff --git a/libcontainer/cgroups/fs/memory.go b/libcontainer/cgroups/fs/memory.go
index a0e78074..783566d6 100644
--- a/libcontainer/cgroups/fs/memory.go
+++ b/libcontainer/cgroups/fs/memory.go
@@ -170,6 +170,10 @@ func (s *MemoryGroup) GetStats(path string, stats *cgroups.Stats) error {
 		return err
 	}
 	stats.MemoryStats.SwapUsage = swapUsage
+	stats.MemoryStats.SwapOnlyUsage = cgroups.MemoryData{
+		Usage:   swapUsage.Usage - memoryUsage.Usage,
+		Failcnt: swapUsage.Failcnt - memoryUsage.Failcnt,
+	}
 	kernelUsage, err := getMemoryData(path, "kmem")
 	if err != nil {
 		return err
diff --git a/libcontainer/cgroups/fs2/memory.go b/libcontainer/cgroups/fs2/memory.go
index 85e96b1c..29656597 100644
--- a/libcontainer/cgroups/fs2/memory.go
+++ b/libcontainer/cgroups/fs2/memory.go
@@ -105,7 +105,7 @@ func statMemory(dirPath string, stats *cgroups.Stats) error {
 	memoryUsage, err := getMemoryDataV2(dirPath, "")
 	if err != nil {
 		if errors.Is(err, unix.ENOENT) && dirPath == UnifiedMountpoint {
-			// The root cgroup does not have memory.{current,max}
+			// The root cgroup does not have memory.{current,max,peak}
 			// so emulate those using data from /proc/meminfo and
 			// /sys/fs/cgroup/memory.stat
 			return rootStatsFromMeminfo(stats)
@@ -113,10 +113,12 @@ func statMemory(dirPath string, stats *cgroups.Stats) error {
 		return err
 	}
 	stats.MemoryStats.Usage = memoryUsage
-	swapUsage, err := getMemoryDataV2(dirPath, "swap")
+	swapOnlyUsage, err := getMemoryDataV2(dirPath, "swap")
 	if err != nil {
 		return err
 	}
+	stats.MemoryStats.SwapOnlyUsage = swapOnlyUsage
+	swapUsage := swapOnlyUsage
 	// As cgroup v1 reports SwapUsage values as mem+swap combined,
 	// while in cgroup v2 swap values do not include memory,
 	// report combined mem+swap for v1 compatibility.
@@ -124,6 +126,9 @@ func statMemory(dirPath string, stats *cgroups.Stats) error {
 	if swapUsage.Limit != math.MaxUint64 {
 		swapUsage.Limit += memoryUsage.Limit
 	}
+	// The `MaxUsage` of mem+swap cannot simply combine mem with
+	// swap. So set it to 0 for v1 compatibility.
+	swapUsage.MaxUsage = 0
 	stats.MemoryStats.SwapUsage = swapUsage
 
 	return nil
@@ -138,6 +143,7 @@ func getMemoryDataV2(path, name string) (cgroups.MemoryData, error) {
 	}
 	usage := moduleName + ".current"
 	limit := moduleName + ".max"
+	maxUsage := moduleName + ".peak"
 
 	value, err := fscommon.GetCgroupParamUint(path, usage)
 	if err != nil {
@@ -157,6 +163,14 @@ func getMemoryDataV2(path, name string) (cgroups.MemoryData, error) {
 	}
 	memoryData.Limit = value
 
+	// `memory.peak` since kernel 5.19
+	// `memory.swap.peak` since kernel 6.5
+	value, err = fscommon.GetCgroupParamUint(path, maxUsage)
+	if err != nil && !os.IsNotExist(err) {
+		return cgroups.MemoryData{}, err
+	}
+	memoryData.MaxUsage = value
+
 	return memoryData, nil
 }
 
diff --git a/libcontainer/cgroups/stats.go b/libcontainer/cgroups/stats.go
index 8ff1fbb5..363017f1 100644
--- a/libcontainer/cgroups/stats.go
+++ b/libcontainer/cgroups/stats.go
@@ -91,6 +91,8 @@ type MemoryStats struct {
 	Usage MemoryData `json:"usage,omitempty"`
 	// usage of memory + swap
 	SwapUsage MemoryData `json:"swap_usage,omitempty"`
+	// usage of swap only
+	SwapOnlyUsage MemoryData `json:"swap_only_usage,omitempty"`
 	// usage of kernel memory
 	KernelUsage MemoryData `json:"kernel_usage,omitempty"`
 	// usage of kernel TCP memory


Test patch:
diff --git a/libcontainer/cgroups/fs/memory_test.go b/libcontainer/cgroups/fs/memory_test.go
index d305a62a..95e9d3cb 100644
--- a/libcontainer/cgroups/fs/memory_test.go
+++ b/libcontainer/cgroups/fs/memory_test.go
@@ -249,12 +249,13 @@ func TestMemoryStats(t *testing.T) {
 		t.Fatal(err)
 	}
 	expectedStats := cgroups.MemoryStats{
-		Cache:        512,
-		Usage:        cgroups.MemoryData{Usage: 2048, MaxUsage: 4096, Failcnt: 100, Limit: 8192},
-		SwapUsage:    cgroups.MemoryData{Usage: 2048, MaxUsage: 4096, Failcnt: 100, Limit: 8192},
-		KernelUsage:  cgroups.MemoryData{Usage: 2048, MaxUsage: 4096, Failcnt: 100, Limit: 8192},
-		Stats:        map[string]uint64{"cache": 512, "rss": 1024},
-		UseHierarchy: true,
+		Cache:         512,
+		Usage:         cgroups.MemoryData{Usage: 2048, MaxUsage: 4096, Failcnt: 100, Limit: 8192},
+		SwapUsage:     cgroups.MemoryData{Usage: 2048, MaxUsage: 4096, Failcnt: 100, Limit: 8192},
+		SwapOnlyUsage: cgroups.MemoryData{Usage: 0, MaxUsage: 0, Failcnt: 0, Limit: 0},
+		KernelUsage:   cgroups.MemoryData{Usage: 2048, MaxUsage: 4096, Failcnt: 100, Limit: 8192},
+		Stats:         map[string]uint64{"cache": 512, "rss": 1024},
+		UseHierarchy:  true,
 		PageUsageByNUMA: cgroups.PageUsageByNUMA{
 			PageUsageByNUMAInner: cgroups.PageUsageByNUMAInner{
 				Total:       cgroups.PageStats{Total: 44611, Nodes: map[uint8]uint64{0: 32631, 1: 7501, 2: 1982, 3: 2497}},
diff --git a/libcontainer/cgroups/fs2/memory_test.go b/libcontainer/cgroups/fs2/memory_test.go
index 2e2713c2..89c999d0 100644
--- a/libcontainer/cgroups/fs2/memory_test.go
+++ b/libcontainer/cgroups/fs2/memory_test.go
@@ -94,6 +94,10 @@ func TestStatMemoryPodCgroup(t *testing.T) {
 		t.Fatal(err)
 	}
 
+	if err := os.WriteFile(filepath.Join(fakeCgroupDir, "memory.peak"), []byte("987654321"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
 	gotStats := cgroups.NewStats()
 
 	// use a fake root path to trigger the pod cgroup lookup.
@@ -107,6 +111,18 @@ func TestStatMemoryPodCgroup(t *testing.T) {
 	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)
 	}
+
+	// result should be "memory.max"
+	var expectedLimitBytes uint64 = 999999999
+	if gotStats.MemoryStats.Usage.Limit != expectedLimitBytes {
+		t.Errorf("parsed cgroupv2 memory.stat doesn't match expected result: \ngot %#v\nexpected %#v\n", gotStats.MemoryStats.Usage.Limit, expectedLimitBytes)
+	}
+
+	// result should be "memory.peak"
+	var expectedMaxUsageBytes uint64 = 987654321
+	if gotStats.MemoryStats.Usage.MaxUsage != expectedMaxUsageBytes {
+		t.Errorf("parsed cgroupv2 memory.stat doesn't match expected result: \ngot %#v\nexpected %#v\n", gotStats.MemoryStats.Usage.MaxUsage, expectedMaxUsageBytes)
+	}
 }
 
 func TestRootStatsFromMeminfo(t *testing.T) {