From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752053AbdFLMcO (ORCPT ); Mon, 12 Jun 2017 08:32:14 -0400 Received: from merlin.infradead.org ([205.233.59.134]:46378 "EHLO merlin.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750730AbdFLMcN (ORCPT ); Mon, 12 Jun 2017 08:32:13 -0400 Date: Mon, 12 Jun 2017 14:31:50 +0200 From: Peter Zijlstra To: Tejun Heo Cc: Li Zefan , hannes@cmpxchg.org, mingo@redhat.com, longman@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 Subject: Re: [PATCHSET for-4.13] cgroup: implement cgroup2 thread mode, v2 Message-ID: <20170612123150.scopfxela7v26dct@hirez.programming.kicks-ass.net> References: <20170610140351.10703-1-tj@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170610140351.10703-1-tj@kernel.org> User-Agent: NeoMutt/20170113 (1.7.2) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Please don't rush this; also, I might not be around much the coming weeks due to taking some leave 'soon' (kid #3 is imminent). And I really need more time to look at this (and re-read the old discussions, because I've forgot most everything again). On Sat, Jun 10, 2017 at 10:03:41AM -0400, Tejun Heo wrote: > * Thread mode is explicitly enabled on a cgroup by writing "enable" > into "cgroup.threads" file. The cgroup shouldn't have any child > cgroups or enabled controllers. > > * Once enabled, arbitrary sub-hierarchy can be created and threads can > be put anywhere in the subtree by writing TIDs into "cgroup.threads" > file. Process granularity and no-internal-process constraint don't > apply in a threaded subtree. > > * To be used in a threaded subtree, controllers should explicitly > declare thread mode support and should be able to handle internal > competition in some way. > > * The root of a threaded subtree serves as the resource domain for the > whole subtree. This is where all the controllers are guaranteed to > have a common ground and resource consumptions in the threaded > subtree which aren't tied to a specific thread are charged. > Non-threaded controllers never see beyond thread root and can assume > that all controllers will follow the same rules upto that point. > > * Root cgroup can enable thread mode anytime and a first level child > can opt-in to that thread subtree anchored at root by writing "join" > to "cgroup.threads" files, start its own thread subtree or just be a > normal cgroup. Yuck... this again is a consequence of tagging the 'wrong' thing. Again, the primary construct is the resource domain. If you use that as a tag, you don't need this weird join crap. Because as soon as you clear the 'resource domain' flag on a group, it instantly becomes a thread group and 'obviously' connects to the first parent that is a resource domain. And, as per the last time, this threaded marker isn't uniquely identifying things, so it hard prohibits from ever extending the model to allow resource domains nested in a thread subtree. Now I understand why you don't implement that now -- you were struggling with the views API, but that is no excuse to create an API that permanently disables that feature. I cannot at this time remember if there was a strong use-case for that scenario -- like said, I really need to re-read the email threads, but I might not have enough time to do so now. Again, please don't rush this. > This allows threaded controllers to implement thread granular resource > control without getting in the way of system level resource > partitioning. > > This patchset contains the following ten patches. For more details on > the interface and behavior, please refer to 0007. > > 0001-cgroup-separate-out-cgroup_has_tasks.patch > 0002-cgroup-reorganize-cgroup.procs-task-write-path.patch > 0003-cgroup-Fix-reference-counting-bug-in-cgroup_procs_wr.patch > 0004-cgroup-add-flags-to-css_task_iter_start-and-implemen.patch > 0005-cgroup-introduce-cgroup-proc_cgrp-and-threaded-css_s.patch > 0006-cgroup-implement-CSS_TASK_ITER_THREADED.patch > 0007-cgroup-implement-cgroup-v2-thread-support.patch > 0008-sched-Misc-preps-for-cgroup-unified-hierarchy-interf.patch > 0009-sched-Implement-interface-for-cgroup-unified-hierarc.patch > 0010-sched-Make-cpu-cpuacct-threaded-controllers.patch > > 0001-0007 implement cgroup2 thread mode. 0008-0010 enable CPU > controller on cgroup2 and mark them as supporting thread mode. > 0008-0010 are included for reference. So I really regret the 'shares' interface; we really should have done a nice thing. https://lkml.kernel.org/r/20170410073622.2y6tnpcd2ssuoztz@hirez.programming.kicks-ass.net So I would like to change to that instead of the weird 100 thing. As for the RT thing, the runtime/period thing is not a MAX but a MIN limit (conceptually -- in practise its both). Also, we need cpuset to be a thread controller.