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

[Tejun Heo <tj@kernel.org>]

Hello, Waiman.

On Mon, Jul 17, 2017 at 05:12:31PM -0400, Waiman Long wrote:
> > +Marking a cgroup threaded makes it join the resource domain of its
> > +parent as a threaded cgroup.  The parent may be another threaded
> > +cgroup whose resource domain is further up in the hierarchy.  The root
> > +of a threaded subtree, that is, the nearest ancestor which is not
> > +threaded, is called threaded domain and serves as the resource domain
> > +for the entire subtree.
> 
> The cgroup code uses the term "thread root" in quite a number of places.
> So a developer may be confused when comparing the code and the
> documentation. I would recommend either introducing "thread root" as an
> alias for threaded domain here in the documentation or documenting that
> "threaded domain = thread root" in the code.

Yeah, I was a bit hesitant to introduce an extra term for it, but both
terms make sense and thread root is less cumbersome.  I'll incorporate
it into the doc.

> >    cgroup.procs
> >  	A read-write new-line separated values file which exists on
> >  	all cgroups.
> 
> Do we need to document that cgroup.procs isn't writable in a threaded
> cgroup?

Yeah, will update.

> > @@ -4301,6 +4606,7 @@ static struct cgroup *cgroup_create(struct cgroup *parent)
> >  	cgrp->self.parent = &parent->self;
> >  	cgrp->root = root;
> >  	cgrp->level = level;
> > +	cgrp->dom_cgrp = cgrp->dom_cgrp;
> 
> It is a no-op. I think it is better to modify it to
> 
> +    cgrp->dom_cgrp = cgroup_is_threaded(parent) ? parent->dom_cgrp : cgrp;
> 
> Then we won't have an invalid domain state.

I'll respond to this on the other sub-thread.

Thanks.

-- 
tejun