[PATCH cgroup/for-4.21 1/2] cpuset: Minor cgroup2 interface updates

[PATCH cgroup/for-4.21 1/2] cpuset: Minor cgroup2 interface updates

[Tejun Heo]

From b1e3aeb11c5e86ee0988a038c4e7682d6beaa977 Mon Sep 17 00:00:00 2001
From: Tejun Heo <tj@kernel.org>
Date: Tue, 13 Nov 2018 12:03:33 -0800

* Rename the partition file from "cpuset.sched.partition" to
  "cpuset.cpus.partition".

* When writing to the partition file, drop "0" and "1" and only accept
  "member" and "root".

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Waiman Long <longman@redhat.com>
---
Two minor cpuset interface followup patches.  Applying to
cgroup/for-4.21.

Thanks.

 Documentation/admin-guide/cgroup-v2.rst | 6 +++---
 kernel/cgroup/cpuset.c                  | 8 ++++----
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
index f83a5231bbe3..07e06136a550 100644
--- a/Documentation/admin-guide/cgroup-v2.rst
+++ b/Documentation/admin-guide/cgroup-v2.rst
@@ -1708,15 +1708,15 @@ Cpuset Interface Files
 
 	Its value will be affected by memory nodes hotplug events.
 
-  cpuset.sched.partition
+  cpuset.cpus.partition
 	A read-write single value file which exists on non-root
 	cpuset-enabled cgroups.  This flag is owned by the parent cgroup
 	and is not delegatable.
 
         It accepts only the following input values when written to.
 
