Thread (39 messages) 39 messages, 6 authors, 2021-12-03

Re: [PATCH v8 5/6] cgroup/cpuset: Update description of cpuset.cpus.partition in cgroup-v2.rst

From: Tejun Heo <hidden>
Date: 2021-12-01 16:47:01
Also in: linux-doc, linux-kselftest, lkml

Hello, Waiman.

On Tue, Nov 30, 2021 at 10:56:34PM -0500, Waiman Long wrote:
quoted
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"?
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help