On Mon, Feb 14, 2022 at 07:17:24AM -0800, srinivas pandruvada wrote:
Hi Doug,
I think you use CONFIG_NO_HZ_FULL. Here we are getting callback from scheduler. Can we check that if scheduler woke up on those CPUs? We can run "trace-cmd -e sched" and check in kernel shark if there is similar gaps in activity.
Srinivas analyzed the scheduler trace data from trace-cmd, and thought is related with the cpufreq callback is not called timeley from scheduling events:
" I mean we ignore the callback when the target CPU is not a local CPU as we have to do IPI to adjust MSRs. This will happen many times when sched_wake will wake up a new CPU for the thread (we will get a callack for the target) but once the remote thread start executing "sched_switch", we will get a callback on local CPU, so we will adjust frequencies (provided 10ms interval from the last call).
From the trace file I see the scenario where it took 72sec between two
updates: CPU 2 34412.597161 busy=78 freq=3232653 34484.450725 busy=63 freq=2606793
There is periodic activity in between, related to active load balancing in scheduler (since last frequency was higher these small work will also run at higher frequency). But those threads are not CFS class, so scheduler callback will not be called for them.
So removing the patch removed a trigger which would have caused a sched_switch to a CFS task and call a cpufreq/intel_pstate callback. But calling for every class, will be too many callbacks and not sure we can even call for "stop" class, which these migration threads are using. "
Following this direction, I made a hacky debug patch which should help to restore the previous behavior.
Doug, could you help to try it? thanks
It basically tries to make sure the cpufreq-update-util be called timely even for a silent system with very few interrupts (even from tick).
Thanks, Feng
From 6be5f5da66a847860b0b9924fbb09f93b2e2d6e6 Mon Sep 17 00:00:00 2001 From: Feng Tang feng.tang@intel.com Date: Tue, 22 Feb 2022 22:59:00 +0800 Subject: [PATCH] idle/intel-pstate: hacky debug patch to make sure the cpufreq_update_util callback being called timely in silent system
--- kernel/sched/idle.c | 10 ++++++++++ kernel/sched/sched.h | 13 +++++++++++++ 2 files changed, 23 insertions(+)
diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c index d17b0a5ce6ac..cc538acb3f1a 100644 --- a/kernel/sched/idle.c +++ b/kernel/sched/idle.c @@ -258,15 +258,25 @@ static void cpuidle_idle_call(void) * * Called with polling cleared. */ +DEFINE_PER_CPU(u64, last_util_update_time); /* in jiffies */ static void do_idle(void) { int cpu = smp_processor_id(); + u64 expire;
/* * Check if we need to update blocked load */ nohz_run_idle_balance(cpu);
+#ifdef CONFIG_X86_INTEL_PSTATE + expire = __this_cpu_read(last_util_update_time) + HZ * 3; + if (unlikely(time_is_before_jiffies(expire))) { + idle_update_util(); + __this_cpu_write(last_util_update_time, get_jiffies_64()); + } +#endif + /* * If the arch has a polling bit, we maintain an invariant: * diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h index 0e66749486e7..2a8d87988d1f 100644 --- a/kernel/sched/sched.h +++ b/kernel/sched/sched.h @@ -2809,6 +2809,19 @@ static inline void cpufreq_update_util(struct rq *rq, unsigned int flags) if (data) data->func(data, rq_clock(rq), flags); } + +static inline void idle_update_util(void) +{ + struct update_util_data *data; + struct rq *rq = cpu_rq(raw_smp_processor_id()); + + data = rcu_dereference_sched(*per_cpu_ptr(&cpufreq_update_util_data, + cpu_of(rq))); + if (data) + data->func(data, rq_clock(rq), 0); +} + + #else static inline void cpufreq_update_util(struct rq *rq, unsigned int flags) {} #endif /* CONFIG_CPU_FREQ */