Messages in this thread | | | Date | Mon, 30 Aug 2021 11:31:03 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH] sched/core: Bypass arch_scale_freq_tick() on nohz_full CPUs |
| |
On Fri, Aug 27, 2021 at 08:49:10PM +0200, Nicolas Saenz Julienne wrote: > arch_scale_freq_tick() calculations generally track the increment of > frequency performance counters in between scheduler ticks to provide > estimations on how DVFS swayed frequency on that CPU. This information > eventually allows for more precise scheduling decisions. It's all fine > and good, but nohz_full CPUs are capable of disabling the tick > indefinitely and potentially trigger overflows in > arch_scale_freq_tick()'s calculations once it's eventually re-enabled. > > This will happen for both users of this interface: x86 and arm64. And > it's also relevant that the heuristic on what to do in case of > operations overflowing varies depending on the implementation. It goes > from fully disabling frequency invariance scaling on all CPUS, to > ignoring this is a possibility. > > It's arguable that nohz_full CPUs are unlikely to benefit much from this > feature, since their aim is to allow for uninterrupted execution of a > single task, effectively getting the scheduler out of the way. Also, > DVFS itself is also unlikely be used on most nohz_full systems, given > its effects on latency. > > So get around this by not calling arch_scale_freq_tick() on nohz_full > CPUs. > > Note that tick_nohz_full_cpu() relies on a static branch, which avoids > degrading performance on the rest of systems. > > Signed-off-by: Nicolas Saenz Julienne <nsaenzju@redhat.com> > --- > kernel/sched/core.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/kernel/sched/core.c b/kernel/sched/core.c > index 2fd623b2270d..8c04ec0e073a 100644 > --- a/kernel/sched/core.c > +++ b/kernel/sched/core.c > @@ -5016,7 +5016,13 @@ void scheduler_tick(void) > unsigned long thermal_pressure; > u64 resched_latency; > > - arch_scale_freq_tick(); > + /* > + * nohz_full CPUs are capable of disabling the scheduler tick > + * indefinitely, potentially overflowing arch_scale_freq_tick() > + * calculations once it's re-enabled. > + */ > + if (!tick_nohz_full_cpu(smp_processor_id())) > + arch_scale_freq_tick(); > sched_clock_tick(); > > rq_lock(rq, &rf);
Hurmph,.. I'm not too happy with this.. Fundamentally the whole NOHZ_FULL state should be dynamic, it currently isn't but that's arguably a bug of the current implementation.
As such the above doesn't really dtrt, since a CPU disabling NOHZ_FULL will then still suffer all them overflows you mentioned.
Frederic, how should this be done right? Is there a place where upon entering/exiting NOHZ_FULL we can do fixups?
| |