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

[1.1] libct/cg/fs2: use file + anon + swap for usage #3977

Merged
merged 1 commit into from
Aug 10, 2023

Conversation

haircommander
Copy link
Contributor

@haircommander haircommander commented Aug 9, 2023

Backport of #3933 by @alexeldeib to release-1.1 branch. Original description follows.


This aligns v2 usage calculations more closely with v1. Current node-level reporting for v1 vs v2 on the same machine under similar load may differ by ~250-750Mi.

Also return usage as combined swap + memory usage, aligned with v1 and non-root v2 cgroups.

mem_cgroup_usage in the kernel counts NR_FILE_PAGES

  • NR_ANON_MAPPED + nr_swap_pages (if swap enabled) 1.

Using total - free results in higher "usage" numbers. This is likely due to various types of reclaimable memory technically counted as in use (e.g. inactive anon).

See also kubernetes/kubernetes#118916 for more context

Footnotes

  1. https://github.com/torvalds/linux/blob/06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5/mm/memcontrol.c#L3673-L3680

@haircommander
Copy link
Contributor Author

cc @mrunalp @kolyshkin

@haircommander
Copy link
Contributor Author

@AkihiroSuda I know you had hesitation on the other PR, but I think the differences qualify as a bug and that justifies including it in 1.1 (especially so kubernetes can vendor it before 1.2 is released)

@kolyshkin kolyshkin added this to the 1.1.9 milestone Aug 9, 2023
@kolyshkin kolyshkin added the backport/1.1-pr A backport PR to release-1.1 label Aug 9, 2023
Copy link
Contributor

@kolyshkin kolyshkin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Yes, I think this backport is needed, as it solves a real problem.

@kolyshkin
Copy link
Contributor

Let's merge it after #3976 so we can run CI on go 1.21.

This aligns v2 usage calculations more closely with v1.
Current node-level reporting for v1 vs v2 on the same
machine under similar load may differ by ~250-750Mi.

Also return usage as combined swap + memory usage, aligned
with v1 and non-root v2 cgroups.

`mem_cgroup_usage` in the kernel counts NR_FILE_PAGES
+ NR_ANON_MAPPED + `nr_swap_pages` (if swap enabled) [^0].

Using total - free results in higher "usage" numbers.
This is likely due to various types of reclaimable
memory technically counted as in use (e.g. inactive anon).

See also kubernetes/kubernetes#118916 for more context

[^0]: https://github.com/torvalds/linux/blob/06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5/mm/memcontrol.c#L3673-L3680

Signed-off-by: Alexander Eldeib <alexeldeib@gmail.com>
@kolyshkin
Copy link
Contributor

Rebased on top of #3976 to check against updated CI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.1-pr A backport PR to release-1.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants