Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758102AbeAIWsU (ORCPT + 1 other); Tue, 9 Jan 2018 17:48:20 -0500 Received: from mail-pg0-f41.google.com ([74.125.83.41]:35369 "EHLO mail-pg0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753496AbeAIWsS (ORCPT ); Tue, 9 Jan 2018 17:48:18 -0500 X-Google-Smtp-Source: ACJfBotafUHRb9Kp2vKSCHoxcm4WNBVzGWo4O3i8JWeJIHZoQcBOzHZAabKKrl7y1Y07U+Y2dkV/zg== Cc: mtk.manpages@gmail.com, "Serge E. Hallyn" , linux-man , lkml , cgroups@vger.kernel.org, Roman Gushchin Subject: Re: cgroups(7): documenting cgroups v2 delegation To: Tejun Heo References: <1ce0a885-94fb-7480-7180-7b873c95b1bb@gmail.com> <20180108141450.GP3668920@devbig577.frc2.facebook.com> <6c9fdaee-739f-164d-d04e-fb3a7319db90@gmail.com> <20180109210722.GS3668920@devbig577.frc2.facebook.com> From: "Michael Kerrisk (man-pages)" Message-ID: <672cd179-31be-e03a-f9ff-ce59b76e23e2@gmail.com> Date: Tue, 9 Jan 2018 23:48:11 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: <20180109210722.GS3668920@devbig577.frc2.facebook.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: Hello Tejun, On 01/09/2018 10:07 PM, Tejun Heo wrote: > Hello, > > On Tue, Jan 09, 2018 at 12:27:58AM +0100, Michael Kerrisk (man-pages) wrote: >>>> /dlgt_grp/cgroup.subtree_control >>>> Making this file owned by the delegatee is optional. >>>> Doing so means that that the delegatee can enable con‐ >>>> trollers (that are present in /dlgt_grp/cgroup.con‐ >>>> trollers) in order to further redistribute resources at >>>> lower levels in the subtree. As an alternative to >>>> changing the ownership of this file, the delegater might >>>> instead add selected controllers to this file. >>> >>> I'm not sure how useful it is to describe this to be optional. In the >>> same sense, cgroup.procs would be optional too as the delegatee can >>> take control from its first children. Users of course can choose to >>> do mix and match as they see fit but outside of the defined model, >>> there can be surprises - e.g. nsdelegate or some future delegation >>> aware feature can behave differently. I think it'd be better to keep >>> it simple - either a subtree is delegated or not. >> >> So, I changed that piece to: >> >> /dlgt_grp/cgroup.subtree_control >> Changing the ownership of this file means that that the >> delegatee can enable controllers (that are present in >> /dlgt_grp/cgroup.controllers) in order to further redis‐ >> tribute resources at lower levels in the subtree. (As an >> alternative to changing the ownership of this file, the >> delegater might instead add selected controllers to this >> file.) >> >> Okay? > > I haven't thought much about not giving out cgroup.subtree_control > when delegating. It's probably okay but there can be surprises - > e.g. systemd or other agent failing to init resource control because > it failed to write to subtree_control at root. > > Also, given that the recommended way to delegate is now chown all > files in the /sys/kernel/cgroup/delegate file it's a bit weird to > single out subtree_control. I guess the point here is that it really makes no sense not to change ownership of cgroup.procs. On the other hand, if ownership of cgroup.subtree_control is not handed to the delegatee, then the delegater can choose which controllers the delegatee will be allowed to exercise, by writing only those controllers into cgroup.subtree_control. I'm not sure if there's a use case though. > That said, not necessarily an objection. I'm just not sure about it. Okay. For the moment, I'll leave that text as is. I realized that there was also one more file that needs to be included in the list of files that must be delegated: /dlgt_grp/cgroup.threads Changing the ownership of this file is necessary if a threaded subtree is being delegated (see the description of "thread mode", below). Can you please confirm that it's only necessary to delegate this file if we are delegating a threaded subtree? >>> Roman recently added /sys/kernel/cgroup/delegate and >>> /sys/kernel/cgroup/features. The former contains newline separated >>> list of cgroup files which should be chowned on delegation (in >>> addition to the directory itself) and the latter contains optional >>> features (currently only nsdelegate). >> >> Oh -- and this reminds that I've been meaning to ask you for a while >> now: could you please (please please please) CC all cgroup interface >> changes to linux-api@vger.kernel.org (and prod others to do so >> please). There have been many of these changes in recent times >> (addition of new v2 controllers, thread mode, nsdelegate, the >> changes that Roman made that you refer to above), and they really >> all should have been CCed to linux-api@. It's often the only (easy) >> way that I have to discover changes that should be documented in >> the manual pages. And there are many other groups that are also >> interested in tracking kernel-user-space interface changes; see >> https://www.kernel.org/doc/man-pages/linux-api-ml.html > > Sorry about having not added them before. Will try to. Please feel > free to scream at me if I forget. Okay. (Except I may not not e when you forget ;-).) >>> Also, if nsdelegate is enabled, both the source and destination >>> cgroups must be visible (cgroup namespace-wise) to the writer. >> >> I added the following: >> >> * If the cgroup v2 filesystem was mounted with the nsdelegate >> option, the writer must be able to see the source and destina‐ >> tion cgroup from its cgroup namespace. >> >> Okay? > > Yeah, thanks. Thanks for checking the text, Tejun! Cheers, Michael -- Michael Kerrisk Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/ Linux/UNIX System Programming Training: http://man7.org/training/