linux-kernel - Re: [PATCH] x86/aperfmperf: Fix arch_scale_freq

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Wed, 02 Nov 2022 10:55:18 +0200
From:   ypodemsk@...hat.com
To:     Valentin Schneider <vschneid@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>
Cc:     x86@...nel.org, tglx@...utronix.de, mingo@...hat.com,
        rafael.j.wysocki@...el.com, pauld@...hat.com, frederic@...nel.org,
        ggherdovich@...e.cz, linux-kernel@...r.kernel.org, lenb@...nel.org,
        jlelli@...hat.com, mtosatti@...hat.com, ppandit@...hat.com,
        alougovs@...hat.com, lcapitul@...hat.com, nsaenz@...nel.org
Subject: Re: [PATCH] x86/aperfmperf: Fix arch_scale_freq_tick() on tickless
 systems

Friendly ping? 

On Wed, 2022-10-19 at 14:31 +0300, ypodemsk@...hat.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()
> >