Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Sat, 17 Nov 2018 00:02:28 -0800
From:   Mike Rapoport <rppt@linux.ibm.com>
To:     Roman Gushchin <guroan@gmail.com>
Cc:     Tejun Heo <tj@kernel.org>, Oleg Nesterov <oleg@redhat.com>,
        cgroups@vger.kernel.org, linux-kernel@vger.kernel.org,
        kernel-team@fb.com, Roman Gushchin <guro@fb.com>,
        linux-doc@vger.kernel.org
Subject: Re: [PATCH v3 7/7] cgroup: document cgroup v2 freezer interface
References: <20181117003830.15344-1-guro@fb.com>
 <20181117003830.15344-8-guro@fb.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20181117003830.15344-8-guro@fb.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
Message-Id: <20181117080227.GA22672@rapoport-lnx>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi,

On Fri, Nov 16, 2018 at 04:38:30PM -0800, Roman Gushchin wrote:
> Describe cgroup v2 freezer interface in the cgroup v2 admin guide.
> 
> Signed-off-by: Roman Gushchin <guro@fb.com>
> Cc: Tejun Heo <tj@kernel.org>
> Cc: linux-doc@vger.kernel.org
> Cc: kernel-team@fb.com
> ---
>  Documentation/admin-guide/cgroup-v2.rst | 26 +++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
> index 184193bcb262..a065c0bed88c 100644
> --- a/Documentation/admin-guide/cgroup-v2.rst
> +++ b/Documentation/admin-guide/cgroup-v2.rst
> @@ -862,6 +862,8 @@ All cgroup core files are prefixed with "cgroup."
>  	  populated
>  		1 if the cgroup or its descendants contains any live
>  		processes; otherwise, 0.
> +	  frozen
> +		1 if the cgroup is frozen; otherwise, 0.
> 
>    cgroup.max.descendants
>  	A read-write single value files.  The default is "max".
> @@ -895,6 +897,30 @@ All cgroup core files are prefixed with "cgroup."
>  		A dying cgroup can consume system resources not exceeding
>  		limits, which were active at the moment of cgroup deletion.
> 
> +  cgroup.freeze
> +	A read-write single value file which exists on non-root cgroups.
> +	Allowed values are "0" and "1". The default is "0".
> +
> +	Writing "1" to the file causes freezing of the cgroup and all
> +	descendant cgroups. This means that all belonging processes will
> +	be stopped and will not run until the cgroup will be explicitly
> +	unfrozen. Freezing of the cgroup may take some time; when the process

"when the process is complete" sounds somewhat ambiguous, it's unclear
whether freezing is complete or the process that's being frozen is
complete.

Maybe "when this action is completed"?

> +	is complete, the "frozen" value in the cgroup.events control file
> +	will be updated and the corresponding notification will be issued.

Can you please clarify how exactly cgroup.events would be updated?

> +	Cgroup can be frozen either by its own settings, either by settings

      ^ A cgroup ... and maybe there are more "a" and "the" that should be
fixed, it's hard for me to tell.

Also, I believe "either ..., or ..." sounds better than "either ...,
either ..."

> +	of any ancestor cgroups. If any of ancestor cgroups is frozen, the
> +	cgroup will remain frozen.
> +
> +	Processes in the frozen cgroup can be killed by a fatal signal.
> +	They also can enter and leave a frozen cgroup: either by an explicit
> +	move by a user, either if freezing of the cgroup races with fork().

ditto

> +	If a cgroup is moved to a frozen cgroup, it stops. If a process is

            ^ process?

> +	moving out of a frozen cgroup, it becomes running.

       ^ moved

> +	Frozen status of a cgroup doesn't affect any cgroup tree operations:
> +	it's possible to delete a frozen (and empty) cgroup, as well as
> +	create new sub-cgroups.

Maybe it's also worth adding that freezing a process has no effect on its
memory consumption, at least directly.

>  Controllers
>  ===========
> -- 
> 2.17.2
> 

-- 
Sincerely yours,
Mike.