Messages in this thread | | | Date | Wed, 1 Dec 2021 06:46:50 -1000 | From | Tejun Heo <> | Subject | Re: [PATCH v8 5/6] cgroup/cpuset: Update description of cpuset.cpus.partition in cgroup-v2.rst |
| |
Hello, Waiman.
On Tue, Nov 30, 2021 at 10:56:34PM -0500, Waiman Long wrote: > > What happens if an isolated domain becomes invalid and then valid again due > > to cpu hotplug? Does it go "root invalid" and then back to "isolated"? > > Yes, the current code allow recovering from an invalid state. In this > particular case, the transition will be "isolated" --> "root invalid" --> > "isolated".
Wouldn't it be clearer if it became "isolated invalid"?
> > While it isn't necessarily tied to this series, it's a big no-no to restrict > > what a parent can do depending on what its descendants are doing. A cgroup > > higher up in the hierarchy should be able to change configuration however it > > sees fit as deligation breaks down otherwise. > > > > Maybe you can argue that cpuset is special and shouldn't be subject to such > > convention but I can't see strong enough justifications especially given > > that most of these restrictions can be broken by hotplug operations anyway > > and thus need code to handle those situations. > > These are all pre-existing restrictions before the introduction of > partition. These are checks done in validate_change(). I am just saying out > loud the existing behavior. If you think that needs to be changed, I am fine > with that. However, it will be a separate patch as it is not a behavior that > is introduced by this series.
I see. It looks more problematic now with the addtion of the state transition error reporting, more possible state transitions and, well, actual documentation.
> Once an invalid partition is changed to "member", there is no way for a > child invalid partition root to recover and become valid again. There is why > I force them to become "member" also. I am OK if you believe it is better to > keep them in the invalid state forever until we explicitly changed them to > "member" eventually.
That's because we don't allow turning a cgroup with descendants into a partition, right?
So, when we were first adding the partition support, the thinking was that as it's pretty niche anyway, we can take some aberrations and restrictions, but I don't think it's a good direction to be building up on top of those like this and would much prefer to clean up the rules and restrictions. I know that this has been going on for quite a while and am sorry that am coming back to the same issue repeatedly which isn't necessarily caused by the proposed change. What do you think?
Thanks.
-- tejun
| |