Re: [PATCH 5/6] cgroup: implement cgroup v2 thread support

[Waiman Long]

On 07/19/2017 12:29 PM, Tejun Heo wrote:
> Hello,
>
> On Tue, Jul 18, 2017 at 01:23:14PM -0400, Waiman Long wrote:
>>> If we could get rid of the invalid state completely that way, I'd
>>> completely agree with you but that isn't the case here as you noted
>>> yourself, so the choice between the two isn't something trivially
>>> clear.  Both choices come with their pros and cons.  We can absoultely
>>> discuss them comparing the pros and cons.
>> I am not advocating on removing the invalid state now as I note about
> Yeah, removing invalid state would be great but we can't at least yet.
>
>> sibling cgroups. I am just saying that there is no point in not doing an
>> automatic conversion to threaded for newly created children of threaded
>> cgroups (not thread root). I don't see any cons in doing that.
> So, the cons I see is inconsistency, now and in the future.
>
> This may seem less clear with system root because we can have both
> domain and theraded children below it, which makes a newly created
> cgroup being a domain seem natural.  More importantly, we can't do it
> any other way because we'd break existing users otherwise - creating a
> threaded cgroup would cause future first level cgroups to be threaded
> which will be very unexpected.
>
> Let's think about a non-root threaded domain.  At least for now, a
> non-root threaded domain is terminal - they can't host valid domain
> children.  As the alternative term "thread root" implies, the threaded
> domain can be the root of a threaded subtree and nothing else, so it's
> kinda weird to make a new child cgroup there start out as a domain
> which can't be used, just like it'd be for the second level descendant
> cgroup.
>
> However, the alternative is even stranger.  Let's say we make the
> first level child automatically threaded, but that is inconsistent
> with when we first enable threaded mode.  We either would have to turn
> all siblings at the same time or disallow enabling threaded mode if
> there are domain siblings, which I fear would be unnecessarily
> restrictive.
>
> Another point is that what if we eventually make non-root threaded
> roots able to host domain children?  Making children automatically
> threaded wouldn't make any sense then, right?  I'll come back to this
> later.
>
> So, it looks like if we're gonna automatically turn on threaded mode
> for new cgroups, the only thing we can do right now is what you're
> suggesting; however, we didn't arrive there through some
> straight-forward intuition or overall design.  It started as a simple
> idea (I want it to be automatic) but the end result is a contorted
> destination shaped by constraints and happenstance.
>
> To me, behaving differently on the first-level threaded children than
> on second+ level ones is too strange to be justified by the
> convenience of not having to turn on threaded on new cgroups.

OK, I get your point of being inconsistent. However, I don't think that
is a big deal.

> On top of that, what happens if we get to implement PeterZ's idea of
> skipping over threaded internal cgroups to allow domains under
> threaded subtrees?  That'd imply that we'd be able to host domains
> under threaded domains too.  The end result would be completely
> non-sensical.  We'd be defaulting to different modes for different
> reasons where half of those reasons won't hold anymore.  This isn't
> surprising given that there's nothing actually consistent about the
> suggested default behavior.

For me, that is the only good reason why we should keep the current
behavior. So I am fine with that.

+ cgrp->dom_cgrp = cgrp->dom_cgrp;

However, I am still puzzled by above line of code, should it be just

  cgrp->dom_cgrp = cgrp;

Cheers,
Longman