From 69b9868db81945fda3cb104986a6f6f67b3560c1 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Fri, 23 Oct 2026 21:02:31 +0000 Subject: [PATCH 1/2] fix: read cgroupv2 cpu usage from the cgroup that owns cpu.max Fixes #41. --- cgroupv2.go | 71 +++++++++++++++++++++++++++++++++++-------- stat_internal_test.go | 45 +++++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 12 deletions(-) diff --git a/cgroupv2.go b/cgroupv2.go index e3b7aaa..ecff41f 100644 --- a/cgroupv2.go +++ b/cgroupv2.go @@ -89,12 +89,35 @@ func getFromParentOrFallback[T any]( } func (s cgroupV2Statter) cpuUsed() (used float64, err error) { - cpuStatPath := filepath.Join(s.path, cgroupV2CPUStat) - - usageUs, err := readInt64Prefix(s.fs, cpuStatPath, "usage_usec") + limit, err := s.cpuLimit() if err != nil { - return 0, xerrors.Errorf("get cgroupv2 cpu used: %w", err) + return 0, xerrors.Errorf("get cpu limit: %w", err) + } + + // Read usage from the cgroup that owns the quota so that used and + // total describe the same domain. Otherwise sibling cgroups under + // the limit owner (for example /docker/ next to /init.scope in + // docker-in-docker) would count against the quota but not the usage. + var usageUs int64 + readOwnUsage := true + if limit != nil && limit.owner.path != s.path { + usageUs, err = limit.owner.cpuUsageUs() + switch { + case err == nil: + readOwnUsage = false + case !errors.Is(err, fs.ErrNotExist): + return 0, xerrors.Errorf("read limit owner cpu usage: %w", err) + } + // The limit owner does not expose cpu.stat, so fall back to the + // process's own cgroup below. + } + if readOwnUsage { + usageUs, err = s.cpuUsageUs() + if err != nil { + return 0, err + } } + periodUs, err := s.cpuPeriod() if err != nil { return 0, xerrors.Errorf("get cpu period: %w", err) @@ -103,23 +126,44 @@ func (s cgroupV2Statter) cpuUsed() (used float64, err error) { return float64(usageUs) / periodUs, nil } -func (s cgroupV2Statter) cpuQuota() (float64, error) { +func (s cgroupV2Statter) cpuUsageUs() (int64, error) { + cpuStatPath := filepath.Join(s.path, cgroupV2CPUStat) + + usageUs, err := readInt64Prefix(s.fs, cpuStatPath, "usage_usec") + if err != nil { + return 0, xerrors.Errorf("get cgroupv2 cpu used: %w", err) + } + + return usageUs, nil +} + +// cpuLimit is the effective cpu.max quota and the cgroup that sets it. +// owner is kept so usage can be read from the same cgroup the quota is +// enforced on. +type cpuLimit struct { + owner *cgroupV2Statter + quotaUs float64 +} + +// cpuLimit returns the nearest cgroup, starting at s and walking up, +// that sets a cpu.max quota. It returns nil when no cgroup sets a quota. +func (s *cgroupV2Statter) cpuLimit() (*cpuLimit, error) { cpuMaxPath := filepath.Join(s.path, cgroupV2CPUMax) quotaUs, err := readInt64SepIdx(s.fs, cpuMaxPath, " ", 0) if err != nil { if !errors.Is(err, strconv.ErrSyntax) && !errors.Is(err, fs.ErrNotExist) { - return 0, xerrors.Errorf("get cpu quota: %w", err) + return nil, xerrors.Errorf("get cpu quota: %w", err) } // If the value is not a valid integer, assume it is the string - // 'max' and that there is no limit set. Try the parent or return -1. - return getFromParentOrFallback(&s, (*cgroupV2Statter).cpuQuota, func() (float64, error) { - return -1.0, nil + // 'max' and that there is no limit set. Try the parent or return nil. + return getFromParentOrFallback(s, (*cgroupV2Statter).cpuLimit, func() (*cpuLimit, error) { + return nil, nil }) } - return float64(quotaUs), nil + return &cpuLimit{owner: s, quotaUs: float64(quotaUs)}, nil } func (s cgroupV2Statter) cpuPeriod() (float64, error) { @@ -148,12 +192,15 @@ func (s cgroupV2Statter) cpuTotal() (total float64, err error) { return 0, xerrors.Errorf("get cpu period: %w", err) } - quotaUs, err := s.cpuQuota() + limit, err := s.cpuLimit() if err != nil { return 0, xerrors.Errorf("get cpu quota: %w", err) } + if limit == nil { + return -1.0 / periodUs, nil + } - return quotaUs / periodUs, nil + return limit.quotaUs / periodUs, nil } func (s cgroupV2Statter) memoryMaxBytes() (*float64, error) { diff --git a/stat_internal_test.go b/stat_internal_test.go index cc139de..3a4faea 100644 --- a/stat_internal_test.go +++ b/stat_internal_test.go @@ -440,6 +440,32 @@ func TestStatter(t *testing.T) { assert.Equal(t, "cores", cpu.Unit) }) + t.Run("CPU/UsageFromLimitOwner", func(t *testing.T) { + t.Parallel() + + fs := initFS(t, fsContainerCgroupV2NestedContainerCPU) + fakeWait := func(time.Duration) { + // The root's usage includes both children: 10000us from + // /init.scope and 190000us from the sibling /docker/. + mungeFS(t, fs, filepath.Join(cgroupRootPath, cgroupV2CPUStat), "usage_usec 200000") + mungeFS(t, fs, filepath.Join(cgroupRootPath, "init.scope", cgroupV2CPUStat), "usage_usec 10000") + mungeFS(t, fs, filepath.Join(cgroupRootPath, cgroupV2Path, cgroupV2CPUStat), "usage_usec 190000") + } + s, err := New(WithFS(fs), withWait(fakeWait), withIsCgroupV2(true)) + require.NoError(t, err) + + cpu, err := s.ContainerCPU() + require.NoError(t, err) + + require.NotNil(t, cpu) + // Usage comes from the root that owns cpu.max, which + // includes the sibling /docker/ cgroup. + assert.Equal(t, 2.0, cpu.Used) + require.NotNil(t, cpu.Total) + assert.Equal(t, 2.5, *cpu.Total) + assert.Equal(t, "cores", cpu.Unit) + }) + t.Run("Memory/Limit", func(t *testing.T) { t.Parallel() @@ -819,6 +845,25 @@ sysboxfs /proc/sys sysboxfs rw,nosuid,nodev,noexec,relatime 0 0`, filepath.Join(cgroupRootPath, "init.scope", cgroupV2MemoryStat): "inactive_file 268435456", filepath.Join(cgroupRootPath, "init.scope", cgroupV2MemoryUsageBytes): "536870912", } + // fsContainerCgroupV2NestedContainerCPU simulates a container that runs + // nested containers. The process runs in /init.scope and a nested + // container runs in the sibling /docker/. Only the container root + // sets cpu.max, so usage must be read from the root to include both + // children. + fsContainerCgroupV2NestedContainerCPU = map[string]string{ + procOneCgroup: "0::/init.scope", + procSelfCgroup: "0::/init.scope", + procMounts: `overlay / overlay rw,relatime,lowerdir=/some/path:/some/path,upperdir=/some/path:/some/path,workdir=/some/path:/some/path 0 0 +proc /proc/sys proc ro,nosuid,nodev,noexec,relatime 0 0`, + sysCgroupType: "domain", + + filepath.Join(cgroupRootPath, cgroupV2CPUMax): "250000 100000", + filepath.Join(cgroupRootPath, cgroupV2CPUStat): "usage_usec 0", + filepath.Join(cgroupRootPath, "init.scope", cgroupV2CPUMax): "max 100000", + filepath.Join(cgroupRootPath, "init.scope", cgroupV2CPUStat): "usage_usec 0", + filepath.Join(cgroupRootPath, cgroupV2Path, cgroupV2CPUMax): "max 100000", + filepath.Join(cgroupRootPath, cgroupV2Path, cgroupV2CPUStat): "usage_usec 0", + } // fsContainerCgroupV2InitScopeNoCPUMax simulates a scenario where cpu.max // doesn't exist at any level in the hierarchy. Tests the default period fallback. fsContainerCgroupV2InitScopeNoCPUMax = map[string]string{ From 1c39b412f5bc278c71026c5cc13f474a1d3519ca Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Fri, 23 Oct 2026 21:04:52 +0000 Subject: [PATCH 2/2] refactor: match cpu limit owner code to memory implementation --- cgroupv2.go | 69 +++++++++++++++++++++---------------------- stat_internal_test.go | 19 ++++++------ 2 files changed, 43 insertions(+), 45 deletions(-) diff --git a/cgroupv2.go b/cgroupv2.go index ecff41f..add70ce 100644 --- a/cgroupv2.go +++ b/cgroupv2.go @@ -89,56 +89,42 @@ func getFromParentOrFallback[T any]( } func (s cgroupV2Statter) cpuUsed() (used float64, err error) { + periodUs, err := s.cpuPeriod() + if err != nil { + return 0, xerrors.Errorf("get cpu period: %w", err) + } + limit, err := s.cpuLimit() if err != nil { - return 0, xerrors.Errorf("get cpu limit: %w", err) + return 0, xerrors.Errorf("get cpu quota: %w", err) } - // Read usage from the cgroup that owns the quota so that used and + // Read usage from the cgroup that owns the limit so that used and // total describe the same domain. Otherwise sibling cgroups under // the limit owner (for example /docker/ next to /init.scope in - // docker-in-docker) would count against the quota but not the usage. - var usageUs int64 - readOwnUsage := true + // docker-in-docker) would count against the limit but not the usage. if limit != nil && limit.owner.path != s.path { - usageUs, err = limit.owner.cpuUsageUs() - switch { - case err == nil: - readOwnUsage = false - case !errors.Is(err, fs.ErrNotExist): - return 0, xerrors.Errorf("read limit owner cpu usage: %w", err) - } - // The limit owner does not expose cpu.stat, so fall back to the - // process's own cgroup below. - } - if readOwnUsage { - usageUs, err = s.cpuUsageUs() - if err != nil { - return 0, err + if usedUs, err := limit.owner.cpuUsedUs(); err != nil { + if !errors.Is(err, fs.ErrNotExist) { + return 0, xerrors.Errorf("read limit owner cpu: %w", err) + } + // The limit owner does not expose usage files, so fall + // back to the process's own cgroup below. + } else { + return float64(usedUs) / periodUs, nil } } - periodUs, err := s.cpuPeriod() + usedUs, err := s.cpuUsedUs() if err != nil { - return 0, xerrors.Errorf("get cpu period: %w", err) + return 0, err } - return float64(usageUs) / periodUs, nil -} - -func (s cgroupV2Statter) cpuUsageUs() (int64, error) { - cpuStatPath := filepath.Join(s.path, cgroupV2CPUStat) - - usageUs, err := readInt64Prefix(s.fs, cpuStatPath, "usage_usec") - if err != nil { - return 0, xerrors.Errorf("get cgroupv2 cpu used: %w", err) - } - - return usageUs, nil + return float64(usedUs) / periodUs, nil } // cpuLimit is the effective cpu.max quota and the cgroup that sets it. -// owner is kept so usage can be read from the same cgroup the quota is +// owner is kept so usage can be read from the same cgroup the limit is // enforced on. type cpuLimit struct { owner *cgroupV2Statter @@ -146,7 +132,7 @@ type cpuLimit struct { } // cpuLimit returns the nearest cgroup, starting at s and walking up, -// that sets a cpu.max quota. It returns nil when no cgroup sets a quota. +// that sets a cpu.max quota. It returns nil when no cgroup sets a limit. func (s *cgroupV2Statter) cpuLimit() (*cpuLimit, error) { cpuMaxPath := filepath.Join(s.path, cgroupV2CPUMax) @@ -157,7 +143,7 @@ func (s *cgroupV2Statter) cpuLimit() (*cpuLimit, error) { } // If the value is not a valid integer, assume it is the string - // 'max' and that there is no limit set. Try the parent or return nil. + // 'max' and that there is no limit set. Try the parent or return nil (no limit). return getFromParentOrFallback(s, (*cgroupV2Statter).cpuLimit, func() (*cpuLimit, error) { return nil, nil }) @@ -166,6 +152,17 @@ func (s *cgroupV2Statter) cpuLimit() (*cpuLimit, error) { return &cpuLimit{owner: s, quotaUs: float64(quotaUs)}, nil } +func (s cgroupV2Statter) cpuUsedUs() (int64, error) { + cpuStatPath := filepath.Join(s.path, cgroupV2CPUStat) + + usageUs, err := readInt64Prefix(s.fs, cpuStatPath, "usage_usec") + if err != nil { + return 0, xerrors.Errorf("get cgroupv2 cpu used: %w", err) + } + + return usageUs, nil +} + func (s cgroupV2Statter) cpuPeriod() (float64, error) { cpuMaxPath := filepath.Join(s.path, cgroupV2CPUMax) diff --git a/stat_internal_test.go b/stat_internal_test.go index 3a4faea..dc246c2 100644 --- a/stat_internal_test.go +++ b/stat_internal_test.go @@ -443,7 +443,7 @@ func TestStatter(t *testing.T) { t.Run("CPU/UsageFromLimitOwner", func(t *testing.T) { t.Parallel() - fs := initFS(t, fsContainerCgroupV2NestedContainerCPU) + fs := initFS(t, fsContainerCgroupV2DockerInDockerCPU) fakeWait := func(time.Duration) { // The root's usage includes both children: 10000us from // /init.scope and 190000us from the sibling /docker/. @@ -845,22 +845,23 @@ sysboxfs /proc/sys sysboxfs rw,nosuid,nodev,noexec,relatime 0 0`, filepath.Join(cgroupRootPath, "init.scope", cgroupV2MemoryStat): "inactive_file 268435456", filepath.Join(cgroupRootPath, "init.scope", cgroupV2MemoryUsageBytes): "536870912", } - // fsContainerCgroupV2NestedContainerCPU simulates a container that runs - // nested containers. The process runs in /init.scope and a nested - // container runs in the sibling /docker/. Only the container root - // sets cpu.max, so usage must be read from the root to include both - // children. - fsContainerCgroupV2NestedContainerCPU = map[string]string{ + // fsContainerCgroupV2DockerInDockerCPU simulates a docker-in-docker workspace. + // The agent runs in /init.scope and a nested container runs in the + // sibling /docker/. Only the container root sets cpu.max, so + // usage must be read from the root to include both children. + fsContainerCgroupV2DockerInDockerCPU = map[string]string{ procOneCgroup: "0::/init.scope", procSelfCgroup: "0::/init.scope", procMounts: `overlay / overlay rw,relatime,lowerdir=/some/path:/some/path,upperdir=/some/path:/some/path,workdir=/some/path:/some/path 0 0 proc /proc/sys proc ro,nosuid,nodev,noexec,relatime 0 0`, sysCgroupType: "domain", - filepath.Join(cgroupRootPath, cgroupV2CPUMax): "250000 100000", - filepath.Join(cgroupRootPath, cgroupV2CPUStat): "usage_usec 0", + filepath.Join(cgroupRootPath, cgroupV2CPUMax): "250000 100000", + filepath.Join(cgroupRootPath, cgroupV2CPUStat): "usage_usec 0", + filepath.Join(cgroupRootPath, "init.scope", cgroupV2CPUMax): "max 100000", filepath.Join(cgroupRootPath, "init.scope", cgroupV2CPUStat): "usage_usec 0", + filepath.Join(cgroupRootPath, cgroupV2Path, cgroupV2CPUMax): "max 100000", filepath.Join(cgroupRootPath, cgroupV2Path, cgroupV2CPUStat): "usage_usec 0", }