runc icon indicating copy to clipboard operation
runc copied to clipboard

cpuset: option for updating cpus and mems in child groups

Open askervin opened this issue 4 years ago • 5 comments

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 avatar Aug 30 '21 14:08 askervin

@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.

askervin avatar Aug 30 '21 14:08 askervin

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.

kolyshkin avatar Aug 30 '21 18:08 kolyshkin

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.

askervin avatar Aug 31 '21 14:08 askervin

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: https://github.com/opencontainers/runc/issues/3040 / https://github.com/opencontainers/runc/pull/3059.

kolyshkin avatar Aug 31 '21 16:08 kolyshkin

@askervin do you intend to work on this?

(removing 1.2.0 milestone for now)

kolyshkin avatar Nov 10 '23 00:11 kolyshkin