Re: [PATCH v7 2/3] x86/cacheinfo: Delete global num_cache_leaves

From: Borislav Petkov
Date: Tue Oct 22 2024 - 09:21:31 EST


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