Messages in this thread Patch in this message | | | Date | Wed, 24 Nov 2021 15:42:14 +0800 | From | Aili Yao <> | Subject | Re: [PATCH] sched/isolation: delete redundant housekeeping_overridden check |
| |
> The static branch is a jump and a nop, and the two conditions are not > the same. So nothing above is redundant. > > The static_branch_unlikely(&housekeeping_overridden) is a switch that > is either a nop which being an unlikely, is the fast path, and the > content of the if block is the out-of-band condition. That is, the > static branch keeps the expensive if conditional from ever being tested > (because it is "overridden"). > > Now when it's not overridden, that static branch turns into a jump to > the flags test. Which then performs the expensive conditional compare > against flags to see if it should do the cpumask_test_cpu(). > > I state "expensive" because compared to a jmp or nop, any branch based > on a test causes cache speculation to be executed. Which means branch > prediction, etc. The jmp and nop are just like any other atomic > instruction that goes through the pipeline and is considered 100% > predictable, hence it doesn't need the extra logic in the CPU to figure > it out. > > The only thing your patch does is remove the optimization of the static > branch logic. > Great thanks for your detailed explanation, and in some way, I get your points and partially understand the static branch's necessary, Thanks!
I follow you explanations and dig more into the code, and i have another RFC option, And I want it shared and reviewed:
From 40c13fad29b22242df6ff5ac97bddafd59e04f3e Mon Sep 17 00:00:00 2001 From: Aili Yao <yaoaili@kingsoft.com> Date: Wed, 24 Nov 2021 02:15:28 -0500 Subject: [PATCH] sched/isolation: little optimization for housekeeping_cpu
the housekeeping_test_cpu resides in isolation.c and is not a inline function; For general housekeeping_cpu interface, we seems want to unify the inline definition, so housekeeping_cpu call housekeeping_test_cpu when CONFIG_CPU_ISOLATION, while this public inline function still will call non-static housekeeping_test_cpu; this seems not nessary;
For different CPU_ISOLATION configuration, we may use different inline property, this will gain a little optimization;
Signed-off-by: Aili Yao <yaoaili@kingsoft.com> --- include/linux/sched/isolation.h | 9 ++------- kernel/sched/isolation.c | 4 ++-- 2 files changed, 4 insertions(+), 9 deletions(-)
diff --git a/include/linux/sched/isolation.h b/include/linux/sched/isolation.h index cc9f393e2a70..3ccc19e52186 100644 --- a/include/linux/sched/isolation.h +++ b/include/linux/sched/isolation.h @@ -23,9 +23,8 @@ extern int housekeeping_any_cpu(enum hk_flags flags); extern const struct cpumask *housekeeping_cpumask(enum hk_flags flags); extern bool housekeeping_enabled(enum hk_flags flags); extern void housekeeping_affine(struct task_struct *t, enum hk_flags flags); -extern bool housekeeping_test_cpu(int cpu, enum hk_flags flags); +extern bool housekeeping_cpu(int cpu, enum hk_flags flags); extern void __init housekeeping_init(void); - #else static inline int housekeeping_any_cpu(enum hk_flags flags) @@ -46,15 +45,11 @@ static inline bool housekeeping_enabled(enum hk_flags flags) static inline void housekeeping_affine(struct task_struct *t, enum hk_flags flags) { } static inline void housekeeping_init(void) { } -#endif /* CONFIG_CPU_ISOLATION */ static inline bool housekeeping_cpu(int cpu, enum hk_flags flags) { -#ifdef CONFIG_CPU_ISOLATION - if (static_branch_unlikely(&housekeeping_overridden)) - return housekeeping_test_cpu(cpu, flags); -#endif return true; } +#endif /* CONFIG_CPU_ISOLATION */ #endif /* _LINUX_SCHED_ISOLATION_H */ diff --git a/kernel/sched/isolation.c b/kernel/sched/isolation.c index 7f06eaf12818..b5e81df4b04a 100644 --- a/kernel/sched/isolation.c +++ b/kernel/sched/isolation.c @@ -54,14 +54,14 @@ void housekeeping_affine(struct task_struct *t, enum hk_flags flags) } EXPORT_SYMBOL_GPL(housekeeping_affine); -bool housekeeping_test_cpu(int cpu, enum hk_flags flags) +bool housekeeping_cpu(int cpu, enum hk_flags flags) { if (static_branch_unlikely(&housekeeping_overridden)) if (housekeeping_flags & flags) return cpumask_test_cpu(cpu, housekeeping_mask); return true; } -EXPORT_SYMBOL_GPL(housekeeping_test_cpu); +EXPORT_SYMBOL_GPL(housekeeping_cpu); void __init housekeeping_init(void) { -- 2.27.0
| |