Skip to content
This repository was archived by the owner on Dec 13, 2018. It is now read-only.

Conversation

@hqhq
Copy link
Contributor

@hqhq hqhq commented Jun 18, 2015

Currently we can't start container with kmem limit, because we
set kmem limit after processes joined to cgroup, we'll get device
busy error in this case.

Fix it by moving set before join.

Signed-off-by: Qiang Huang [email protected]

@dqminh
Copy link
Contributor

dqminh commented Jun 18, 2015

LGTM.

IIRC, kmem is the only thing that has this requirement. Please correct me if i'm wrong :D

@hqhq
Copy link
Contributor Author

hqhq commented Jun 18, 2015

@dqminh Yes, AFAIK only kmem has this requirement.

Currently we can't start container with kmem limit, because we
set kmem limit after processes joined to cgroup, we'll get device
busy error in this case.

Fix it by moving set before join.

Signed-off-by: Qiang Huang <[email protected]>
@hqhq hqhq force-pushed the hq_fix_kmem_cgroup branch from d71b678 to 39279b1 Compare June 18, 2015 08:39
@hqhq
Copy link
Contributor Author

hqhq commented Jun 18, 2015

Sorry I forgot systemd part, updated now, please review again.

@mrunalp
Copy link
Contributor

mrunalp commented Jun 18, 2015

@vmarmol @rjnagal PTAL

@rjnagal
Copy link
Contributor

rjnagal commented Jun 18, 2015

yes, this is a kmem requirement as limiting can't happen when a cgroup already has a child:

from kernel docs:
"Kernel memory won't be accounted at all until limit on a group is set. This
allows for existing setups to continue working without disruption. The limit
cannot be set if the cgroup have children, or if there are already tasks in the
cgroup. Attempting to set the limit under those conditions will return -EBUSY.
When use_hierarchy == 1 and a group is accounted, its children will
automatically be accounted regardless of their limit value."

Does it make sense to do Set before Join for everything?

@rjnagal
Copy link
Contributor

rjnagal commented Jun 18, 2015

LGTM

rjnagal added a commit that referenced this pull request Jun 18, 2015
@rjnagal rjnagal merged commit ed1146a into docker-archive:master Jun 18, 2015
hqhq added a commit to hqhq/runc that referenced this pull request Jul 13, 2015
Fixes: opencontainers#57

Normally all cgroup subsystems are optional except device cgroup,
but memory cgroup optional was broken by:
docker-archive/libcontainer#637

This patch fixes this.

Signed-off-by: Qiang Huang <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants