linux-kernel - Re: [PATCH V3 3/3] arm64: topology: Make AMUs work with modular cpufreq drivers

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20201217105049.z3aqhl4mo56hhqvk@vireshk-i7>
Date:   Thu, 17 Dec 2020 16:20:49 +0530
From:   Viresh Kumar <viresh.kumar@...aro.org>
To:     Ionela Voinescu <ionela.voinescu@....com>
Cc:     Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will@...nel.org>,
        Vincent Guittot <vincent.guittot@...aro.org>,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH V3 3/3] arm64: topology: Make AMUs work with modular
 cpufreq drivers

On 16-12-20, 19:37, Ionela Voinescu wrote:
> I did not yet test this, but reading this comment made me wonder..
> 
> arch_scale_freq_invariant() (or topology_scale_freq_invariant()) is also
> called from schedutil when obtaining the next frequency.
> 
> So if we had a system that only partly supports AMUs but had at some
> point a cpufreq driver that provided FIE for the other CPUs, when we
> unregister the driver, the cpufreq_freq_invariance static key is
> disabled. Therefore, none of the conditions for system invariance is
> now accomplished and arch_scale_freq_invariant() will return false.
> This will be broken as utilization is still scaled, but the algorithm
> for computing the next frequency in schedutil will not take this into
> account.

I think the best and the easiest solution for this is:

bool arch_freq_counters_available(const struct cpumask *cpus)
{
        return amu_freq_invariant();
}

But we probably need to rename it to something like arch_is_fie().

> 
> [..]
> > > > +	ret = cpufreq_register_notifier(&init_amu_fie_notifier,
> > > > +					CPUFREQ_POLICY_NOTIFIER);
> 
> The above makes the use of AMUs for FIE tightly coupled with cpufreq.
> 
> Initially I made cpufreq_get_hw_max_freq(cpu) a weak function for the
> possible platforms that might not use a cpufreq driver and might want to
> provide this function to still benefit from the use of counters for
> frequency invariance.
> 
> But I'm starting to believe that supporting all these corner-cases in
> advance just introduces messiness.
> 
> So feel free to remove the 'weak' state of cpufreq_get_hw_max_freq() as
> well, so we don't keep wondering why we had that in the first place.
> It would not make any sense keeping that in light of these changes.

Will do it in a separate patch then.

> P.S. I will be on holiday starting tomorrow until beginning of January.
> Were you intending this for 5.11, or can I take more time to review
> future versions and continue testing?

I wanted to  :)

-- 
viresh