init/Kconfig | 2 +- kernel/cgroup/cpuset-v1.c | 40 +++++++++++++++++++++++++++++++++++ kernel/cgroup/cpuset.c | 44 --------------------------------------- 3 files changed, 41 insertions(+), 45 deletions(-)
The cpuset file is a legacy attribute that is bound primarily to cpuset
v1 (and it'd be equal to effective /proc/$pid/cgroup path on the unified
hierarchy).
Followup to commit b0ced9d378d49 ("cgroup/cpuset: move v1 interfaces to
cpuset-v1.c") and hide CONFIG_PROC_PID_CPUSET under CONFIG_CPUSETS_V1.
Drop an obsolete comment too.
Signed-off-by: Michal Koutný <mkoutny@suse.com>
---
init/Kconfig | 2 +-
kernel/cgroup/cpuset-v1.c | 40 +++++++++++++++++++++++++++++++++++
kernel/cgroup/cpuset.c | 44 ---------------------------------------
3 files changed, 41 insertions(+), 45 deletions(-)
diff --git a/init/Kconfig b/init/Kconfig
index a20e6efd3f0fb..2a1d5ef3fa48e 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1190,7 +1190,7 @@ config CPUSETS_V1
config PROC_PID_CPUSET
bool "Include legacy /proc/<pid>/cpuset file"
- depends on CPUSETS
+ depends on CPUSETS_V1
default y
config CGROUP_DEVICE
diff --git a/kernel/cgroup/cpuset-v1.c b/kernel/cgroup/cpuset-v1.c
index 25c1d7b77e2f2..fff1a38f2725f 100644
--- a/kernel/cgroup/cpuset-v1.c
+++ b/kernel/cgroup/cpuset-v1.c
@@ -373,6 +373,46 @@ int cpuset1_validate_change(struct cpuset *cur, struct cpuset *trial)
return ret;
}
+#ifdef CONFIG_PROC_PID_CPUSET
+/*
+ * proc_cpuset_show()
+ * - Print tasks cpuset path into seq_file.
+ * - Used for /proc/<pid>/cpuset.
+ */
+int proc_cpuset_show(struct seq_file *m, struct pid_namespace *ns,
+ struct pid *pid, struct task_struct *tsk)
+{
+ char *buf;
+ struct cgroup_subsys_state *css;
+ int retval;
+
+ retval = -ENOMEM;
+ buf = kmalloc(PATH_MAX, GFP_KERNEL);
+ if (!buf)
+ goto out;
+
+ rcu_read_lock();
+ spin_lock_irq(&css_set_lock);
+ css = task_css(tsk, cpuset_cgrp_id);
+ retval = cgroup_path_ns_locked(css->cgroup, buf, PATH_MAX,
+ current->nsproxy->cgroup_ns);
+ spin_unlock_irq(&css_set_lock);
+ rcu_read_unlock();
+
+ if (retval == -E2BIG)
+ retval = -ENAMETOOLONG;
+ if (retval < 0)
+ goto out_free;
+ seq_puts(m, buf);
+ seq_putc(m, '\n');
+ retval = 0;
+out_free:
+ kfree(buf);
+out:
+ return retval;
+}
+#endif /* CONFIG_PROC_PID_CPUSET */
+
static u64 cpuset_read_u64(struct cgroup_subsys_state *css, struct cftype *cft)
{
struct cpuset *cs = css_cs(css);
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 0f910c828973a..7d6e8db234290 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -4244,50 +4244,6 @@ void cpuset_print_current_mems_allowed(void)
rcu_read_unlock();
}
-#ifdef CONFIG_PROC_PID_CPUSET
-/*
- * proc_cpuset_show()
- * - Print tasks cpuset path into seq_file.
- * - Used for /proc/<pid>/cpuset.
- * - No need to task_lock(tsk) on this tsk->cpuset reference, as it
- * doesn't really matter if tsk->cpuset changes after we read it,
- * and we take cpuset_mutex, keeping cpuset_attach() from changing it
- * anyway.
- */
-int proc_cpuset_show(struct seq_file *m, struct pid_namespace *ns,
- struct pid *pid, struct task_struct *tsk)
-{
- char *buf;
- struct cgroup_subsys_state *css;
- int retval;
-
- retval = -ENOMEM;
- buf = kmalloc(PATH_MAX, GFP_KERNEL);
- if (!buf)
- goto out;
-
- rcu_read_lock();
- spin_lock_irq(&css_set_lock);
- css = task_css(tsk, cpuset_cgrp_id);
- retval = cgroup_path_ns_locked(css->cgroup, buf, PATH_MAX,
- current->nsproxy->cgroup_ns);
- spin_unlock_irq(&css_set_lock);
- rcu_read_unlock();
-
- if (retval == -E2BIG)
- retval = -ENAMETOOLONG;
- if (retval < 0)
- goto out_free;
- seq_puts(m, buf);
- seq_putc(m, '\n');
- retval = 0;
-out_free:
- kfree(buf);
-out:
- return retval;
-}
-#endif /* CONFIG_PROC_PID_CPUSET */
-
/* Display task mems_allowed in /proc/<pid>/status file. */
void cpuset_task_status_allowed(struct seq_file *m, struct task_struct *task)
{
--
2.47.1
On 1/16/25 4:56 AM, Michal Koutný wrote:
> The cpuset file is a legacy attribute that is bound primarily to cpuset
> v1 (and it'd be equal to effective /proc/$pid/cgroup path on the unified
> hierarchy).
>
> Followup to commit b0ced9d378d49 ("cgroup/cpuset: move v1 interfaces to
> cpuset-v1.c") and hide CONFIG_PROC_PID_CPUSET under CONFIG_CPUSETS_V1.
> Drop an obsolete comment too.
>
> Signed-off-by: Michal Koutný <mkoutny@suse.com>
I do have some reservation in taking out /proc/<pid>/cpuset by default
as CPUSETS_V1 is off by default. This may break some existing user scripts.
Also the statement that the cpuset file is the same as the path in
cgroup file in unified hierarchy is not true if that cgroup doesn't have
cpuset enabled.
Cheers,
Longman
On Thu, Jan 16, 2025 at 08:40:56AM -0500, Waiman Long <llong@redhat.com> wrote: > I do have some reservation in taking out /proc/<pid>/cpuset by default as > CPUSETS_V1 is off by default. This may break some existing user scripts. Cannot be /proc/$pid/cpuset declared a v1 feature? Similar to cpuset fs (that is under CPUSETS_V1). If there's a breakage, the user is not non-v1 ready and CPUSETS_V1 must be enabled. > Also the statement that the cpuset file is the same as the path in cgroup > file in unified hierarchy is not true if that cgroup doesn't have cpuset > enabled. (I tried to capture that with the "effective path", I should reword it more clearly.) Thanks, Michal
Hello, On Thu, Jan 16, 2025 at 03:05:32PM +0100, Michal Koutný wrote: > On Thu, Jan 16, 2025 at 08:40:56AM -0500, Waiman Long <llong@redhat.com> wrote: > > I do have some reservation in taking out /proc/<pid>/cpuset by default as > > CPUSETS_V1 is off by default. This may break some existing user scripts. > > Cannot be /proc/$pid/cpuset declared a v1 feature? > Similar to cpuset fs (that is under CPUSETS_V1). If there's a breakage, > the user is not non-v1 ready and CPUSETS_V1 must be enabled. I think we can try given that the config is providing an exit path. After all, users who were depending on cpusets on v1 are in the same boat. Thanks. -- tejun
On 1/16/25 12:25 PM, Tejun Heo wrote: > Hello, > > On Thu, Jan 16, 2025 at 03:05:32PM +0100, Michal Koutný wrote: >> On Thu, Jan 16, 2025 at 08:40:56AM -0500, Waiman Long <llong@redhat.com> wrote: >>> I do have some reservation in taking out /proc/<pid>/cpuset by default as >>> CPUSETS_V1 is off by default. This may break some existing user scripts. >> Cannot be /proc/$pid/cpuset declared a v1 feature? >> Similar to cpuset fs (that is under CPUSETS_V1). If there's a breakage, >> the user is not non-v1 ready and CPUSETS_V1 must be enabled. > I think we can try given that the config is providing an exit path. After > all, users who were depending on cpusets on v1 are in the same boat. I am not totally against this, but I think we need to make the relationship between the CPUSETS_V1 config and /proc/<pid>/cpuset file more visible if we want to go this route. We should update the help text of CPUSETS_V1 config entry to emphasize this. Cheers, Longman
On Thu, Jan 16, 2025 at 02:15:00PM -0500, Waiman Long wrote: > > On 1/16/25 12:25 PM, Tejun Heo wrote: > > Hello, > > > > On Thu, Jan 16, 2025 at 03:05:32PM +0100, Michal Koutný wrote: > > > On Thu, Jan 16, 2025 at 08:40:56AM -0500, Waiman Long <llong@redhat.com> wrote: > > > > I do have some reservation in taking out /proc/<pid>/cpuset by default as > > > > CPUSETS_V1 is off by default. This may break some existing user scripts. > > > Cannot be /proc/$pid/cpuset declared a v1 feature? > > > Similar to cpuset fs (that is under CPUSETS_V1). If there's a breakage, > > > the user is not non-v1 ready and CPUSETS_V1 must be enabled. > > I think we can try given that the config is providing an exit path. After > > all, users who were depending on cpusets on v1 are in the same boat. > > I am not totally against this, but I think we need to make the relationship > between the CPUSETS_V1 config and /proc/<pid>/cpuset file more visible if we > want to go this route. We should update the help text of CPUSETS_V1 config > entry to emphasize this. +1 Thanks. -- tejun
The cpuset file is a legacy attribute that is bound primarily to cpuset
v1 hierarchy (equivalent information is available in /proc/$pid/cgroup path
on the unified hierarchy in conjunction with respective
cgroup.controllers showing where cpuset controller is enabled).
Followup to commit b0ced9d378d49 ("cgroup/cpuset: move v1 interfaces to
cpuset-v1.c") and hide CONFIG_PROC_PID_CPUSET under CONFIG_CPUSETS_V1.
Drop an obsolete comment too.
Signed-off-by: Michal Koutný <mkoutny@suse.com>
---
init/Kconfig | 5 +++--
kernel/cgroup/cpuset-v1.c | 40 +++++++++++++++++++++++++++++++++++
kernel/cgroup/cpuset.c | 44 ---------------------------------------
3 files changed, 43 insertions(+), 46 deletions(-)
v2 changes:
- explicitly say what's part of CPUSETS_V1
- commit message wrt effective paths
diff --git a/init/Kconfig b/init/Kconfig
index a20e6efd3f0fb..2f3121c49ed23 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1182,7 +1182,8 @@ config CPUSETS_V1
help
Legacy cgroup v1 cpusets controller which has been deprecated by
cgroup v2 implementation. The v1 is there for legacy applications
- which haven't migrated to the new cgroup v2 interface yet. If you
+ which haven't migrated to the new cgroup v2 interface yet. Legacy
+ interface includes cpuset filesystem and /proc/<pid>/cpuset. If you
do not have any such application then you are completely fine leaving
this option disabled.
@@ -1190,7 +1191,7 @@ config CPUSETS_V1
config PROC_PID_CPUSET
bool "Include legacy /proc/<pid>/cpuset file"
- depends on CPUSETS
+ depends on CPUSETS_V1
default y
config CGROUP_DEVICE
diff --git a/kernel/cgroup/cpuset-v1.c b/kernel/cgroup/cpuset-v1.c
index 25c1d7b77e2f2..fff1a38f2725f 100644
--- a/kernel/cgroup/cpuset-v1.c
+++ b/kernel/cgroup/cpuset-v1.c
@@ -373,6 +373,46 @@ int cpuset1_validate_change(struct cpuset *cur, struct cpuset *trial)
return ret;
}
+#ifdef CONFIG_PROC_PID_CPUSET
+/*
+ * proc_cpuset_show()
+ * - Print tasks cpuset path into seq_file.
+ * - Used for /proc/<pid>/cpuset.
+ */
+int proc_cpuset_show(struct seq_file *m, struct pid_namespace *ns,
+ struct pid *pid, struct task_struct *tsk)
+{
+ char *buf;
+ struct cgroup_subsys_state *css;
+ int retval;
+
+ retval = -ENOMEM;
+ buf = kmalloc(PATH_MAX, GFP_KERNEL);
+ if (!buf)
+ goto out;
+
+ rcu_read_lock();
+ spin_lock_irq(&css_set_lock);
+ css = task_css(tsk, cpuset_cgrp_id);
+ retval = cgroup_path_ns_locked(css->cgroup, buf, PATH_MAX,
+ current->nsproxy->cgroup_ns);
+ spin_unlock_irq(&css_set_lock);
+ rcu_read_unlock();
+
+ if (retval == -E2BIG)
+ retval = -ENAMETOOLONG;
+ if (retval < 0)
+ goto out_free;
+ seq_puts(m, buf);
+ seq_putc(m, '\n');
+ retval = 0;
+out_free:
+ kfree(buf);
+out:
+ return retval;
+}
+#endif /* CONFIG_PROC_PID_CPUSET */
+
static u64 cpuset_read_u64(struct cgroup_subsys_state *css, struct cftype *cft)
{
struct cpuset *cs = css_cs(css);
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 0f910c828973a..7d6e8db234290 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -4244,50 +4244,6 @@ void cpuset_print_current_mems_allowed(void)
rcu_read_unlock();
}
-#ifdef CONFIG_PROC_PID_CPUSET
-/*
- * proc_cpuset_show()
- * - Print tasks cpuset path into seq_file.
- * - Used for /proc/<pid>/cpuset.
- * - No need to task_lock(tsk) on this tsk->cpuset reference, as it
- * doesn't really matter if tsk->cpuset changes after we read it,
- * and we take cpuset_mutex, keeping cpuset_attach() from changing it
- * anyway.
- */
-int proc_cpuset_show(struct seq_file *m, struct pid_namespace *ns,
- struct pid *pid, struct task_struct *tsk)
-{
- char *buf;
- struct cgroup_subsys_state *css;
- int retval;
-
- retval = -ENOMEM;
- buf = kmalloc(PATH_MAX, GFP_KERNEL);
- if (!buf)
- goto out;
-
- rcu_read_lock();
- spin_lock_irq(&css_set_lock);
- css = task_css(tsk, cpuset_cgrp_id);
- retval = cgroup_path_ns_locked(css->cgroup, buf, PATH_MAX,
- current->nsproxy->cgroup_ns);
- spin_unlock_irq(&css_set_lock);
- rcu_read_unlock();
-
- if (retval == -E2BIG)
- retval = -ENAMETOOLONG;
- if (retval < 0)
- goto out_free;
- seq_puts(m, buf);
- seq_putc(m, '\n');
- retval = 0;
-out_free:
- kfree(buf);
-out:
- return retval;
-}
-#endif /* CONFIG_PROC_PID_CPUSET */
-
/* Display task mems_allowed in /proc/<pid>/status file. */
void cpuset_task_status_allowed(struct seq_file *m, struct task_struct *task)
{
--
2.47.1
On Fri, Jan 17, 2025 at 11:44:36AM +0100, Michal Koutný wrote:
> The cpuset file is a legacy attribute that is bound primarily to cpuset
> v1 hierarchy (equivalent information is available in /proc/$pid/cgroup path
> on the unified hierarchy in conjunction with respective
> cgroup.controllers showing where cpuset controller is enabled).
>
> Followup to commit b0ced9d378d49 ("cgroup/cpuset: move v1 interfaces to
> cpuset-v1.c") and hide CONFIG_PROC_PID_CPUSET under CONFIG_CPUSETS_V1.
> Drop an obsolete comment too.
>
> Signed-off-by: Michal Koutný <mkoutny@suse.com>
Applied to cgroup/for-6.14.
Thanks.
--
tejun
On 1/17/25 5:44 AM, Michal Koutný wrote:
> The cpuset file is a legacy attribute that is bound primarily to cpuset
> v1 hierarchy (equivalent information is available in /proc/$pid/cgroup path
> on the unified hierarchy in conjunction with respective
> cgroup.controllers showing where cpuset controller is enabled).
>
> Followup to commit b0ced9d378d49 ("cgroup/cpuset: move v1 interfaces to
> cpuset-v1.c") and hide CONFIG_PROC_PID_CPUSET under CONFIG_CPUSETS_V1.
> Drop an obsolete comment too.
>
> Signed-off-by: Michal Koutný <mkoutny@suse.com>
> ---
> init/Kconfig | 5 +++--
> kernel/cgroup/cpuset-v1.c | 40 +++++++++++++++++++++++++++++++++++
> kernel/cgroup/cpuset.c | 44 ---------------------------------------
> 3 files changed, 43 insertions(+), 46 deletions(-)
>
> v2 changes:
> - explicitly say what's part of CPUSETS_V1
> - commit message wrt effective paths
>
> diff --git a/init/Kconfig b/init/Kconfig
> index a20e6efd3f0fb..2f3121c49ed23 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -1182,7 +1182,8 @@ config CPUSETS_V1
> help
> Legacy cgroup v1 cpusets controller which has been deprecated by
> cgroup v2 implementation. The v1 is there for legacy applications
> - which haven't migrated to the new cgroup v2 interface yet. If you
> + which haven't migrated to the new cgroup v2 interface yet. Legacy
> + interface includes cpuset filesystem and /proc/<pid>/cpuset. If you
> do not have any such application then you are completely fine leaving
> this option disabled.
>
> @@ -1190,7 +1191,7 @@ config CPUSETS_V1
>
> config PROC_PID_CPUSET
> bool "Include legacy /proc/<pid>/cpuset file"
> - depends on CPUSETS
> + depends on CPUSETS_V1
> default y
>
> config CGROUP_DEVICE
> diff --git a/kernel/cgroup/cpuset-v1.c b/kernel/cgroup/cpuset-v1.c
> index 25c1d7b77e2f2..fff1a38f2725f 100644
> --- a/kernel/cgroup/cpuset-v1.c
> +++ b/kernel/cgroup/cpuset-v1.c
> @@ -373,6 +373,46 @@ int cpuset1_validate_change(struct cpuset *cur, struct cpuset *trial)
> return ret;
> }
>
> +#ifdef CONFIG_PROC_PID_CPUSET
> +/*
> + * proc_cpuset_show()
> + * - Print tasks cpuset path into seq_file.
> + * - Used for /proc/<pid>/cpuset.
> + */
> +int proc_cpuset_show(struct seq_file *m, struct pid_namespace *ns,
> + struct pid *pid, struct task_struct *tsk)
> +{
> + char *buf;
> + struct cgroup_subsys_state *css;
> + int retval;
> +
> + retval = -ENOMEM;
> + buf = kmalloc(PATH_MAX, GFP_KERNEL);
> + if (!buf)
> + goto out;
> +
> + rcu_read_lock();
> + spin_lock_irq(&css_set_lock);
> + css = task_css(tsk, cpuset_cgrp_id);
> + retval = cgroup_path_ns_locked(css->cgroup, buf, PATH_MAX,
> + current->nsproxy->cgroup_ns);
> + spin_unlock_irq(&css_set_lock);
> + rcu_read_unlock();
> +
> + if (retval == -E2BIG)
> + retval = -ENAMETOOLONG;
> + if (retval < 0)
> + goto out_free;
> + seq_puts(m, buf);
> + seq_putc(m, '\n');
> + retval = 0;
> +out_free:
> + kfree(buf);
> +out:
> + return retval;
> +}
> +#endif /* CONFIG_PROC_PID_CPUSET */
> +
> static u64 cpuset_read_u64(struct cgroup_subsys_state *css, struct cftype *cft)
> {
> struct cpuset *cs = css_cs(css);
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 0f910c828973a..7d6e8db234290 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -4244,50 +4244,6 @@ void cpuset_print_current_mems_allowed(void)
> rcu_read_unlock();
> }
>
> -#ifdef CONFIG_PROC_PID_CPUSET
> -/*
> - * proc_cpuset_show()
> - * - Print tasks cpuset path into seq_file.
> - * - Used for /proc/<pid>/cpuset.
> - * - No need to task_lock(tsk) on this tsk->cpuset reference, as it
> - * doesn't really matter if tsk->cpuset changes after we read it,
> - * and we take cpuset_mutex, keeping cpuset_attach() from changing it
> - * anyway.
> - */
> -int proc_cpuset_show(struct seq_file *m, struct pid_namespace *ns,
> - struct pid *pid, struct task_struct *tsk)
> -{
> - char *buf;
> - struct cgroup_subsys_state *css;
> - int retval;
> -
> - retval = -ENOMEM;
> - buf = kmalloc(PATH_MAX, GFP_KERNEL);
> - if (!buf)
> - goto out;
> -
> - rcu_read_lock();
> - spin_lock_irq(&css_set_lock);
> - css = task_css(tsk, cpuset_cgrp_id);
> - retval = cgroup_path_ns_locked(css->cgroup, buf, PATH_MAX,
> - current->nsproxy->cgroup_ns);
> - spin_unlock_irq(&css_set_lock);
> - rcu_read_unlock();
> -
> - if (retval == -E2BIG)
> - retval = -ENAMETOOLONG;
> - if (retval < 0)
> - goto out_free;
> - seq_puts(m, buf);
> - seq_putc(m, '\n');
> - retval = 0;
> -out_free:
> - kfree(buf);
> -out:
> - return retval;
> -}
> -#endif /* CONFIG_PROC_PID_CPUSET */
> -
> /* Display task mems_allowed in /proc/<pid>/status file. */
> void cpuset_task_status_allowed(struct seq_file *m, struct task_struct *task)
> {
Acked-by: Waiman Long <longman@redhat.com>
© 2016 - 2025 Red Hat, Inc.