Re: [PATCH v2 -next 09/11] cgroup/cpuset: move v1 interfaces to cpuset-v1.c

From: Waiman Long
Date: Mon Aug 26 2024 - 15:30:37 EST


On 8/26/24 09:27, Chen Ridong wrote:
Move legacy cpuset controller interfaces files and corresponding code
into cpuset-v1.c. 'update_flag', 'cpuset_write_resmask' and
'cpuset_common_seq_show' are also used for v1, so declare them in
cpuset-internal.h.

'cpuset_write_s64', 'cpuset_read_s64' and 'fmeter_getrate' are only used
cpuset-v1.c now, make it static.

Signed-off-by: Chen Ridong <chenridong@xxxxxxxxxx>
---
kernel/cgroup/cpuset-internal.h | 9 +-
kernel/cgroup/cpuset-v1.c | 194 ++++++++++++++++++++++++++++++-
kernel/cgroup/cpuset.c | 195 +-------------------------------
3 files changed, 199 insertions(+), 199 deletions(-)

diff --git a/kernel/cgroup/cpuset-internal.h b/kernel/cgroup/cpuset-internal.h
index 07551ff0812e..a6c71c86e58d 100644
--- a/kernel/cgroup/cpuset-internal.h
+++ b/kernel/cgroup/cpuset-internal.h
@@ -271,15 +271,16 @@ void callback_lock_irq(void);
void callback_unlock_irq(void);
void update_tasks_cpumask(struct cpuset *cs, struct cpumask *new_cpus);
void update_tasks_nodemask(struct cpuset *cs);
+int update_flag(cpuset_flagbits_t bit, struct cpuset *cs, int turning_on);
+ssize_t cpuset_write_resmask(struct kernfs_open_file *of,
+ char *buf, size_t nbytes, loff_t off);
+int cpuset_common_seq_show(struct seq_file *sf, void *v);
/*
* cpuset-v1.c
*/
+extern struct cftype legacy_files[];

The legacy_files name is rather generic. By making it globally visible within the kernel, it runs the risk conflicting with another variable of the same name (namespace pollution). I would suggest adding "cpuset_" prefix to make it unique to cpuset.

The following functions also have similar issue.

- update_flag()
- update_tasks_flags()
- validate_change_legacy()
- callback_lock_irq()
- callback_unlock_irq().

Another alternative is to include cpuset-v1.c directly into cpuset.c like

#ifdef CONFIG_CPUSETS_V1
#include "cpuset-v1.c"
#else
   ....
#endif

Then you don't need to change the names and will not need cpuset-internal.h. It is up to you to decide what you want to do.

Cheers,
Longman