memory: remove wrong memory.kmem.limit_in_bytes check#175
Merged
estesp merged 1 commit intocontainerd:masterfrom Aug 17, 2020
Merged
Conversation
Snorch
force-pushed
the
memory-remove-wrong-kernel-limit-check
branch
from
6f3061b to
9f25424
Compare
This check is useless as:
"We have to limit the kernel memory here as it won't be accounted at all
until a limit is set on the cgroup" - if kmem accounting is enabled on
boot (no nokmem) kernel memory would be accounted without setting the
limit.
"limit cannot be set once the cgroup has children, or if there are
already tasks in the cgroup." - It can, the only restriction it should
be > usage, but setting this limit < future usage is a bad thing itself.
So there is no difference if we set this limit on an active cgroup.
More over memory.kmem.limit_in_bytes is deprecated in mainstream linux
kernel as it was considered unreliable, see more information in:
commit 0158115f702b ("memcg, kmem: deprecate kmem.limit_in_bytes")
Link: torvalds/linux@0158115f702b
Signed-off-by: Pavel Tikhomirov <[email protected]>
Snorch
force-pushed
the
memory-remove-wrong-kernel-limit-check
branch
from
9f25424 to
4029019
Compare
AkihiroSuda
approved these changes
Jul 24, 2020
Contributor
Author
|
Are there any news? Can it be merged? |
Member
|
Sorry needs one more LGTM. Probably after v1.4 (soon) |
Member
Contributor
Author
|
Thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This check is useless as:
"We have to limit the kernel memory here as it won't be accounted at all
until a limit is set on the cgroup" - if kmem accounting is enabled on
boot (no nokmem) kernel memory would be accounted without setting the
limit.
"limit cannot be set once the cgroup has children, or if there are
already tasks in the cgroup." - It can, the only restriction it should
be > usage, but setting this limit < future usage is a bad thing itself.
So there is no difference when to set this limit.
More over memory.kmem.limit_in_bytes is deprecated in mainstream linux
kernel as it was considered unreliable, see more information in:
commit 0158115f702b ("memcg, kmem: deprecate kmem.limit_in_bytes")
Link: torvalds/linux@0158115f702b
Signed-off-by: Pavel Tikhomirov [email protected]