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

cpuset: option for updating cpus and mems in child groups #3189

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

askervin
Copy link
Contributor

Trying to set cpuset.cpus of a cgroup so that cpus of a child is not a subset of cpus of the cgroup causes an error (device or resource busy).

New option --child-policy="copy" sets cpuset.cpus of the group and all its children to the same value. Introducing a string-valued policy for this purpose leaves room for alternative policies in the future. Other sensible policies include, but are not limited to, modifying only children that already share the same values with the group, and keeping the relative number and intersections of cpus unchanged in all child groups.

cpuset.mems is handled similarly to cpuset.cpus.

This patch continues work started by @Ace-Tang in PR #1636, including the original commit that is rebased and modified to support the child policy option.

This patch fixes #1635.

askervin and others added 2 commits August 30, 2021 17:15
Trying to set cpuset.cpus of a cgroup so that cpus of a child is not a
subset of cpus of the cgroup causes an error (device or resource
busy).

New option --child-policy="copy" sets cpuset.cpus of the group and all
its children to the same value. Introducing a string-valued policy for
this purpose leaves room for alternative policies in the future. Other
sensible policies include, but are not limited to, modifying only
children that already share the same values with the group, and
keeping the relative number and intersections of cpus unchanged in all
child groups.

cpuset.mems is handled similarly to cpuset.cpus.

Signed-off-by: Antti Kervinen <[email protected]>
cpuset cgroup form a nested hierarchy visible in a virtual
file system, and child cpusets must contain a subset of the
parents CPU and Memory resources. When root user get permission
to make nested cpuset cgroup, this patch will allow update
operation in this case.

Fixes: opencontainers#1635

Signed-off-by: Ace-Tang <[email protected]>
@askervin
Copy link
Contributor Author

@hqhq commented to the (possibly abandoned) PR #1636, that values in nested cgroups should not be overridden by default. I totally agree with that. In this change I want to keep the default behavior unchanged, and keep the door open for other sensible policies to handle subgroups. However, there are cases where originally suggested behavior makes sense, so I'm adding here an option that implements it.

@kolyshkin
Copy link
Contributor

In general, I think the approach totally makes sense, but should be implemented in runtime-spec first.

As to the PR:

  • What about cgroup v2?
  • This needs tests.
  • runc update man page should be amended accordingly.

There's also another (semi) related thing that always bugged me -- cgroup v1 cpuset controller sets parent cgroups as well, which no other controller does. Maybe such behavior should be made configurable in a similar manner.

@askervin
Copy link
Contributor Author

Thanks for timely and encouraging feedback, @kolyshkin! Now that I know this is on right track, I'll address the shortcomings in this PR soon.

One clarifying question:

In general, I think the approach totally makes sense, but should be implemented in runtime-spec first.

Do you mean creating another PR to modify the schema in config-linux.json and/or something else? I'm probably not aware of all the places I should touch to introduce a configurable cgroup policy in runtime-spec.

@kolyshkin
Copy link
Contributor

One more thing. I guess this should be --children-policy (or maybe --sub-cgroups-policy, --cgroup-children-policy, or something similar) -- i.e. in plural.

Do you mean creating another PR to modify the schema in config-linux.json and/or something else? I'm probably not aware of all the places I should touch to introduce a configurable cgroup policy in runtime-spec.

I do not have lots of experience in runtime-spec myself either, but basically, yes, I think that this parameter should also be a part of config-linux.json (and be described in https://github.com/opencontainers/runtime-spec/blob/master/config-linux.md#control-groups next to cgroupsPath, we can call it cgroupChildrenPolicy or so).

Somewhat related: #3040 / #3059.

@kolyshkin
Copy link
Contributor

@askervin do you intend to work on this?

(removing 1.2.0 milestone for now)

@kolyshkin kolyshkin removed this from the 1.2.0 milestone Nov 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

runc update error when there is a nested cpuset cgroup
3 participants