Re: [PATCH v1 -next 10/11] cgroup/cpuset: guard cpuset-v1 code under CONFIG_CPUSETS_V1

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 2024/8/26 10:18, Waiman Long wrote:
On 8/25/24 21:34, Chen Ridong wrote:


On 2024/8/26 1:25, Waiman Long wrote:
On 8/23/24 06:01, Chen Ridong wrote:
This patch introduces CONFIG_CPUSETS_V1 and guard cpuset-v1 code under
CONFIG_CPUSETS_V1. The default value of CONFIG_CPUSETS_V1 is N, so that
user who adopted v2 don't have 'pay' for cpuset v1.
Besides, to make code succinct, rename '__cpuset_memory_pressure_bump()'
to 'cpuset_memory_pressure_bump()', and expose it to the world, which
takes place of the old mocro 'cpuset_memory_pressure_bump'.

Signed-off-by: Chen Ridong <chenridong@xxxxxxxxxx>
---
  include/linux/cpuset.h          |  8 +-------
  init/Kconfig                    | 13 +++++++++++++
  kernel/cgroup/Makefile          |  3 ++-
  kernel/cgroup/cpuset-internal.h | 15 +++++++++++++++
  kernel/cgroup/cpuset-v1.c       | 10 ++++++----
  kernel/cgroup/cpuset.c          |  2 ++
  6 files changed, 39 insertions(+), 12 deletions(-)

diff --git a/include/linux/cpuset.h b/include/linux/cpuset.h
index 2a6981eeebf8..f91f1f61f482 100644
--- a/include/linux/cpuset.h
+++ b/include/linux/cpuset.h
@@ -99,13 +99,7 @@ static inline bool cpuset_zone_allowed(struct zone *z, gfp_t gfp_mask)   extern int cpuset_mems_allowed_intersects(const struct task_struct *tsk1,
                        const struct task_struct *tsk2);
-#define cpuset_memory_pressure_bump()                 \
-    do {                            \
-        if (cpuset_memory_pressure_enabled)        \
-            __cpuset_memory_pressure_bump();    \
-    } while (0)
-extern int cpuset_memory_pressure_enabled;
-extern void __cpuset_memory_pressure_bump(void);
+extern void cpuset_memory_pressure_bump(void);
  extern void cpuset_task_status_allowed(struct seq_file *m,
                      struct task_struct *task);

As you are introducing a new CONFIG_CPUSET_V1 kconfig option, you can use it to eliminate a useless function call if cgroup v1 isn't enabled. Not just this function, all the v1 specific functions should have a !CONFIG_CPUSET_V1 version that can be optimized out.

Cheers,
Longman


Hi, Longman, currently, we have only added one place to manage all the functions whose are empty if !CONFIG_CPUSET_V1 is not defined , and the caller does not need to care about using cpuset v1 or v2, which makes it succinct.

However, we have to add !CONFIG_CPUSET_V1 to many places to avoid useless function calls. I am not opposed to do this if you thick it 'a better implementation.

You don't need to touch any files that use these cgroup v1 only functions. You only need to add some #ifdef code in the cpuset.h header file like

#ifdef CONFIG_CPUSET_V1
extern void cpuset_memory_pressure_bump(void);
#else
static inline void cpuset_memory_pressure_bump(void) { }
#endif

BTW, try not to introduce unnecessary performance regression. The reason cpuset_memory_pressure_bump() is a macro is to avoid a function call if cpuset_memory_pressure_enabled isn't set which is most likely case. It is cheaper to read a variable than do a function call.

Cheers,
Longman




Thank you for your patience, I just misunderstood.
I will do it in v2.

Best regards,
Ridong





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]     [Monitors]

  Powered by Linux