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

[Borislav Petkov]

On Tue, Oct 22, 2024 at 08:50:22PM -0700, Ricardo Neri wrote:
> I agree. Another wrapper is not needed. I did not use cache_leaves() because
> it was internal to drivers/base/cacheinfo.c I can convert it to a function
> and expose it in include/linux/cacheinfo.h. I can rename it as
> get_cacheinfo_leaves(unsigned int cpu).
> 
> Would that make sense?

I think you should use get_cpu_cacheinfo() everywhere and simply access the
struct members like ->num_leaves where you need it. No need for a bunch of
other silly one-liners.

> The only caller of init_cache_level() also checks for !cache_leaves(cpu). I
> saw no need to repeat the check here.
> 
> Also, I understand that the purpose of the function is to initialize
> cpu_cacheinfo::num_levels, which is not used on x86. Moreover,
> cpu_cacheinfo::num_levels do not depend on num_leaves.
> 
> Having said that, I see other architectures initializing both num_levels
> and num_leaves in this function.
> 
> Adding this check probably makes the x86 implementation more future-proof
> in case callers change their behavior.

But you're practically zapping its body in the next patch. So why does patch
3 even exist as a separate patch instead of being part of patch 2?

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette