On 12/1/21 11:46, Tejun Heo wrote:
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"?
You are right. I have overlooked that. Will make the change.
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.
I am going to add a patch to take out the child superset limitation for
the default hierarchy as I believe it is probably an oversight that we
were not aware of before. I would like to keep the exclusivity rule
though as I think it makes sense.
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?
Yes, that is a major part of it.
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?
I think I can relax some of the restrictions, but probably not all of
them at this time. We can certainly working on removing as much
restriction and limitations as possible in future update to the
partition code.
Cheers,
Longman
