mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Waiman Long <longman@redhat.com>
To: Peter Zijlstra <peterz@infradead.org>, Tejun Heo <tj@kernel.org>
Cc: 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: Tue, 18 Jul 2017 13:35:51 -0400	[thread overview]
Message-ID: <b9fed984-76a4-b27a-b570-9b5fafa2614b@redhat.com> (raw)
In-Reply-To: <20170718172801.f56273tzgzn3xkne@hirez.programming.kicks-ass.net>

On 07/18/2017 01:28 PM, Peter Zijlstra wrote:
> On Mon, Jul 17, 2017 at 10:26:09AM -0400, Tejun Heo wrote:
>> Hello, Peter.
>>
>> On Mon, Jul 17, 2017 at 04:14:09PM +0200, Peter Zijlstra wrote:
>>> AFAICT this is not in fact what I suggested... :/
>> Heh, sorry about misattributing that.  I was mostly referring to the
>> overall idea of marking each cgroup domain or threaded rather than
>> subtree.
>>
>>> My proposal did not have that invalid state. It would simply refuse to
>>> change the type from thread to domain in the case where the parent is
>>> not a domain.
>>>
>>> Also, my proposal maintained the normal property inheritance rules. A
>>> child cgroup's creation 'type' would be that of its parent and not
>>> always be 'domain'.
>> But aren't both of the above get weird when the parent can host both
>> domain and threaded children?
>>
>> 	  R
>>        /
>>       A(D)
>>
>> If you create another child B under R, it's naturally gonna be a
>> domain.  Let's say you turn that to threaded.
>>
>> 	   R
>>        /   \
>>      A(D) B(T)
>>
>> And now try to create another child C, should that be a domain or
>> threaded?
> Domain of course, as R must be a domain, and hence all its children
> start out as such.
>
>> If we only inherit from the second level on, which is in itself
>> already confusing, that still leads to invalid configs for non-root
>> thread roots.
> I don't see how. I don't get the example Waiman gave, what is wrong
> with:
>
> 	R (D)
> 	|
> 	A (D)
>        / \
>      C(D) B(T) 
>
> ? Afaict that's a perfectly valid configuration.

>From what I understand, C is considered to be in an invalid state
because of the no internal process rule. A in this case is the thread
root, so it can have internal process. If C is a domain and a child of
A, we will have the case that internal processes in A is competing
against cgroup C.

I have been advocating (in one of my RFC patches) that we should relax
the rules to allow internal processes when only threaded controllers are
enabled as they are supposed to be able to handle internal processes.

Cheers,
Longman

  reply	other threads:[~2017-07-18 17:35 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 [this message]
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
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
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=b9fed984-76a4-b27a-b570-9b5fafa2614b@redhat.com \
    --to=longman@redhat.com \
    --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=luto@amacapital.net \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=pjt@google.com \
    --cc=tj@kernel.org \
    --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®