Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Cc:     mtk.manpages@gmail.com, Christian Brauner <christian@brauner.io>,
        cgroups@vger.kernel.org, linux-api@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-man@vger.kernel.org,
        oleg@redhat.com, tj@kernel.org
Subject: Re: [PATCH v2] clone.2: Document CLONE_INTO_CGROUP
To:     Christian Brauner <christian.brauner@ubuntu.com>
References: <CAKgNAkhL0zCj11LS9vfae872YVeRsxdz20sZWuXdi+UjH21=0g@mail.gmail.com>
 <20200518175549.3400948-1-christian@brauner.io>
 <25b2d051-d276-d570-5608-2bf0f4f46ef1@gmail.com>
 <20200519135124.xhgdeaogmvmwbofc@wittgenstein>
From:   "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com>
Message-ID: <db0d3e1b-a0d4-20de-90e4-bb6549ac4cb7@gmail.com>
Date:   Tue, 19 May 2020 21:44:12 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.7.0
MIME-Version: 1.0
In-Reply-To: <20200519135124.xhgdeaogmvmwbofc@wittgenstein>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 5/19/20 3:51 PM, Christian Brauner wrote:
> On Tue, May 19, 2020 at 03:36:28PM +0200, Michael Kerrisk (man-pages) wrote:
>> On 5/18/20 7:55 PM, Christian Brauner wrote:
>>> From: Christian Brauner <christian.brauner@ubuntu.com>
>>> +
>>> +Spawning a process into a cgroup different from the parent's cgroup
>>> +makes it possible for a service manager to directly spawn new
>>> +services into dedicated cgroups. This allows eliminating accounting
>>> +jitter which would be caused by the new process living in the
>>> +parent's cgroup for a short amount of time before being
>>> +moved into the target cgroup. This flag also allows the creation of
>>> +frozen child process by spawning them into a frozen cgroup (see
>>> +.BR cgroups (7)
>>> +for a description of the freezer feature in version 2 cgroups).
>>> +For threaded applications or even thread implementations which
>>> +make use of cgroups to limit individual threads it is possible to
>>> +establish a fixed cgroup layout before spawning each thread
>>> +directly into its target cgroup.
>>
>> Thanks for these use cases; that's great!
>>
>> So, I did some fairly heavy editing, which resulted in the
>> following (the sum of the diffs is shown at the end of this
>> mail):
>>
>>        CLONE_INTO_CGROUP (since Linux 5.7)
>>               By default, a child process is placed in the same version 2
>>               cgroup  as  its  parent.   The CLONE_INTO_CGROUP allows the
> 
> Not a native speaker, but is this missing a noun like "flag"?
> "The CLONE_INTO_CGROUP {flag,feature} allows the [...]"?

Yes, "flag" was missing. Thanks.

>>               child process to  be  created  in  a  different  version  2
>>               cgroup.   (Note  that CLONE_INTO_CGROUP has effect only for
>>               version 2 cgroups.)
>>
>>               In order to place the child process in a different  cgroup,
>>               the caller specifies CLONE_INTO_CGROUP in cl_args.flags and
>>               passes a file descriptor that refers to a version 2  cgroup
>>               in  the cl_args.cgroup field.  (This file descriptor can be
>>               obtained by opening a cgroup v2 directory file using either
> 
> Should this just be "opening a cgroup v2 directory" and not "directory
> file"? Feels redundant.

Yes, better. Changed.
 
>>               the  O_RDONLY  or  the  O_PATH flag.)  Note that all of the
>>               usual restrictions (described in cgroups(7)) on  placing  a
>>               process into a version 2 cgroup apply.
>>
>>               Spawning  a  process  into a cgroup different from the par‐
>>               ent's cgroup makes it possible for  a  service  manager  to
>>               directly  spawn  new services into dedicated cgroups.  This
>>               eliminates the accounting jitter that would  be  caused  if
>>               the  child  process was first created in the same cgroup as
>>               the parent and then moved  into  the  target  cgroup.   The
> 
> I forgot to mention that spawning directly into a target cgroup is also
> more efficient than moving it after creation. The specific reason is
> mentioned in the commit message, the write lock of the semaphore need
> not be taken in contrast to when it is moved afterwards. That
> implementation details is not that interesting but it might be
> interesting to know that it provides performance benefits in general.

Thanks. I added this sentence:

    Furthermore, spawning the child process directly into a 
    target cgroup is significantly cheaper than moving the child 
    process into the target cgroup after it has been created.

>> Look okay to you?
> 
> Yep, looks great!

Good!

Thanks for the review.

Cheers,

Michael


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/