Message-ID: <20170711165233.xx6wko4pdxk4rb72@hirez.programming.kicks-ass.net>
Date:   Tue, 11 Jul 2017 18:52:33 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Waiman Long <longman@...hat.com>
Cc:     Tejun Heo <tj@...nel.org>, Li Zefan <lizefan@...wei.com>,
        hannes@...xchg.org, mingo@...hat.com, cgroups@...r.kernel.org,
        linux-kernel@...r.kernel.org, kernel-team@...com, pjt@...gle.com,
        luto@...capital.net, efault@....de, torvalds@...ux-foundation.org
Subject: Re: [PATCHSET for-4.13] cgroup: implement cgroup2 thread mode, v2

On Tue, Jul 11, 2017 at 10:14:42AM -0400, Waiman Long wrote:

> The "join" was a special op for the children of cgroup root to join the
> root as part of a threaded subtree. The children can instead use the
> "enable" option to become a thread root which was the configuration
> shown above.  This behavior applied only to children of root. Down the
> hierarchy, you can't have configuration like:
> 
>      R (t=0)
>     / \
>        D (t=1)
>       / \
>      T   D (t=1)

Why not?

First you create:

      R (t=0)
     / \
        D (t=1)
       / \
      T   T (t=1)

Then you flip t=0 like:

      R (t=0)
     / \
        D (t=1)
       / \
      T   D (t=0)

And then you flip t=1 again:

      R (t=0)
     / \
        D (t=1)
       / \
      T   D (t=1)

> With Tejun's v3 patch, the "join" operation was removed and "enable"

I've no clue what 'enable' is... :-(

> behaved like "join" in joining the threaded subtree of the root. I was
> wrong in saying that the configuration listed in your example was not
> possible. It was, but it depends on the order of activating the thread
> mode. If we enables thread mode on a child of root first followed by the
> root itself, we can have your configuration, but not in the reverse
> order. It was possible in the reverse order in the previous patch.

Just create a T child, then flip t=0 to convert it to D, then flip it to
1 again to create a new thread-root, no?

> > And this is detection by inference, which breaks the moment you disable
> > all resource domain controllers, because at that point those files will
> > not be present.
> 
> It is true that there is no external marker to find out if a threaded
> cgroup is a root or not when the parent of a thread root is also a
> thread root of a separate threaded subtree if the domain controller
> files are not present. However, we can always add a status file to
> indicate the state of threaded-ness of a cgroup if we want to.

Why add status files when a simple change in marker can readily provide
this information?

> Tejun's patch makes resource domain the default and threaded-ness as an
> additional attribute that needs to be specified. Your proposal make
> non-resource domain where threads can exist as the default and resource
> domain as something that needs to be explicitly specified.

Not so. My proposal has resource domains as the default (remember, root
_MUST_ be a resource domain, therefore we must start start with
root.d=1). Therefore, any new subgroup will be a resource domain by
default and if you ignore the new attribute it will work exactly like
cgroup-v2 does today.

Only if you clear the new attribute do you get a thread subgroup.

> They are just
> different ways of partitioning a cgroup hierarchy into different
> domains. Tejun's patch has a well defined boundary for threaded subtree
> where threads can be migrated from one part of a subtree to another.
> Your proposal is less clear-cut on how to handle thread migration. 

Disagree again. We have the exact same boundaries. Just ensure the
migration doesn't escape the resource domain.

Powered by blists - more mailing lists