[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20241022132050.GHZxemsmJSLg8Q_7U7@fat_crate.local>
Date: Tue, 22 Oct 2024 15:20:50 +0200
From: Borislav Petkov <bp@...en8.de>
To: Ricardo Neri <ricardo.neri-calderon@...ux.intel.com>
Cc: x86@...nel.org, Andreas Herrmann <aherrmann@...e.com>,
Catalin Marinas <catalin.marinas@....com>,
Chen Yu <yu.c.chen@...el.com>, Len Brown <len.brown@...el.com>,
Radu Rendec <rrendec@...hat.com>,
Pierre Gondois <Pierre.Gondois@....com>, Pu Wen <puwen@...on.cn>,
"Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
Sudeep Holla <sudeep.holla@....com>,
Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
Will Deacon <will@...nel.org>, Zhang Rui <rui.zhang@...el.com>,
Nikolay Borisov <nik.borisov@...e.com>,
Huang Ying <ying.huang@...el.com>,
Ricardo Neri <ricardo.neri@...el.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v7 2/3] x86/cacheinfo: Delete global num_cache_leaves
On Fri, Sep 13, 2024 at 01:31:54AM -0700, Ricardo Neri wrote:
> diff --git a/arch/x86/kernel/cpu/cacheinfo.c b/arch/x86/kernel/cpu/cacheinfo.c
> index 392d09c936d6..182cacd772b8 100644
> --- a/arch/x86/kernel/cpu/cacheinfo.c
> +++ b/arch/x86/kernel/cpu/cacheinfo.c
> @@ -178,7 +178,16 @@ struct _cpuid4_info_regs {
> struct amd_northbridge *nb;
> };
>
> -static unsigned short num_cache_leaves;
> +static inline unsigned int get_num_cache_leaves(unsigned int cpu)
> +{
> + return get_cpu_cacheinfo(cpu)->num_leaves;
> +}
There already is
#define cache_leaves(cpu) (ci_cacheinfo(cpu)->num_leaves)
And there's also get_cpu_cacheinfo().
And now you're adding more silly wrappers. Yuck.
Can we pls use *one* of those things and work with it everywhere?
> @@ -742,19 +753,19 @@ void init_intel_cacheinfo(struct cpuinfo_x86 *c)
> unsigned int l2_id = 0, l3_id = 0, num_threads_sharing, index_msb;
>
> if (c->cpuid_level > 3) {
> - static int is_initialized;
> -
> - if (is_initialized == 0) {
> - /* Init num_cache_leaves from boot CPU */
> - num_cache_leaves = find_num_cache_leaves(c);
> - is_initialized++;
> - }
> + /*
> + * There should be at least one leaf. A non-zero value means
> + * that the number of leaves has been initialized.
> + */
> + if (!get_num_cache_leaves(c->cpu_index))
> + set_num_cache_leaves(c->cpu_index,
> + find_num_cache_leaves(c));
Ugly linebreak.
>
> /*
> * Whenever possible use cpuid(4), deterministic cache
> * parameters cpuid leaf to find the cache details
> */
> - for (i = 0; i < num_cache_leaves; i++) {
> + for (i = 0; i < get_num_cache_leaves(c->cpu_index); i++) {
> struct _cpuid4_info_regs this_leaf = {};
> int retval;
>
> @@ -790,14 +801,14 @@ void init_intel_cacheinfo(struct cpuinfo_x86 *c)
> * Don't use cpuid2 if cpuid4 is supported. For P4, we use cpuid2 for
> * trace cache
> */
> - if ((num_cache_leaves == 0 || c->x86 == 15) && c->cpuid_level > 1) {
> + if ((!get_num_cache_leaves(c->cpu_index) || c->x86 == 15) && c->cpuid_level > 1) {
> /* supports eax=2 call */
> int j, n;
> unsigned int regs[4];
> unsigned char *dp = (unsigned char *)regs;
> int only_trace = 0;
>
> - if (num_cache_leaves != 0 && c->x86 == 15)
> + if (get_num_cache_leaves(c->cpu_index) && c->x86 == 15)
> only_trace = 1;
>
> /* Number of times to iterate */
> @@ -993,12 +1004,9 @@ int init_cache_level(unsigned int cpu)
> {
> struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu);
>
> - if (!num_cache_leaves)
> - return -ENOENT;
Why not
if (!cache_leaves(cpu))
return -ENOENT;
?
> if (!this_cpu_ci)
> return -EINVAL;
> this_cpu_ci->num_levels = 3;
> - this_cpu_ci->num_leaves = num_cache_leaves;
> return 0;
> }
>
> --
> 2.34.1
>
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Powered by blists - more mailing lists