* [PATCH cgroup/for-4.21 1/2] cpuset: Minor cgroup2 interface updates
@ 2018-11-13 20:13 Tejun Heo
2018-11-13 20:16 ` [PATCH cgroup/for-4.21 2/2] cgroup: Add .__DEBUG__. prefix to debug file names Tejun Heo
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Tejun Heo @ 2018-11-13 20:13 UTC (permalink / raw)
To: Peter Zijlstra, Waiman Long
Cc: Li Zefan, Johannes Weiner, cgroups, linux-kernel
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH cgroup/for-4.21 2/2] cgroup: Add .__DEBUG__. prefix to debug file names
2018-11-13 20:13 [PATCH cgroup/for-4.21 1/2] cpuset: Minor cgroup2 interface updates Tejun Heo
@ 2018-11-13 20:16 ` Tejun Heo
2018-11-20 12:46 ` Peter Zijlstra
2018-11-14 2:09 ` [PATCH cgroup/for-4.21 1/2] cpuset: Minor cgroup2 interface updates Waiman Long
2018-11-20 12:46 ` Peter Zijlstra
2 siblings, 1 reply; 7+ messages in thread
From: Tejun Heo @ 2018-11-13 20:16 UTC (permalink / raw)
To: Peter Zijlstra, Waiman Long
Cc: Li Zefan, Johannes Weiner, cgroups, linux-kernel
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH cgroup/for-4.21 1/2] cpuset: Minor cgroup2 interface updates
2018-11-13 20:13 [PATCH cgroup/for-4.21 1/2] cpuset: Minor cgroup2 interface updates Tejun Heo
2018-11-13 20:16 ` [PATCH cgroup/for-4.21 2/2] cgroup: Add .__DEBUG__. prefix to debug file names Tejun Heo
@ 2018-11-14 2:09 ` Waiman Long
2018-11-20 12:46 ` Peter Zijlstra
2 siblings, 0 replies; 7+ messages in thread
From: Waiman Long @ 2018-11-14 2:09 UTC (permalink / raw)
To: Tejun Heo, Peter Zijlstra
Cc: Li Zefan, Johannes Weiner, cgroups, linux-kernel
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>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH cgroup/for-4.21 1/2] cpuset: Minor cgroup2 interface updates
2018-11-13 20:13 [PATCH cgroup/for-4.21 1/2] cpuset: Minor cgroup2 interface updates Tejun Heo
2018-11-13 20:16 ` [PATCH cgroup/for-4.21 2/2] cgroup: Add .__DEBUG__. prefix to debug file names Tejun Heo
2018-11-14 2:09 ` [PATCH cgroup/for-4.21 1/2] cpuset: Minor cgroup2 interface updates Waiman Long
@ 2018-11-20 12:46 ` Peter Zijlstra
2018-11-20 15:44 ` Tejun Heo
2 siblings, 1 reply; 7+ messages in thread
From: Peter Zijlstra @ 2018-11-20 12:46 UTC (permalink / raw)
To: Tejun Heo; +Cc: Waiman Long, Li Zefan, Johannes Weiner, cgroups, linux-kernel
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?
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH cgroup/for-4.21 2/2] cgroup: Add .__DEBUG__. prefix to debug file names
2018-11-13 20:16 ` [PATCH cgroup/for-4.21 2/2] cgroup: Add .__DEBUG__. prefix to debug file names Tejun Heo
@ 2018-11-20 12:46 ` Peter Zijlstra
0 siblings, 0 replies; 7+ messages in thread
From: Peter Zijlstra @ 2018-11-20 12:46 UTC (permalink / raw)
To: Tejun Heo; +Cc: Waiman Long, Li Zefan, Johannes Weiner, cgroups, linux-kernel
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH cgroup/for-4.21 1/2] cpuset: Minor cgroup2 interface updates
2018-11-20 12:46 ` Peter Zijlstra
@ 2018-11-20 15:44 ` Tejun Heo
2018-11-20 17:19 ` Peter Zijlstra
0 siblings, 1 reply; 7+ messages in thread
From: Tejun Heo @ 2018-11-20 15:44 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Waiman Long, Li Zefan, Johannes Weiner, cgroups, linux-kernel
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH cgroup/for-4.21 1/2] cpuset: Minor cgroup2 interface updates
2018-11-20 15:44 ` Tejun Heo
@ 2018-11-20 17:19 ` Peter Zijlstra
0 siblings, 0 replies; 7+ messages in thread
From: Peter Zijlstra @ 2018-11-20 17:19 UTC (permalink / raw)
To: Tejun Heo; +Cc: Waiman Long, Li Zefan, Johannes Weiner, cgroups, linux-kernel
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 ;-)
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-11-20 17:20 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-13 20:13 [PATCH cgroup/for-4.21 1/2] cpuset: Minor cgroup2 interface updates Tejun Heo
2018-11-13 20:16 ` [PATCH cgroup/for-4.21 2/2] cgroup: Add .__DEBUG__. prefix to debug file names Tejun Heo
2018-11-20 12:46 ` Peter Zijlstra
2018-11-14 2:09 ` [PATCH cgroup/for-4.21 1/2] cpuset: Minor cgroup2 interface updates Waiman Long
2018-11-20 12:46 ` Peter Zijlstra
2018-11-20 15:44 ` Tejun Heo
2018-11-20 17:19 ` Peter Zijlstra
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).