libct/cg/stats: support PSI for cgroup v2 #3900
Conversation
|
The full diff from PR #3679 is here: #3679 (comment) |
kolyshkin
force-pushed
the
psi
branch
2 times, most recently
from
9b47353
to
22836b3
Compare
CentOS 9 kernel returns ENOTSUP on read() not open(). The fix is: diff --git a/libcontainer/cgroups/fs2/psi.go b/libcontainer/cgroups/fs2/psi.go
index 603c6eaf..0d5f1d7a 100644
--- a/libcontainer/cgroups/fs2/psi.go
+++ b/libcontainer/cgroups/fs2/psi.go
@@ -16,12 +16,9 @@ import (
func statPSI(dirPath string, file string) (*cgroups.PSIStats, error) {
f, err := cgroups.OpenFile(dirPath, file, os.O_RDONLY)
if err != nil {
- if errors.Is(err, os.ErrNotExist) || errors.Is(err, unix.ENOTSUP) {
- // Open *.pressure file returns:
- // - ErrNotExist when kernel < 4.20 or when CONFIG_PSI is disabled;
- // - ENOTSUP when psi=1 kernel cmdline is required for PSI support.
- // In both cases, stats are not available, so return nil.
- err = nil
+ if errors.Is(err, os.ErrNotExist) {
+ // Kernel < 4.20, or CONFIG_PSI is not set.
+ return nil, nil
}
return nil, err
}
@@ -46,6 +43,11 @@ func statPSI(dirPath string, file string) (*cgroups.PSIStats, error) {
}
}
if err := sc.Err(); err != nil {
+ if errors.Is(err, unix.ENOTSUP) {
+ // Some kernels (e.g. CS9) may return ENOTSUP on read
+ // if psi=1 kernel cmdline parameter is required.
+ return nil, nil
+ }
return nil, &parseError{Path: dirPath, File: file, Err: err}
}
return &psistats, nil |
|
Thanks for the work. The change looks pretty good to me ! |
|
Thanks @kolyshkin ! |
We read output from the following files if they exists: - cpu.pressure - memory.pressure - io.pressure Each are in format: ``` some avg10=0.00 avg60=0.00 avg300=0.00 total=0 full avg10=0.00 avg60=0.00 avg300=0.00 total=0 ``` Signed-off-by: Daniel Dao <dqminh89@gmail.com> Co-authored-by: Sandor Szücs <sandor.szuecs@zalando.de> Co-authored-by: Kir Kolyshkin <kolyshkin@gmail.com> Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
|
Thanks all, and @kolyshkin for getting this into ready state! It looks good to me, would be great to get this in so we can make use of it it in cAdvisor, and later in k8s. |
|
@opencontainers/runc-maintainers PTAL |
1 similar comment
|
@opencontainers/runc-maintainers PTAL |
| @@ -114,6 +114,17 @@ func (m *Manager) GetStats() (*cgroups.Stats, error) { | |||
| if err := statCpu(m.dirPath, st); err != nil && !os.IsNotExist(err) { | |||
| errs = append(errs, err) | |||
| } | |||
| // PSI (since kernel 4.20). | |||
| var err error | |||
There was a problem hiding this comment.
@kolyshkin I think this one could be optimized. We should write their own PSI state function for cpu, memory and io. For example: add statCpuPressure in cpu.go, add statMemoryPressure in memory.go, add statIOPressure in io.go. Then, we can keep the code style like statCpu.
But this one could be done in a seperate PR if the upstream projects are very anxious to need this feature.
|
The code looks good now. Let's go ahead, and test it in practice, welcome contibutors/maintainers to do some refactor jobs if need. |
|
Thanks everyone for pushing this one over the line !!! 🥳 |
This is a carry over of #3679, adding support for PSI stats for cgroup v2 to cgroup manager' Stats(), as well as `runc events'. Mostly written by @dqminh, amended by @szuecs and @kolyshkin.
Closes: #3679
Closes: #3358
Closes: #3627