From: Tejun Heo <tj@kernel.org>
To: Waiman Long <longman@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
lizefan@huawei.com, hannes@cmpxchg.org, mingo@redhat.com,
cgroups@vger.kernel.org, linux-kernel@vger.kernel.org,
kernel-team@fb.com, pjt@google.com, luto@amacapital.net,
efault@gmx.de, torvalds@linux-foundation.org, guro@fb.com
Subject: Re: [PATCH 5/6] cgroup: implement cgroup v2 thread support
Date: Wed, 19 Jul 2017 12:29:37 -0400 [thread overview]
Message-ID: <20170719162937.GP3365493@devbig577.frc2.facebook.com> (raw)
In-Reply-To: <df14a222-f1f8-3415-b29a-2712c9e77f94@redhat.com>
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.
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.
So, that's why I think it'd be better to be simple here, even if that
adds a bit of hassle when creating threded children. It is simple and
consistent and can stay that way even if we make the hierarchy more
flexible in the future.
Thanks.
--
tejun
next prev parent reply other threads:[~2017-07-19 16:29 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-17 2:07 [PATCHSET for-4.13] cgroup: implement cgroup2 thread mode, v3 Tejun Heo
2017-07-17 2:07 ` [PATCH 1/6] cgroup: reorganize cgroup.procs / task write path Tejun Heo
2017-07-17 2:07 ` [PATCH 2/6] cgroup: add @flags to css_task_iter_start() and implement CSS_TASK_ITER_PROCS Tejun Heo
2017-07-17 2:07 ` [PATCH 3/6] cgroup: introduce cgroup->dom_cgrp and threaded css_set handling Tejun Heo
2017-07-17 2:07 ` [PATCH 4/6] cgroup: implement CSS_TASK_ITER_THREADED Tejun Heo
2017-07-17 2:07 ` [PATCH 5/6] cgroup: implement cgroup v2 thread support Tejun Heo
2017-07-17 14:14 ` Peter Zijlstra
2017-07-17 14:26 ` Tejun Heo
2017-07-18 17:28 ` Peter Zijlstra
2017-07-18 17:35 ` Waiman Long
2017-07-18 17:54 ` Tejun Heo
2017-07-18 18:41 ` Peter Zijlstra
2017-07-18 18:47 ` Tejun Heo
2017-07-19 14:07 ` Peter Zijlstra
2017-07-19 16:34 ` Tejun Heo
2017-07-17 20:56 ` Waiman Long
2017-07-18 14:37 ` Waiman Long
2017-07-18 17:10 ` Tejun Heo
2017-07-18 17:23 ` Waiman Long
2017-07-19 16:29 ` Tejun Heo [this message]
2017-07-19 17:09 ` Waiman Long
2017-07-19 17:48 ` Tejun Heo
2017-07-17 21:12 ` Waiman Long
2017-07-19 15:40 ` Tejun Heo
2017-07-17 2:07 ` [PATCH 6/6] cgroup: update debug controller to print out thread mode information Tejun Heo
2017-07-17 21:19 ` Waiman Long
2017-07-19 15:31 ` Tejun Heo
2017-07-19 15:41 ` Waiman Long
2017-07-19 15:44 ` Tejun Heo
2017-07-17 14:48 ` [PATCHSET for-4.13] cgroup: implement cgroup2 thread mode, v3 Waiman Long
2017-07-17 14:51 ` Tejun Heo
-- strict thread matches above, loose matches on Subject: below --
2017-07-19 19:44 [PATCHSET for-4.13] cgroup: implement cgroup2 thread mode, v4 Tejun Heo
2017-07-19 19:44 ` [PATCH 5/6] cgroup: implement cgroup v2 thread support Tejun Heo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170719162937.GP3365493@devbig577.frc2.facebook.com \
--to=tj@kernel.org \
--cc=cgroups@vger.kernel.org \
--cc=efault@gmx.de \
--cc=guro@fb.com \
--cc=hannes@cmpxchg.org \
--cc=kernel-team@fb.com \
--cc=linux-kernel@vger.kernel.org \
--cc=lizefan@huawei.com \
--cc=longman@redhat.com \
--cc=luto@amacapital.net \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=pjt@google.com \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox