linux-kernel - Re: [PATCH 1/2] x86/cpu: introduce x86_get

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]

Message-ID: <87lf15ba1g.ffs@tglx>
Date:   Tue, 30 Nov 2021 22:36:43 +0100
From:   Thomas Gleixner <tglx@...utronix.de>
To:     zhenwei pi <pizhenwei@...edance.com>, pbonzini@...hat.com
Cc:     kvm@...r.kernel.org, linux-kernel@...r.kernel.org, x86@...nel.org,
        zhenwei pi <pizhenwei@...edance.com>
Subject: Re: [PATCH 1/2] x86/cpu: introduce x86_get_freq

On Mon, Nov 29 2021 at 13:20, zhenwei pi wrote:

> Wrapper function x86_get_freq to get freq on a x86 platform, hide
> detailed implementation for proc routine.

Please rename this to x86_get_cpufreq_khz() simply because
x86_get_freq() is way too unspecific.

Please mention function names with 'function()' which makes it obvious
what it is. Here and in the Subject.

Also spell out frequency all over the place in the change log. There is
no reason for using abbreviations in text.

> +
> +unsigned int x86_get_freq(unsigned int cpu)
> +{
> +	unsigned int freq = 0;
> +
> +	if (cpu_has(&cpu_data(cpu), X86_FEATURE_TSC)) {

Please use cpu_feature_enabled(X86....) and make this condition
negated:

     if (!cpu_feature_enabled(X86_FEATURE_TSC))
            return 0;

which spares an indentation level and the 0 initialization of the variable.

Thanks,

        tglx