Message-ID: <672cd179-31be-e03a-f9ff-ce59b76e23e2@gmail.com>
Date:   Tue, 9 Jan 2018 23:48:11 +0100
From:   "Michael Kerrisk (man-pages)" <mtk.manpages@...il.com>
To:     Tejun Heo <tj@...nel.org>
Cc:     mtk.manpages@...il.com, "Serge E. Hallyn" <serge@...lyn.com>,
        linux-man <linux-man@...r.kernel.org>,
        lkml <linux-kernel@...r.kernel.org>, cgroups@...r.kernel.org,
        Roman Gushchin <guro@...com>
Subject: Re: cgroups(7): documenting cgroups v2 delegation

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@...r.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/

Powered by blists - more mailing lists