-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
remove JoinControllers= setting #10785
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
Conversation
cdown
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks really good. Thanks a lot for your work on this!
I have no idea on the SMACK stuff, but perhaps nobody else really does either, so... ;-)
|
I'm setting the flag in the sense "needs response to comments". The comments are mostly questions. |
This removes the ability to configure which cgroup controllers to mount together. Instead, we'll now hardcode that "cpu" and "cpuacct" are mounted together as well as "net_cls" and "net_prio". The concept of mounting controllers together has no future as it does not exist to cgroupsv2. Moreover, the current logic is systematically broken, as revealed by the discussions in systemd#10507. Also, we surveyed Red Hat customers and couldn't find a single user of the concept (which isn't particularly surprising, as it is broken...) This reduced the (already way too complex) cgroup handling for us, since we now know whenever we make a change to a cgroup for one controller to which other controllers it applies.
… mask according to cpu/cpuacct joint mounting Note that for cgroup_context_get_mask() this doesn't actually change much, but it does prepare the ground for systemd#10507 later on.
… take jointly mounted controlelrs into account If we create a cgroup in one controller it might already have been created in another too, if we have jointly mounted controllers. Take that into consideration.
96b42ff to
f543534
Compare
|
I force pushed a new version. Addresses everything mentioned, modulo my comments above. I also dropped the if clean-up patch, it's now part of #10803 instead. No other changes. PTAL |
|
Looks good to me from cgroups side. |
|
Thanks for the review! I think that should be enough to set the green label (in particular as @keszybz pretty much liked it too already.) |
Let's remove some old cruft, in preparation for merging #10507