libct/cg: add disable_cgroup_devices build tag#3421
Conversation
0c0c13b to
2daae6f
Compare
f54b538 to
6a897a1
Compare
AkihiroSuda
left a comment
There was a problem hiding this comment.
I don't think we should complicate tags.
Can we just move fs2.setDevices() to factory_linux.go so as to decouple fs2 pkg from the ebpf pkg?
1d3cca9 to
ec2297f
Compare
I looked at it, and it seems this will require more changes. Currently devices are an integral part of cgroup manager, and while I can see how it is decoupled, it seems it's going to look ugly. |
b81426f to
67d2fb7
Compare
I took a look, and it seems more complicated than that. Devices is the integral part of fs manager, and they can definitely be separated out, but that looks like a medium-sized refactoring, which I don't quite know how to do yet. In the meantime I updated this PR to be more reviewable -- first commit merely moves the code around (no code or functionality changes), and the second adds the tags, tests, and docs. |
This moves the functionality related to devices, SkipDevices, and SkipFreezeOnSet to a separate file, in preparation for the next commit. No code changes. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The idea is to use this tag for software which wants to use libcontainer/cgroups packages, but does not require the functionality to manage devices in cgroups, which brings some additional overhead, especially in fs2 where it exports cilium/ebpf. Using this build tag enables that software to save over 900 Kb (text+data+bss), which reduces runc binary size by more than 8%: text data bss dec hex filename 7016344 3938048 229704 11184096 aaa7e0 runc.devices 6423082 3597960 228808 10249850 9c667a runc.no-devices Obviously, we do not want runc to be compiled with this tag, so add a guard to main.go and libcontainer itself. NOTE than the tag is used for fs, we still create a directory under devices cgroup root, and it is used by Exists/GetPids/GetAllPids, but otherwise nothing is done with devices. The effect is similar to that of Resources.SkipDevices. Document the tag in README, and test this tag in unittest target. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
67d2fb7 to
4c16d51
Compare
|
@kolyshkin this seems to be the preferable option in podman with the tag as is |
So, I took another look at this and it seems this is more complicated than that. Doing this properly requires removing devices management from the cgroup managers (basically, creating a separate "devices" manager which is now part of cgroups. Ultimately it might be the best thing to do (for one thing, we can remove all these ugly I don't know. Should I try to separate devices out of cgroup manager instead of what this PR does, @opencontainers/runc-maintainers ? |
Implemented the separation in #3452, PTAL |
|
We went with #3452 instead, so closing this one |
The idea is to use this tag for software which wants to use
libcontainer/cgroupspackages, but does not require the functionalityto manage devices in cgroups, which brings some additional overhead,
especially in
fs2where it exportscilium/ebpf. Using this build tagenables that software to save about 586 Kb (text+data+bss), which
reduces runc binary size by about 6% (using go 1.18.0):
(The difference is more dramatic then using older versions of golang, e.g. 1.17.x)
Obviously, we do not want runc to be compiled with this tag, so add
a guard to
main.goand libcontainer itself.NOTE than the tag is used, the packages still create a directory under
devices cgroup root, and it is used by Exists/GetPids/GetAllPids,
but otherwise nothing is done with devices. The effect is similar to
that of
Resources.SkipDevicesandResources.SkipFreezeOnSet(for systemd cgroup v1) both set to
true, except it is a compile-time thing.Document the tag in README, and add a unit tests of libct/cg with this tag set.