[PATCH] cgroup/cpuset: Clean up cpuset_task_status_allowed

[PATCH] cgroup/cpuset: Clean up cpuset_task_status_allowed

[haifeng.xu]

cpuset_task_status_allowed just shows mems_allowed status, so
rename it to task_mems_allowed. Moreover, it's only used in
proc_pid_status, so move it to fs/proc/array.c. There is no
intentional function change.

Signed-off-by: haifeng.xu <haifeng.xu@shopee.com>
---
 fs/proc/array.c        | 13 +++++++++++--
 include/linux/cpuset.h |  7 -------
 kernel/cgroup/cpuset.c |  9 ---------
 3 files changed, 11 insertions(+), 18 deletions(-)

diff --git a/fs/proc/array.c b/fs/proc/array.c
index 49283b8103c7..e6cdef8387e6 100644
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -80,7 +80,6 @@
 #include <linux/file.h>
 #include <linux/fdtable.h>
 #include <linux/times.h>
-#include <linux/cpuset.h>
 #include <linux/rcupdate.h>
 #include <linux/delayacct.h>
 #include <linux/seq_file.h>
@@ -413,6 +412,16 @@ static void task_cpus_allowed(struct seq_file *m, struct task_struct *task)
 		   cpumask_pr_args(&task->cpus_mask));
 }
 
