From f27d2334d79fa6779e8797c0598a8e13e8ee471d Mon Sep 17 00:00:00 2001 From: jay-mckay Date: Tue, 3 Sep 2024 13:22:00 -0600 Subject: [PATCH 1/2] Read cpu.stat regardless if controller enabled. The unified hierarchy provides the cpu.stat file for every cgroup, regardless if the CPU controller is enabled (in fact, setting the systemd property CPUAccounting=True does not enable this controller because of this fact). It provides the usage_usec, user_usec, and system_usec by default. Instead of reading the stat for each enabled controller (CPU and memory), just attempt to read them each time the Stat() function is called. Attempting to read the memory.stat file even if memory accounting is not enabled seems insignificant (some other files always have a read attempt, such as memory.current), and eliminates finding and looping over enabled controllers. Resolves: #347 Signed-off-by: Jackson McKay --- cgroup2/manager.go | 118 +++++++++++++++++++++------------------------ 1 file changed, 54 insertions(+), 64 deletions(-) diff --git a/cgroup2/manager.go b/cgroup2/manager.go index b446939d..0b50a400 100644 --- a/cgroup2/manager.go +++ b/cgroup2/manager.go @@ -550,78 +550,61 @@ func (c *Manager) MoveTo(destination *Manager) error { } func (c *Manager) Stat() (*stats.Metrics, error) { - controllers, err := c.Controllers() - if err != nil { - return nil, err - } - // Sizing this avoids an allocation to increase the map at runtime; - // currently the default bucket size is 8 and we put 40+ elements - // in it so we'd always end up allocating. - out := make(map[string]uint64, 50) - for _, controller := range controllers { - switch controller { - case "cpu", "memory": - if err := readKVStatsFile(c.path, controller+".stat", out); err != nil { - if os.IsNotExist(err) { - continue - } - return nil, err - } - } - } - memoryEvents := make(map[string]uint64) - if err := readKVStatsFile(c.path, "memory.events", memoryEvents); err != nil { + var metrics stats.Metrics + + cpuStat := make(map[string]uint64) + if err := readKVStatsFile(c.path, "cpu.stat", cpuStat); err != nil { if !os.IsNotExist(err) { return nil, err } } - - var metrics stats.Metrics - metrics.Pids = &stats.PidsStat{ - Current: getStatFileContentUint64(filepath.Join(c.path, "pids.current")), - Limit: getStatFileContentUint64(filepath.Join(c.path, "pids.max")), - } metrics.CPU = &stats.CPUStat{ - UsageUsec: out["usage_usec"], - UserUsec: out["user_usec"], - SystemUsec: out["system_usec"], - NrPeriods: out["nr_periods"], - NrThrottled: out["nr_throttled"], - ThrottledUsec: out["throttled_usec"], + UsageUsec: cpuStat["usage_usec"], + UserUsec: cpuStat["user_usec"], + SystemUsec: cpuStat["system_usec"], + NrPeriods: cpuStat["nr_periods"], + NrThrottled: cpuStat["nr_throttled"], + ThrottledUsec: cpuStat["throttled_usec"], PSI: getStatPSIFromFile(filepath.Join(c.path, "cpu.pressure")), } + memoryStat := make(map[string]uint64, 40) + if err := readKVStatsFile(c.path, "memory.stat", memoryStat); err != nil { + if !os.IsNotExist(err) { + return nil, err + } + } metrics.Memory = &stats.MemoryStat{ - Anon: out["anon"], - File: out["file"], - KernelStack: out["kernel_stack"], - Slab: out["slab"], - Sock: out["sock"], - Shmem: out["shmem"], - FileMapped: out["file_mapped"], - FileDirty: out["file_dirty"], - FileWriteback: out["file_writeback"], - AnonThp: out["anon_thp"], - InactiveAnon: out["inactive_anon"], - ActiveAnon: out["active_anon"], - InactiveFile: out["inactive_file"], - ActiveFile: out["active_file"], - Unevictable: out["unevictable"], - SlabReclaimable: out["slab_reclaimable"], - SlabUnreclaimable: out["slab_unreclaimable"], - Pgfault: out["pgfault"], - Pgmajfault: out["pgmajfault"], - WorkingsetRefault: out["workingset_refault"], - WorkingsetActivate: out["workingset_activate"], - WorkingsetNodereclaim: out["workingset_nodereclaim"], - Pgrefill: out["pgrefill"], - Pgscan: out["pgscan"], - Pgsteal: out["pgsteal"], - Pgactivate: out["pgactivate"], - Pgdeactivate: out["pgdeactivate"], - Pglazyfree: out["pglazyfree"], - Pglazyfreed: out["pglazyfreed"], - ThpFaultAlloc: out["thp_fault_alloc"], - ThpCollapseAlloc: out["thp_collapse_alloc"], + Anon: memoryStat["anon"], + File: memoryStat["file"], + KernelStack: memoryStat["kernel_stack"], + Slab: memoryStat["slab"], + Sock: memoryStat["sock"], + Shmem: memoryStat["shmem"], + FileMapped: memoryStat["file_mapped"], + FileDirty: memoryStat["file_dirty"], + FileWriteback: memoryStat["file_writeback"], + AnonThp: memoryStat["anon_thp"], + InactiveAnon: memoryStat["inactive_anon"], + ActiveAnon: memoryStat["active_anon"], + InactiveFile: memoryStat["inactive_file"], + ActiveFile: memoryStat["active_file"], + Unevictable: memoryStat["unevictable"], + SlabReclaimable: memoryStat["slab_reclaimable"], + SlabUnreclaimable: memoryStat["slab_unreclaimable"], + Pgfault: memoryStat["pgfault"], + Pgmajfault: memoryStat["pgmajfault"], + WorkingsetRefault: memoryStat["workingset_refault"], + WorkingsetActivate: memoryStat["workingset_activate"], + WorkingsetNodereclaim: memoryStat["workingset_nodereclaim"], + Pgrefill: memoryStat["pgrefill"], + Pgscan: memoryStat["pgscan"], + Pgsteal: memoryStat["pgsteal"], + Pgactivate: memoryStat["pgactivate"], + Pgdeactivate: memoryStat["pgdeactivate"], + Pglazyfree: memoryStat["pglazyfree"], + Pglazyfreed: memoryStat["pglazyfreed"], + ThpFaultAlloc: memoryStat["thp_fault_alloc"], + ThpCollapseAlloc: memoryStat["thp_collapse_alloc"], Usage: getStatFileContentUint64(filepath.Join(c.path, "memory.current")), UsageLimit: getStatFileContentUint64(filepath.Join(c.path, "memory.max")), MaxUsage: getStatFileContentUint64(filepath.Join(c.path, "memory.peak")), @@ -630,6 +613,13 @@ func (c *Manager) Stat() (*stats.Metrics, error) { SwapMaxUsage: getStatFileContentUint64(filepath.Join(c.path, "memory.swap.peak")), PSI: getStatPSIFromFile(filepath.Join(c.path, "memory.pressure")), } + + memoryEvents := make(map[string]uint64) + if err := readKVStatsFile(c.path, "memory.events", memoryEvents); err != nil { + if !os.IsNotExist(err) { + return nil, err + } + } if len(memoryEvents) > 0 { metrics.MemoryEvents = &stats.MemoryEvents{ Low: memoryEvents["low"], From 40b4a5fb2c1228c4877375498aa60073df92cd36 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 11 Mar 2025 17:41:04 +0100 Subject: [PATCH 2/2] cgroups2/Stats: split out reading cpu, memory, memory events (#1) As outlined in the PR, usage_usec, user_usec, and system_use may always exist, but in situations where the files parsed do not exist, we shouldn't have to try to assign values (which would always be 0). Split out parsing of these to separate functions, so make this more transparent. Signed-off-by: Sebastiaan van Stijn --- cgroup2/manager.go | 103 ++++++++++++++++++++++++++++----------------- 1 file changed, 65 insertions(+), 38 deletions(-) diff --git a/cgroup2/manager.go b/cgroup2/manager.go index 0b50a400..6a4f2727 100644 --- a/cgroup2/manager.go +++ b/cgroup2/manager.go @@ -552,28 +552,63 @@ func (c *Manager) MoveTo(destination *Manager) error { func (c *Manager) Stat() (*stats.Metrics, error) { var metrics stats.Metrics + var err error + metrics.CPU, err = readCPUStats(c.path) + if err != nil { + return nil, err + } + + metrics.Memory, err = readMemoryStats(c.path) + if err != nil { + return nil, err + } + + metrics.MemoryEvents, err = readMemoryEvents(c.path) + if err != nil { + return nil, err + } + + metrics.Io = &stats.IOStat{ + Usage: readIoStats(c.path), + PSI: getStatPSIFromFile(filepath.Join(c.path, "io.pressure")), + } + metrics.Rdma = &stats.RdmaStat{ + Current: rdmaStats(filepath.Join(c.path, "rdma.current")), + Limit: rdmaStats(filepath.Join(c.path, "rdma.max")), + } + metrics.Hugetlb = readHugeTlbStats(c.path) + + return &metrics, nil +} + +func readCPUStats(cgroupPath string) (*stats.CPUStat, error) { cpuStat := make(map[string]uint64) - if err := readKVStatsFile(c.path, "cpu.stat", cpuStat); err != nil { - if !os.IsNotExist(err) { - return nil, err + if err := readKVStatsFile(cgroupPath, "cpu.stat", cpuStat); err != nil { + if os.IsNotExist(err) { + return &stats.CPUStat{}, nil } + return nil, err } - metrics.CPU = &stats.CPUStat{ + return &stats.CPUStat{ UsageUsec: cpuStat["usage_usec"], UserUsec: cpuStat["user_usec"], SystemUsec: cpuStat["system_usec"], NrPeriods: cpuStat["nr_periods"], NrThrottled: cpuStat["nr_throttled"], ThrottledUsec: cpuStat["throttled_usec"], - PSI: getStatPSIFromFile(filepath.Join(c.path, "cpu.pressure")), - } + PSI: getStatPSIFromFile(filepath.Join(cgroupPath, "cpu.pressure")), + }, nil +} + +func readMemoryStats(cgroupPath string) (*stats.MemoryStat, error) { memoryStat := make(map[string]uint64, 40) - if err := readKVStatsFile(c.path, "memory.stat", memoryStat); err != nil { - if !os.IsNotExist(err) { - return nil, err + if err := readKVStatsFile(cgroupPath, "memory.stat", memoryStat); err != nil { + if os.IsNotExist(err) { + return &stats.MemoryStat{}, nil } + return nil, err } - metrics.Memory = &stats.MemoryStat{ + return &stats.MemoryStat{ Anon: memoryStat["anon"], File: memoryStat["file"], KernelStack: memoryStat["kernel_stack"], @@ -605,41 +640,33 @@ func (c *Manager) Stat() (*stats.Metrics, error) { Pglazyfreed: memoryStat["pglazyfreed"], ThpFaultAlloc: memoryStat["thp_fault_alloc"], ThpCollapseAlloc: memoryStat["thp_collapse_alloc"], - Usage: getStatFileContentUint64(filepath.Join(c.path, "memory.current")), - UsageLimit: getStatFileContentUint64(filepath.Join(c.path, "memory.max")), - MaxUsage: getStatFileContentUint64(filepath.Join(c.path, "memory.peak")), - SwapUsage: getStatFileContentUint64(filepath.Join(c.path, "memory.swap.current")), - SwapLimit: getStatFileContentUint64(filepath.Join(c.path, "memory.swap.max")), - SwapMaxUsage: getStatFileContentUint64(filepath.Join(c.path, "memory.swap.peak")), - PSI: getStatPSIFromFile(filepath.Join(c.path, "memory.pressure")), - } + Usage: getStatFileContentUint64(filepath.Join(cgroupPath, "memory.current")), + UsageLimit: getStatFileContentUint64(filepath.Join(cgroupPath, "memory.max")), + MaxUsage: getStatFileContentUint64(filepath.Join(cgroupPath, "memory.peak")), + SwapUsage: getStatFileContentUint64(filepath.Join(cgroupPath, "memory.swap.current")), + SwapLimit: getStatFileContentUint64(filepath.Join(cgroupPath, "memory.swap.max")), + SwapMaxUsage: getStatFileContentUint64(filepath.Join(cgroupPath, "memory.swap.peak")), + PSI: getStatPSIFromFile(filepath.Join(cgroupPath, "memory.pressure")), + }, nil +} +func readMemoryEvents(cgroupPath string) (*stats.MemoryEvents, error) { memoryEvents := make(map[string]uint64) - if err := readKVStatsFile(c.path, "memory.events", memoryEvents); err != nil { + if err := readKVStatsFile(cgroupPath, "memory.events", memoryEvents); err != nil { if !os.IsNotExist(err) { return nil, err } } - if len(memoryEvents) > 0 { - metrics.MemoryEvents = &stats.MemoryEvents{ - Low: memoryEvents["low"], - High: memoryEvents["high"], - Max: memoryEvents["max"], - Oom: memoryEvents["oom"], - OomKill: memoryEvents["oom_kill"], - } + if len(memoryEvents) == 0 { + return nil, nil } - metrics.Io = &stats.IOStat{ - Usage: readIoStats(c.path), - PSI: getStatPSIFromFile(filepath.Join(c.path, "io.pressure")), - } - metrics.Rdma = &stats.RdmaStat{ - Current: rdmaStats(filepath.Join(c.path, "rdma.current")), - Limit: rdmaStats(filepath.Join(c.path, "rdma.max")), - } - metrics.Hugetlb = readHugeTlbStats(c.path) - - return &metrics, nil + return &stats.MemoryEvents{ + Low: memoryEvents["low"], + High: memoryEvents["high"], + Max: memoryEvents["max"], + Oom: memoryEvents["oom"], + OomKill: memoryEvents["oom_kill"], + }, nil } func readKVStatsFile(path string, file string, out map[string]uint64) error {