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

Container: Do not set "soft" limit when hard limit is set for cgroupv2 #13192

Merged
merged 1 commit into from
Mar 22, 2024

Conversation

mihalicyn
Copy link
Member

In cgroup-v1 memcg implementation we have a concept of "soft" memory limit (memory.soft_limit_in_bytes). In cgroup-v2 memcg we don't have an explicit analog of it. Originally, when hard memory limit is set, we set it value to memory.limit_in_bytes and set 90% of its value to memory.soft_limit_in_bytes. And it was working quite well. But in cgroup-v2 we tried to treat memory.high as analogy to memory.soft_limit_in_bytes which is not fully correct. Yes, reaching memory.high limit does not invoke OOM-killer, at the same time it forces process to go into memory reclaim on each page fault that want's to charge pages in memcg which makes its execution terribly slow. Let's stop doing that. If hard limit is set, let's set only memory.max value and do nothing with memory.high. But if soft limit is set, then we have not so many options and let's stick with setting memory.high value.

Fixes: #13147

…is set

In cgroup-v1 memcg implementation we have a concept of "soft" memory limit
(`memory.soft_limit_in_bytes`). In cgroup-v2 memcg we don't have an explicit
analog of it. Originally, when hard memory limit is set, we
set it value to `memory.limit_in_bytes` and set 90% of its value to
`memory.soft_limit_in_bytes`. And it was working quite well. But in cgroup-v2
we tried to treat `memory.high` as analogy to `memory.soft_limit_in_bytes`
which is not fully correct. Yes, reaching `memory.high` limit does not
invoke OOM-killer, at the same time it forces process to go into memory
reclaim on each page fault that want's to charge pages in memcg which makes
its execution terribly slow. Let's stop doing that. If hard limit is set,
let's set only `memory.max` value and do nothing with `memory.high`.
But if soft limit is set, then we have not so many options and let's stick
with setting `memory.high` value.

Fixes: canonical#13147

Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
Copy link
Member

@simondeziel simondeziel left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@simondeziel
Copy link
Member

@mihalicyn how hard would it be to add tests for that? Our GHA runners are all cgroup2 ready.

@tomponline
Copy link
Member

yeah we should add a test

mihalicyn added a commit to mihalicyn/lxd-ci that referenced this pull request Mar 21, 2024
canonical/lxd#13192

Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
mihalicyn added a commit to mihalicyn/lxd-ci that referenced this pull request Mar 21, 2024
canonical/lxd#13192

Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
mihalicyn added a commit to mihalicyn/lxd-ci that referenced this pull request Mar 21, 2024
canonical/lxd#13192

Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
mihalicyn added a commit to mihalicyn/lxd-ci that referenced this pull request Mar 21, 2024
canonical/lxd#13192

Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
@mihalicyn
Copy link
Member Author

yeah we should add a test

Test is ready canonical/lxd-ci#107 ;-)

Copy link
Member

@tomponline tomponline left a comment

Choose a reason for hiding this comment

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

Thanks!

@tomponline tomponline merged commit 94da3fe into canonical:main Mar 22, 2024
28 checks passed
@tomponline tomponline changed the title drivers/driver_lxc: do not set "soft" limit when hard limit is set Container: Do not set "soft" limit when hard limit is set for cgroupv2 Mar 22, 2024
mihalicyn added a commit to mihalicyn/lxd-ci that referenced this pull request Mar 22, 2024
canonical/lxd#13192

Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
mihalicyn added a commit to mihalicyn/lxd-ci that referenced this pull request Mar 22, 2024
canonical/lxd#13192

Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
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 this pull request may close these issues.

limits.memory are not set correctly in cgroup2
3 participants