Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

kernel/configs: enable swap extension (CONFIG_MEMCG_SWAP) #104

Merged
merged 1 commit into from
Aug 2, 2018

Conversation

devimc
Copy link

@devimc devimc commented Jul 20, 2018

libcontainer limits the memory+swap usage by writing the limit at
/sys/fs/cgroup/memory/docker/$CONTID/memory.memsw.limit_in_bytes, this path
doesn't exist if CONFIG_MEMCG_SWAP and CONFIG_MEMCG_SWAP_ENABLED are not
enabled.

fixes #103

Signed-off-by: Julio Montes [email protected]

@devimc devimc force-pushed the topic/fixMemConstraints branch from 38cb143 to 6d617f4 Compare July 20, 2018 17:31
@grahamwhaley
Copy link
Contributor

It somehow feels sort of wrong, enabling SWAP in the kernel when we have no swap devices in the container.
For reference, on all of my 'default out of the box' Ubuntu machines I believe, when I run host side something link docker run --rm -ti -m 8G busybox sh I get the warning:

WARNING: Your kernel does not support swap limit capabilities or the cgroup is not mounted, Memory limited without swap.
Which is implying to me either that the missing swap option didn't matter, or it was somehow limited by a different method.
afaict, the code that issues that warning is at:
https://github.com/moby/moby/blob/5fc12449d830ae9005138fb3d3782728fa8d137a/daemon/daemon_unix.go#L368

Does that bear relevance here?

@devimc
Copy link
Author

devimc commented Jul 23, 2018

@grahamwhaley yes, I know it looks weird (enable swap when we don't have swap) but libcontainer specifies the maximum usage permitted for user memory plus swap space, I'll take a look to source code to see if it's possible to skip/disable it.

Take a look #103

@jodh-intel
Copy link
Contributor

Any update on this @devimc? Also, why no updated to ppc64le's config?

/cc @nitkon.

@devimc
Copy link
Author

devimc commented Jul 30, 2018

@jodh-intel I'm waiting for review, ppc64le already has that config

@jodh-intel
Copy link
Contributor

Ah - thiis'll be related to #8, #55, #56 then ;)

@devimc
Copy link
Author

devimc commented Aug 1, 2018

re-ping @kata-containers/packaging

@jodh-intel
Copy link
Contributor

jodh-intel commented Aug 1, 2018

lgtm

/cc @nitkon.

Approved with PullApprove

@nitkon
Copy link
Contributor

nitkon commented Aug 1, 2018

@jodh-intel : Its already enabled in ppc64le kernel config file and this patch changes only for x86 and arm.

@grahamwhaley
Copy link
Contributor

I guess you did the research and didn't find a way to skip/disable it then @devimc ?

@devimc
Copy link
Author

devimc commented Aug 1, 2018

@grahamwhaley that's correct, we need these flags to honour memory constrains in the VM

Copy link
Contributor

@grahamwhaley grahamwhaley left a comment

Choose a reason for hiding this comment

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

OK, given @devimc 's confirmation,
lgtm

@devimc devimc force-pushed the topic/fixMemConstraints branch from 6d617f4 to e9edb2b Compare August 1, 2018 16:42
libcontainer limits the memory+swap usage by writing the limit at
/sys/fs/cgroup/memory/docker/$CONTID/memory.memsw.limit_in_bytes, this path
doesn't exist if CONFIG_MEMCG_SWAP and CONFIG_MEMCG_SWAP_ENABLED are not
enabled.

fixes kata-containers#103

Signed-off-by: Julio Montes <[email protected]>
@devimc devimc force-pushed the topic/fixMemConstraints branch from e9edb2b to 3b18544 Compare August 1, 2018 16:43
@bergwolf
Copy link
Member

bergwolf commented Aug 2, 2018

@devimc What about arm config? It doesn't enable swap yet.

cc @kalyxin02 @Weichen81

@bergwolf
Copy link
Member

bergwolf commented Aug 2, 2018

oops, my bad, arm config is added there.

/lgtm

@bergwolf
Copy link
Member

bergwolf commented Aug 2, 2018

Let's merge it!

@bergwolf bergwolf merged commit 47835a8 into kata-containers:master Aug 2, 2018
@Weichen81
Copy link
Contributor

Thanks @devimc ; )

@devimc devimc deleted the topic/fixMemConstraints branch April 8, 2019 16:14
jcvenegas pushed a commit that referenced this pull request Jul 2, 2019
`.ci/setup.sh` is using dnf instead of yum to install
centos dependencies. This fixes it to use yum.

Fixes: #104.

Signed-off-by: Salvador Fuentes <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Failed to run containers with memory constraints
6 participants