-        "root" or "1"   - a paritition root
-        "member" or "0" - a non-root member of a partition
+        "root"   - a paritition root
+        "member" - a non-root member of a partition
 
 	When set to be a partition root, the current cgroup is the
 	root of a new partition or scheduling domain that comprises
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index b897314bab53..1151e93d71b6 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -2468,11 +2468,11 @@ static ssize_t sched_partition_write(struct kernfs_open_file *of, char *buf,
 	buf = strstrip(buf);
 
 	/*
-	 * Convert "root"/"1" to 1, and convert "member"/"0" to 0.
+	 * Convert "root" to ENABLED, and convert "member" to DISABLED.
 	 */
-	if (!strcmp(buf, "root") || !strcmp(buf, "1"))
+	if (!strcmp(buf, "root"))
 		val = PRS_ENABLED;
-	else if (!strcmp(buf, "member") || !strcmp(buf, "0"))
+	else if (!strcmp(buf, "member"))
 		val = PRS_DISABLED;
 	else
 		return -EINVAL;
@@ -2631,7 +2631,7 @@ static struct cftype dfl_files[] = {
 	},
 
 	{
-		.name = "sched.partition",
+		.name = "cpus.partition",
 		.seq_show = sched_partition_show,
 		.write = sched_partition_write,
 		.private = FILE_PARTITION_ROOT,
-- 
2.17.1

[PATCH cgroup/for-4.21 2/2] cgroup: Add .__DEBUG__. prefix to debug file names

[Tejun Heo]

From c1bbd933e5fae83f96acd3f748bb01a0300b315d Mon Sep 17 00:00:00 2001
From: Tejun Heo <tj@kernel.org>
Date: Tue, 13 Nov 2018 12:06:41 -0800

Clearly mark the debug files and hide them by default by prefixing
".__DEBUG__.".

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Waiman Long <longman@redhat.com>
---
 kernel/cgroup/cgroup.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index ed7f0bfe6429..e06994fd4e34 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -1400,12 +1400,15 @@ static char *cgroup_file_name(struct cgroup *cgrp, const struct cftype *cft,
 	struct cgroup_subsys *ss = cft->ss;
 
 	if (cft->ss && !(cft->flags & CFTYPE_NO_PREFIX) &&
-	    !(cgrp->root->flags & CGRP_ROOT_NOPREFIX))
-		snprintf(buf, CGROUP_FILE_NAME_MAX, "%s.%s",
-			 cgroup_on_dfl(cgrp) ? ss->name : ss->legacy_name,
+	    !(cgrp->root->flags & CGRP_ROOT_NOPREFIX)) {
+		const char *dbg = (cft->flags & CFTYPE_DEBUG) ? ".__DEBUG__." : "";
+
+		snprintf(buf, CGROUP_FILE_NAME_MAX, "%s%s.%s",
+			 dbg, cgroup_on_dfl(cgrp) ? ss->name : ss->legacy_name,
 			 cft->name);
-	else
+	} else {
 		strscpy(buf, cft->name, CGROUP_FILE_NAME_MAX);
+	}
 	return buf;
 }
 
-- 
2.17.1

Re: [PATCH cgroup/for-4.21 2/2] cgroup: Add .__DEBUG__. prefix to debug file names

[Peter Zijlstra]

On Tue, Nov 13, 2018 at 12:16:37PM -0800, Tejun Heo wrote:
> From c1bbd933e5fae83f96acd3f748bb01a0300b315d Mon Sep 17 00:00:00 2001
> From: Tejun Heo <tj@kernel.org>
> Date: Tue, 13 Nov 2018 12:06:41 -0800
> 
> Clearly mark the debug files and hide them by default by prefixing
> ".__DEBUG__.".

ACK

Re: [PATCH cgroup/for-4.21 1/2] cpuset: Minor cgroup2 interface updates

[Waiman Long]

On 11/13/2018 12:13 PM, Tejun Heo wrote:
> From b1e3aeb11c5e86ee0988a038c4e7682d6beaa977 Mon Sep 17 00:00:00 2001
> From: Tejun Heo <tj@kernel.org>
> Date: Tue, 13 Nov 2018 12:03:33 -0800
>
> * Rename the partition file from "cpuset.sched.partition" to
>   "cpuset.cpus.partition".
>
> * When writing to the partition file, drop "0" and "1" and only accept
>   "member" and "root".
>
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: Peter Zijlstra (Intel) <peterz@infradead.org>
> Cc: Waiman Long <longman@redhat.com>
> ---
> Two minor cpuset interface followup patches.  Applying to
> cgroup/for-4.21.
>
> Thanks.
>
>  Documentation/admin-guide/cgroup-v2.rst | 6 +++---
>  kernel/cgroup/cpuset.c                  | 8 ++++----
>  2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
> index f83a5231bbe3..07e06136a550 100644
> --- a/Documentation/admin-guide/cgroup-v2.rst
> +++ b/Documentation/admin-guide/cgroup-v2.rst
> @@ -1708,15 +1708,15 @@ Cpuset Interface Files
>  
>  	Its value will be affected by memory nodes hotplug events.
>  
> -  cpuset.sched.partition
> +  cpuset.cpus.partition
>  	A read-write single value file which exists on non-root
>  	cpuset-enabled cgroups.  This flag is owned by the parent cgroup
>  	and is not delegatable.
>  
>          It accepts only the following input values when written to.
>  
> -        "root" or "1"   - a paritition root
> -        "member" or "0" - a non-root member of a partition
> +        "root"   - a paritition root
> +        "member" - a non-root member of a partition
>  
>  	When set to be a partition root, the current cgroup is the
>  	root of a new partition or scheduling domain that comprises
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index b897314bab53..1151e93d71b6 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -2468,11 +2468,11 @@ static ssize_t sched_partition_write(struct kernfs_open_file *of, char *buf,
>  	buf = strstrip(buf);
>  
>  	/*
> -	 * Convert "root"/"1" to 1, and convert "member"/"0" to 0.
> +	 * Convert "root" to ENABLED, and convert "member" to DISABLED.
>  	 */
> -	if (!strcmp(buf, "root") || !strcmp(buf, "1"))
> +	if (!strcmp(buf, "root"))
>  		val = PRS_ENABLED;
> -	else if (!strcmp(buf, "member") || !strcmp(buf, "0"))
> +	else if (!strcmp(buf, "member"))
>  		val = PRS_DISABLED;
>  	else
>  		return -EINVAL;
> @@ -2631,7 +2631,7 @@ static struct cftype dfl_files[] = {
>  	},
>  
>  	{
> -		.name = "sched.partition",
> +		.name = "cpus.partition",
>  		.seq_show = sched_partition_show,
>  		.write = sched_partition_write,
>  		.private = FILE_PARTITION_ROOT,

I am fine with both of your patches.

Acked-by: Waiman Long <longman@redhat.com>

Re: [PATCH cgroup/for-4.21 1/2] cpuset: Minor cgroup2 interface updates

[Peter Zijlstra]

On Tue, Nov 13, 2018 at 12:13:39PM -0800, Tejun Heo wrote:
> From b1e3aeb11c5e86ee0988a038c4e7682d6beaa977 Mon Sep 17 00:00:00 2001
> From: Tejun Heo <tj@kernel.org>
> Date: Tue, 13 Nov 2018 12:03:33 -0800
> 
> * Rename the partition file from "cpuset.sched.partition" to
>   "cpuset.cpus.partition".
> 
> * When writing to the partition file, drop "0" and "1" and only accept
>   "member" and "root".

Why though? The Changelog doesn't give rationale for the actual changes.

And I'm not sure I agree with either one of them.

The partition is a scheduling feature; and I like 0/1 much better to
type, so why not allow that?

Re: [PATCH cgroup/for-4.21 1/2] cpuset: Minor cgroup2 interface updates

[Tejun Heo]

Hello, Peter.

On Tue, Nov 20, 2018 at 01:46:24PM +0100, Peter Zijlstra wrote:
> Why though? The Changelog doesn't give rationale for the actual changes.

Ah yeah, sorry about that.

> And I'm not sure I agree with either one of them.
> 
> The partition is a scheduling feature;

So is everything with cpuset.cpus prefix.  They're all modifying how
scheduler handles the cpus.

> and I like 0/1 much better to type, so why not allow that?

Mostly for consistency and it's generally better to keep interfaces
minimal - e.g. what if we need to add support for more key words to
the file?  Would we assign incrementing integers to them?

Thanks.

-- 
tejun

Re: [PATCH cgroup/for-4.21 1/2] cpuset: Minor cgroup2 interface updates

[Peter Zijlstra]

On Tue, Nov 20, 2018 at 07:44:10AM -0800, Tejun Heo wrote:
> Hello, Peter.
> 
> On Tue, Nov 20, 2018 at 01:46:24PM +0100, Peter Zijlstra wrote:
> > Why though? The Changelog doesn't give rationale for the actual changes.
> 
> Ah yeah, sorry about that.
> 
> > And I'm not sure I agree with either one of them.
> > 
> > The partition is a scheduling feature;
> 
> So is everything with cpuset.cpus prefix.  They're all modifying how
> scheduler handles the cpus.

Fair enough I suppose.

> > and I like 0/1 much better to type, so why not allow that?
> 
> Mostly for consistency and it's generally better to keep interfaces
> minimal - e.g. what if we need to add support for more key words to
> the file?  Would we assign incrementing integers to them?

Or just not give them a numeric alias at all.

But numbers are more minimal than words, so lets just get rid of them
pesky word things ;-)

Alternatively; we could use kstrtobool(). That accepts a metric ton of
input ;-)