Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cgroup v2 root stats should not be calculated as v1 #4059

Open
linxiulei opened this issue Oct 5, 2023 · 3 comments · May be fixed by #4123
Open

cgroup v2 root stats should not be calculated as v1 #4059

linxiulei opened this issue Oct 5, 2023 · 3 comments · May be fixed by #4123

Comments

@linxiulei
Copy link

linxiulei commented Oct 5, 2023

Description

From the comments in https://github.com/opencontainers/runc/blob/main/libcontainer/cgroups/fs2/memory.go#L218, it appears the root stats of cgroup v2 was collected in a way to be similar to how v1 is implemented. My understanding, root stats v2 should be collected as memory.current in non-root cgroups, which is supposed to be total memory as opposed to just anon + file.

This issue causes missing a visible amount memory not counted in Usage (e.g. kernel memory) and is misleading. I propose using the value from /proc/meminfo to get the total memory usage for root stat.

Steps to reproduce the issue

  1. run a kubelet
  2. curl localhost:10255/stats/summary to check the root stats
  3. stats got from step 2 do not match the system memory usage

Describe the results you received and expected

root stats' usageBytes should match system memory usage

What version of runc are you using?

runc version 1.1.7
commit: a916309
spec: 1.0.2-dev
go: go1.20.5
libseccomp: 2.5.4

Host OS information

No response

Host kernel information

No response

@Zheaoli
Copy link
Contributor

Zheaoli commented Nov 15, 2023

I think this maybe make sense.

For now, we use /proc/meminfo to calculate the swap info, and memory.stat to calculate the usage info. I think this may be strange. Perhaps we should calculate the memory info for the same place

cc @lifubang

@Zheaoli
Copy link
Contributor

Zheaoli commented Nov 20, 2023

ping @lifubang @AkihiroSuda (

@kolyshkin
Copy link
Contributor

@linxiulei @Zheaoli we have changed this code a few times already to please k8s. Feel free to open a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants