runc
runc copied to clipboard
opencontainers
cgroups: Add support for `cgroup.kill` added in Linux Kernel 5.14
Following PR adds support for cgroup.kill to kill all the processes in
a cgroup instead of iterating over each process to save cost if
possible.
Linux kernel 5.14 adds support for cgroup.kill.
Ref
- https://lwn.net/Articles/855049/
- https://lwn.net/Articles/855924/
PS: this is already added in crun by @giuseppe via https://github.com/containers/crun/pull/724
@cyphar @AkihiroSuda @kolyshkin Addressed all the feedback points in latest commit. Could you please take a look again.
@flouthoc on what distro are you testing this? Problem is, we don't have 5.14+ kernel in CI yet.
@kolyshkin I am using ubuntu 21.04 but 5.14 is not shipped by default but its available on kernel.ubuntu.com mainline.
I have booted 5.14 myself and, as I was suspecting, the feature is only available for cgroup v2 (or when using v1 + hybrid hierarchy).
So, it does not make sense to add to v1 managers (unless they gain hybrid support), and the code you're using to get paths to cgroup.kill in v1 managers is probably not working.
Given the fact that this is not ready, and we don't have kernel 5.14 in CI, I think we have to postpone it to 1.2.0.
@kolyshkin fair points i think we can hold on to this till kernel 5.14 and I'll confirm if this works for v1 at all , if not i will remove the case for v1. Will add a .bats test for v2 does that sounds good ?
and I'll confirm if this works for v1 at all , if not i will remove the case for v1.
There's no cgroup.kill for v1, but it is there in case hybrid hierarchy is used. Support for hybrid hierarchy is added by various PRs. In this particular case, we need https://github.com/opencontainers/runc/pull/2087, and once it's in, we can use Path("") to get path to the hybrid hierarchy, if available.
Marked as draft as we need
- #2087 (or #3059 which carries it) for v1 + hybrid unified path support;
- kernel 5.14 in CI;
Problem is, #3059 needs criu 3.16 which is not yet released.
@kolyshkin @cyphar Just curious how would m.paths[""] would look like for hybrid+v1 could you please state with an example. I'll try setting up a hybrid+v1 in spare time for myself to work on this till other PR gets merged.
Just want to do this testing myself here cause i think it will take some time to get new kernel in CI meanwhile i can just do testing on my own and we can keep PR on hold.
Nvm i'll wait for dependent PRs then will pick this up again. I think would be hard to plumb things temporarily into code.
@flouthoc can you revisit this? I almost ended up rewriting this from scratch (which would be a colossal waste of time).
I completely forgot about this patch. Ping me to review it when you're done rebasing. :D
@flouthoc can you take a look and refresh this PR? This would be nice to have in 1.2.0
I am working on an alternative, implemented without adding a new public method to cgroup managers, and modelled after cgroup.kill kernel tests. Still a WIP.