From: Tejun Heo <tj@kernel.org>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Li Zefan <lizefan@huawei.com>,
hannes@cmpxchg.org, mingo@redhat.com, longman@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
Subject: Re: [PATCHSET for-4.13] cgroup: implement cgroup2 thread mode, v2
Date: Mon, 12 Jun 2017 17:27:53 -0400 [thread overview]
Message-ID: <20170612212753.GN19206@htj.duckdns.org> (raw)
In-Reply-To: <20170612123150.scopfxela7v26dct@hirez.programming.kicks-ass.net>
Hello, Peter.
On Mon, Jun 12, 2017 at 02:31:50PM +0200, Peter Zijlstra wrote:
> Please don't rush this; also, I might not be around much the coming
> weeks due to taking some leave 'soon' (kid #3 is imminent).
Congrats. As for this going forward, how can we possibly be slower?
> And I really need more time to look at this (and re-read the old
> discussions, because I've forgot most everything again).
Can we at least unblock the cpu controller part? We can hash the
details of thread support as long as necessary but I'm not sure it's
reasonable to keep blocking the whole cpu controller at this point.
> > * Root cgroup can enable thread mode anytime and a first level child
> > can opt-in to that thread subtree anchored at root by writing "join"
> > to "cgroup.threads" files, start its own thread subtree or just be a
> > normal cgroup.
>
> Yuck... this again is a consequence of tagging the 'wrong' thing. Again,
> the primary construct is the resource domain.
>
> If you use that as a tag, you don't need this weird join crap. Because
> as soon as you clear the 'resource domain' flag on a group, it instantly
> becomes a thread group and 'obviously' connects to the first parent that
> is a resource domain.
It has nothing to do with whether we mark domain or threaded subtrees.
It is solely from whether you wanna express cases where a thread root
is right below another thread root. Tn's are member cgroup of thread
subtrees where the same number means the same threaded subtree, D's
are of domain cgroups.
The following is straight forward.
T0
/ \
T0 D
The following is too.
T0
/ \
T0 D
\
T1
The question is whether to allow something like the following.
T0
/ \
T0 T1
That's where the "join" thing comes from because we wanna be able to
tell apart whether a cgroup is gonna be a part of the existing thread
subtree or starting its own thread subtree. There sure are multiple
ways to express that but one way or the other, if you wanna support
topologies like the last one, you have to distinguish the two.
The previous iteration actually was that way, so the only thread mode
operation was setting whether to enable thread or not as before and if
the parent is already thread mode, it'd always join the existing
threaded subtree. If you like that better, I can post that version
right away.
> And, as per the last time, this threaded marker isn't uniquely
> identifying things, so it hard prohibits from ever extending the model
> to allow resource domains nested in a thread subtree. Now I understand
> why you don't implement that now -- you were struggling with the views
> API, but that is no excuse to create an API that permanently disables
> that feature.
Hmmm? We can just allow disabling thread mode if we ever get to that.
We can't make arbitrary graphs out of these nodes. Whatever mode we
put them in, they have to fall in with the overall tree structure, so
I don't think the interface is unnecessarily restricting in that
direction.
> I cannot at this time remember if there was a strong use-case for that
> scenario -- like said, I really need to re-read the email threads, but I
> might not have enough time to do so now.
>
> Again, please don't rush this.
Well, I don't have a way to do that.
> So I really regret the 'shares' interface; we really should have done a
> nice thing.
>
> https://lkml.kernel.org/r/20170410073622.2y6tnpcd2ssuoztz@hirez.programming.kicks-ass.net
>
> So I would like to change to that instead of the weird 100 thing.
Is it? Relative weights are pretty fundamental and clearly defined in
expressing work-conserving resource distribution. Do you have more
details on what you have on mind?
> As for the RT thing, the runtime/period thing is not a MAX but a MIN
> limit (conceptually -- in practise its both).
Yeah, it's a hard allocation.
> Also, we need cpuset to be a thread controller.
Yeah, absolutely.
Thanks.
--
tejun
next prev parent reply other threads:[~2017-06-12 21:27 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-06-10 14:03 Tejun Heo
2017-06-10 14:03 ` [PATCH 01/10] cgroup: separate out cgroup_has_tasks() Tejun Heo
2017-06-10 14:03 ` [PATCH 02/10] cgroup: reorganize cgroup.procs / task write path Tejun Heo
2017-06-10 14:03 ` [PATCH 03/10] cgroup: Fix reference counting bug in cgroup_procs_write() Tejun Heo
2017-06-10 14:03 ` [PATCH 04/10] cgroup: add @flags to css_task_iter_start() and implement CSS_TASK_ITER_PROCS Tejun Heo
2017-06-10 14:03 ` [PATCH 05/10] cgroup: introduce cgroup->proc_cgrp and threaded css_set handling Tejun Heo
2017-06-10 14:03 ` [PATCH 06/10] cgroup: implement CSS_TASK_ITER_THREADED Tejun Heo
2017-06-10 14:03 ` [PATCH 07/10] cgroup: implement cgroup v2 thread support Tejun Heo
2017-06-12 15:41 ` Waiman Long
2017-06-13 14:06 ` Tejun Heo
2017-06-15 20:14 ` [PATCH v3 " Tejun Heo
2017-06-10 14:03 ` [PATCH 08/10] sched: Misc preps for cgroup unified hierarchy interface Tejun Heo
2017-06-10 14:03 ` [PATCH 09/10] sched: Implement interface for cgroup unified hierarchy Tejun Heo
2017-06-10 14:03 ` [PATCH 10/10] sched: Make cpu/cpuacct threaded controllers Tejun Heo
2017-06-12 12:31 ` [PATCHSET for-4.13] cgroup: implement cgroup2 thread mode, v2 Peter Zijlstra
2017-06-12 21:27 ` Tejun Heo [this message]
2017-06-15 20:16 ` Tejun Heo
2017-06-27 7:01 ` Peter Zijlstra
2017-06-30 13:23 ` Tejun Heo
2017-07-10 8:32 ` Peter Zijlstra
2017-07-10 21:01 ` Waiman Long
2017-07-11 12:15 ` Peter Zijlstra
2017-07-11 14:14 ` Waiman Long
2017-07-11 16:52 ` Peter Zijlstra
2017-07-11 21:12 ` Waiman Long
2017-07-12 7:45 ` Peter Zijlstra
2017-07-12 14:00 ` Waiman Long
2017-06-15 20:17 ` [PATCH] cgroup: update debug controller to print out thread mode information 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=20170612212753.GN19206@htj.duckdns.org \
--to=tj@kernel.org \
--cc=cgroups@vger.kernel.org \
--cc=efault@gmx.de \
--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
all inboxes | Powered by JetHome®