Hi,
The below replaces cpufreq_update_util()'s indirect call with a static_call(). The patch is quite gross, and we definitely need static_call_update_cpuslocked(). cpufreq folks, is there a better way to do that optimize pass? That is, we need to know when all CPUs have the *same* function set. Otherwise we obviously cannot rewrite the text, which is shared by all CPUs. Also, is there a lock order comment in cpufreq somewhere? I tried following it, but eventually gave up and figured 'asking' lockdep was far simpler. --- include/linux/sched/cpufreq.h | 9 ++++--- kernel/sched/cpufreq.c | 55 +++++++++++++++++++++++++++++++++++++++++++ kernel/sched/sched.h | 28 ++++++++++++++++++++-- kernel/static_call.c | 4 ++-- 4 files changed, 89 insertions(+), 7 deletions(-) diff --git a/include/linux/sched/cpufreq.h b/include/linux/sched/cpufreq.h index 6205578ab6ee..6d2972b67fa0 100644 --- a/include/linux/sched/cpufreq.h +++ b/include/linux/sched/cpufreq.h @@ -12,14 +12,17 @@ #ifdef CONFIG_CPU_FREQ struct cpufreq_policy; +struct update_util_data; + +typedef void (*cpu_util_update_f)(struct update_util_data *data, + u64 time, unsigned int flags); struct update_util_data { - void (*func)(struct update_util_data *data, u64 time, unsigned int flags); + cpu_util_update_f func; }; void cpufreq_add_update_util_hook(int cpu, struct update_util_data *data, - void (*func)(struct update_util_data *data, u64 time, - unsigned int flags)); + cpu_util_update_f func); void cpufreq_remove_update_util_hook(int cpu); bool cpufreq_this_cpu_can_update(struct cpufreq_policy *policy); diff --git a/kernel/sched/cpufreq.c b/kernel/sched/cpufreq.c index 7c2fe50fd76d..b362a04e04d1 100644 --- a/kernel/sched/cpufreq.c +++ b/kernel/sched/cpufreq.c @@ -6,11 +6,60 @@ * Author: Rafael J. Wysocki <rafael.j.wyso...@intel.com> */ #include <linux/cpufreq.h> +#include <linux/static_call.h> #include "sched.h" DEFINE_PER_CPU(struct update_util_data __rcu *, cpufreq_update_util_data); +#ifdef CONFIG_HAVE_STATIC_CALL + +void cpufreq_update_indirect(struct update_util_data *data, + u64 time, unsigned int flags) +{ + if (data) + data->func(data, time, flags); +} + +DEFINE_STATIC_CALL(cpufreq_update_util, cpufreq_update_indirect); + +static void cpufreq_update_safe(void) +{ + static_call_update(cpufreq_update_util, cpufreq_update_indirect); +} + +static void cpufreq_update_optimize(void) +{ + struct update_util_data *data; + cpu_util_update_f func = NULL, dfunc; + int cpu; + + for_each_online_cpu(cpu) { + data = per_cpu(cpufreq_update_util_data, cpu); + dfunc = data ? READ_ONCE(data->func) : NULL; + + if (dfunc) { + if (!func) + func = dfunc; + else if (func != dfunc) + return; + } else if (func) + return; + } + + pr_info("sched: optimized cpufreq_update_util\n"); + + /* all CPUs have the same @func */ + static_call_update(cpufreq_update_util, func); +} + +#else + +static inline void cpufreq_update_safe(void) { } +static inline void cpufreq_update_optimize(void) { } + +#endif + /** * cpufreq_add_update_util_hook - Populate the CPU's update_util_data pointer. * @cpu: The CPU to set the pointer for. @@ -39,8 +88,12 @@ void cpufreq_add_update_util_hook(int cpu, struct update_util_data *data, if (WARN_ON(per_cpu(cpufreq_update_util_data, cpu))) return; + cpufreq_update_safe(); + data->func = func; rcu_assign_pointer(per_cpu(cpufreq_update_util_data, cpu), data); + + cpufreq_update_optimize(); } EXPORT_SYMBOL_GPL(cpufreq_add_update_util_hook); @@ -56,7 +109,9 @@ EXPORT_SYMBOL_GPL(cpufreq_add_update_util_hook); */ void cpufreq_remove_update_util_hook(int cpu) { + cpufreq_update_safe(); rcu_assign_pointer(per_cpu(cpufreq_update_util_data, cpu), NULL); + cpufreq_update_optimize(); } EXPORT_SYMBOL_GPL(cpufreq_remove_update_util_hook); diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h index 24ac31b40b55..333e33c3d496 100644 --- a/kernel/sched/sched.h +++ b/kernel/sched/sched.h @@ -2473,6 +2473,30 @@ static inline u64 irq_time_read(int cpu) #ifdef CONFIG_CPU_FREQ DECLARE_PER_CPU(struct update_util_data __rcu *, cpufreq_update_util_data); +#ifdef CONFIG_HAVE_STATIC_CALL + +extern void cpufreq_update_indirect(struct update_util_data *data, + u64 time, unsigned int flags); + +DECLARE_STATIC_CALL(cpufreq_update_util, cpufreq_update_indirect); + +static inline void cpufreq_update_util_call(struct update_util_data *data, + u64 time, unsigned int flags) +{ + static_call_cond(cpufreq_update_util)(data, time, flags); +} + +#else + +static inline void cpufreq_update_util_call(struct update_util_data *data, + u64 time, unsigned int flags) +{ + if (data) + data->func(data, time, flags); +} + +#endif + /** * cpufreq_update_util - Take a note about CPU utilization changes. * @rq: Runqueue to carry out the update for. @@ -2501,9 +2525,9 @@ static inline void cpufreq_update_util(struct rq *rq, unsigned int flags) data = rcu_dereference_sched(*per_cpu_ptr(&cpufreq_update_util_data, cpu_of(rq))); - if (data) - data->func(data, rq_clock(rq), flags); + cpufreq_update_util_call(data, rq_clock(rq), flags); } + #else static inline void cpufreq_update_util(struct rq *rq, unsigned int flags) {} #endif /* CONFIG_CPU_FREQ */ diff --git a/kernel/static_call.c b/kernel/static_call.c index ae825295cf68..fd73dfce3e50 100644 --- a/kernel/static_call.c +++ b/kernel/static_call.c @@ -122,7 +122,7 @@ void __static_call_update(struct static_call_key *key, void *tramp, void *func) struct static_call_site *site, *stop; struct static_call_mod *site_mod, first; - cpus_read_lock(); +// cpus_read_lock(); static_call_lock(); if (key->func == func) @@ -196,7 +196,7 @@ void __static_call_update(struct static_call_key *key, void *tramp, void *func) done: static_call_unlock(); - cpus_read_unlock(); +// cpus_read_unlock(); } EXPORT_SYMBOL_GPL(__static_call_update);