+static void task_mems_allowed(struct seq_file *m, struct task_struct *task)
+{
+#ifdef CONFIG_CPUSETS
+	seq_printf(m, "Mems_allowed:\t%*pb\n",
+		   nodemask_pr_args(&task->mems_allowed));
+	seq_printf(m, "Mems_allowed_list:\t%*pbl\n",
+		   nodemask_pr_args(&task->mems_allowed));
+#endif
+}
+
 static inline void task_core_dumping(struct seq_file *m, struct task_struct *task)
 {
 	seq_put_decimal_ull(m, "CoreDumping:\t", !!task->signal->core_state);
@@ -449,7 +458,7 @@ int proc_pid_status(struct seq_file *m, struct pid_namespace *ns,
 	task_cap(m, task);
 	task_seccomp(m, task);
 	task_cpus_allowed(m, task);
-	cpuset_task_status_allowed(m, task);
+	task_mems_allowed(m, task);
 	task_context_switch_counts(m, task);
 	return 0;
 }
diff --git a/include/linux/cpuset.h b/include/linux/cpuset.h
index d58e0476ee8e..30b91116dd2f 100644
--- a/include/linux/cpuset.h
+++ b/include/linux/cpuset.h
@@ -112,8 +112,6 @@ extern int cpuset_mems_allowed_intersects(const struct task_struct *tsk1,
 extern int cpuset_memory_pressure_enabled;
 extern void __cpuset_memory_pressure_bump(void);
 
-extern void cpuset_task_status_allowed(struct seq_file *m,
-					struct task_struct *task);
 extern int proc_cpuset_show(struct seq_file *m, struct pid_namespace *ns,
 			    struct pid *pid, struct task_struct *tsk);
 
@@ -246,11 +244,6 @@ static inline int cpuset_mems_allowed_intersects(const struct task_struct *tsk1,
 
 static inline void cpuset_memory_pressure_bump(void) {}
 
-static inline void cpuset_task_status_allowed(struct seq_file *m,
-						struct task_struct *task)
-{
-}
-
 static inline int cpuset_mem_spread_node(void)
 {
 	return 0;
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 589827ccda8b..5798d4231662 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -4045,12 +4045,3 @@ int proc_cpuset_show(struct seq_file *m, struct pid_namespace *ns,
 	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)
-{
-	seq_printf(m, "Mems_allowed:\t%*pb\n",
-		   nodemask_pr_args(&task->mems_allowed));
-	seq_printf(m, "Mems_allowed_list:\t%*pbl\n",
-		   nodemask_pr_args(&task->mems_allowed));
-}
-- 
2.25.1

Re: [PATCH] cgroup/cpuset: Clean up cpuset_task_status_allowed

[Waiman Long]

On 11/25/22 02:51, haifeng.xu wrote:
> cpuset_task_status_allowed just shows mems_allowed status, so
> rename it to task_mems_allowed. Moreover, it's only used in
> proc_pid_status, so move it to fs/proc/array.c. There is no
> intentional function change.
>
> Signed-off-by: haifeng.xu <haifeng.xu@shopee.com>
> ---
>   fs/proc/array.c        | 13 +++++++++++--
>   include/linux/cpuset.h |  7 -------
>   kernel/cgroup/cpuset.c |  9 ---------
>   3 files changed, 11 insertions(+), 18 deletions(-)
>
> diff --git a/fs/proc/array.c b/fs/proc/array.c
> index 49283b8103c7..e6cdef8387e6 100644
> --- a/fs/proc/array.c
> +++ b/fs/proc/array.c
> @@ -80,7 +80,6 @@
>   #include <linux/file.h>
>   #include <linux/fdtable.h>
>   #include <linux/times.h>
> -#include <linux/cpuset.h>
>   #include <linux/rcupdate.h>
>   #include <linux/delayacct.h>
>   #include <linux/seq_file.h>
> @@ -413,6 +412,16 @@ static void task_cpus_allowed(struct seq_file *m, struct task_struct *task)
>   		   cpumask_pr_args(&task->cpus_mask));
>   }
>   
> +static void task_mems_allowed(struct seq_file *m, struct task_struct *task)
> +{
> +#ifdef CONFIG_CPUSETS
> +	seq_printf(m, "Mems_allowed:\t%*pb\n",
> +		   nodemask_pr_args(&task->mems_allowed));
> +	seq_printf(m, "Mems_allowed_list:\t%*pbl\n",
> +		   nodemask_pr_args(&task->mems_allowed));
> +#endif
> +}
> +
>   static inline void task_core_dumping(struct seq_file *m, struct task_struct *task)
>   {
>   	seq_put_decimal_ull(m, "CoreDumping:\t", !!task->signal->core_state);
> @@ -449,7 +458,7 @@ int proc_pid_status(struct seq_file *m, struct pid_namespace *ns,
>   	task_cap(m, task);
>   	task_seccomp(m, task);
>   	task_cpus_allowed(m, task);
> -	cpuset_task_status_allowed(m, task);
> +	task_mems_allowed(m, task);
>   	task_context_switch_counts(m, task);
>   	return 0;
>   }
> diff --git a/include/linux/cpuset.h b/include/linux/cpuset.h
> index d58e0476ee8e..30b91116dd2f 100644
> --- a/include/linux/cpuset.h
> +++ b/include/linux/cpuset.h
> @@ -112,8 +112,6 @@ extern int cpuset_mems_allowed_intersects(const struct task_struct *tsk1,
>   extern int cpuset_memory_pressure_enabled;
>   extern void __cpuset_memory_pressure_bump(void);
>   
> -extern void cpuset_task_status_allowed(struct seq_file *m,
> -					struct task_struct *task);
>   extern int proc_cpuset_show(struct seq_file *m, struct pid_namespace *ns,
>   			    struct pid *pid, struct task_struct *tsk);
>   
> @@ -246,11 +244,6 @@ static inline int cpuset_mems_allowed_intersects(const struct task_struct *tsk1,
>   
>   static inline void cpuset_memory_pressure_bump(void) {}
>   
> -static inline void cpuset_task_status_allowed(struct seq_file *m,
> -						struct task_struct *task)
> -{
> -}
> -
>   static inline int cpuset_mem_spread_node(void)
>   {
>   	return 0;
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 589827ccda8b..5798d4231662 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -4045,12 +4045,3 @@ int proc_cpuset_show(struct seq_file *m, struct pid_namespace *ns,
>   	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)
> -{
> -	seq_printf(m, "Mems_allowed:\t%*pb\n",
> -		   nodemask_pr_args(&task->mems_allowed));
> -	seq_printf(m, "Mems_allowed_list:\t%*pbl\n",
> -		   nodemask_pr_args(&task->mems_allowed));
> -}

It is just a minor cleanup. I think that is OK.

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

Re: [PATCH] cgroup/cpuset: Clean up cpuset_task_status_allowed

[Tejun Heo]

On Fri, Nov 25, 2022 at 07:51:33AM +0000, haifeng.xu wrote:
> cpuset_task_status_allowed just shows mems_allowed status, so
> rename it to task_mems_allowed. Moreover, it's only used in
> proc_pid_status, so move it to fs/proc/array.c. There is no
> intentional function change.
> 
> Signed-off-by: haifeng.xu <haifeng.xu@shopee.com>

mems_allowed being a very much cpuset feature, I don't see how this is an
improvement. The new code is different and can be another way of doing it,
sure, but there's no inherent benefit to it. What's the point of the churn?

Thanks.

-- 
tejun

Re: [PATCH] cgroup/cpuset: Clean up cpuset_task_status_allowed

[Haifeng Xu]

On 2022/11/29 01:07, Tejun Heo wrote:
> On Fri, Nov 25, 2022 at 07:51:33AM +0000, haifeng.xu wrote:
>> cpuset_task_status_allowed just shows mems_allowed status, so
>> rename it to task_mems_allowed. Moreover, it's only used in
>> proc_pid_status, so move it to fs/proc/array.c. There is no
>> intentional function change.
>>
>> Signed-off-by: haifeng.xu <haifeng.xu@shopee.com>
> 
> mems_allowed being a very much cpuset feature, I don't see how this is an
> improvement. The new code is different and can be another way of doing it,
> sure, but there's no inherent benefit to it. What's the point of the churn?
> 
> Thanks.
> 

Hi, tejun.

In proc_pid_status, task_cpus_allowed is used to show cpus_allowed, similar to that,
task_mems_allowed is more specific to show mems_allowed. Also cpuset_task_status_allowed
is used to dispaly memory status in '/proc/<pid>/status' and isn't used in other files, so
keep it in the fs/proc/array.c.

Thanks,
Haifeng.