From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933507AbdGSPkz (ORCPT ); Wed, 19 Jul 2017 11:40:55 -0400 Received: from mail-qk0-f194.google.com ([209.85.220.194]:37705 "EHLO mail-qk0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932439AbdGSPkv (ORCPT ); Wed, 19 Jul 2017 11:40:51 -0400 Date: Wed, 19 Jul 2017 11:40:46 -0400 From: Tejun Heo To: Waiman Long Cc: lizefan@huawei.com, hannes@cmpxchg.org, peterz@infradead.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 Message-ID: <20170719154046.GN3365493@devbig577.frc2.facebook.com> References: <20170717020721.3612468-1-tj@kernel.org> <20170717020721.3612468-6-tj@kernel.org> <199bfed8-de84-da6d-6623-c45524443aaa@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <199bfed8-de84-da6d-6623-c45524443aaa@redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.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