[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20210318212826.GW4746@worktop.programming.kicks-ass.net>
Date: Thu, 18 Mar 2021 22:28:26 +0100
From: Peter Zijlstra <peterz@...radead.org>
To: "Rafael J. Wysocki" <rjw@...ysocki.net>, viresh.kumar@...aro.org
Cc: mingo@...hat.com, vincent.guittot@...aro.org,
dietmar.eggemann@....com, rostedt@...dmis.org,
linux-kernel@...r.kernel.org, Josh Poimboeuf <jpoimboe@...hat.com>
Subject: [RFC][PATCH] sched: Optimize cpufreq_update_util
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.wysocki@...el.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);
Powered by blists - more mailing lists