Messages in this thread | | | Subject | Re: [PATCH] x86/aperfmperf: Fix arch_scale_freq_tick() on tickless systems | From | ypodemsk@redhat ... | Date | Wed, 02 Nov 2022 10:55:18 +0200 |
| |
Friendly ping?
On Wed, 2022-10-19 at 14:31 +0300, ypodemsk@redhat.com wrote: > On Tue, 2022-09-06 at 17:17 +0100, Valentin Schneider wrote: > > On 06/09/22 16:54, Peter Zijlstra wrote: > > > On Thu, Aug 04, 2022 at 04:17:28PM +0300, Yair Podemsky wrote: > > > > @@ -392,7 +400,12 @@ void arch_scale_freq_tick(void) > > > > s->mcnt = mcnt; > > > > raw_write_seqcount_end(&s->seq); > > > > > > > > - scale_freq_tick(acnt, mcnt); > > > > + /* > > > > + * Avoid calling scale_freq_tick() when the last update > > > > was too > > > > long ago, > > > > + * as it might overflow during calulation. > > > > + */ > > > > + if ((jiffies - last) <= MAX_SAMPLE_AGE_NOHZ) > > > > + scale_freq_tick(acnt, mcnt); > > > > } > > > > > > All this patch does is avoid the warning; but afaict it doesn't > > > make it > > > behave in a sane way. > > It also avoids the disabling of the frequency invariance accounting > for > all cpus, that occurs immediately after the warning. > That is the bug that is being solved, Since it affects also non- > tickless cpus. > > > > I'm thinking that on nohz_full cpus you don't have load > > > balancing, > > > I'm > > > also thinking that on nohz_full cpus you don't have DVFS. > > > > > > So *why* the heck are we setting this stuff to random values ? > > > Should > > > you not simply kill th entire thing for nohz_full cpus? > > > > IIRC this stems from systems where nohz_full CPUs are not running > > tickless > > at all times (you get transitions to/from latency-sensitive work). > > Also > > from what I've seen isolation is (intentionally) done with just > > isolcpus=managed_irq,<nohz_cpus>; there isn't the 'domain' flag so > > load > > balancing isn't permanently disabled. > > > > DVFS is another point, I don't remember seeing cpufreq governor > > changes in > > the transitions, but I wouldn't be suprised if there were - so we'd > > move > > from tickless, no-DVFS to ticking with DVFS (and would like that to > > behave > > "sanely"). > > > > FWIW arm64 does something similar in that it just saves the > > counters > > but > > doesn't update the scale when the delta overflows/wrapsaround, so > > that the > > next tick can work with a sane delta, cf > > > > arch/arm64/kernel/topology.c::amu_scale_freq_tick() > >
| |