remove JoinControllers= setting#10785
Merged
poettering merged 8 commits intosystemd:masterfrom Nov 16, 2018
Merged
Conversation
cdown
approved these changes
Nov 16, 2018
Member
cdown
left a comment
There was a problem hiding this comment.
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... ;-)
keszybz
reviewed
Nov 16, 2018
Member
|
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
Member
Author
|
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 |
Member
|
Looks good to me from cgroups side. |
Member
Author
|
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.) |
keszybz
approved these changes
Nov 16, 2018
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Let's remove some old cruft, in preparation for merging #10507