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

Tejun Heo
2017-07-19T15:40:55Z

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