From: "Ahmed S. Darwish" <darwi@linutronix.de>
To: K Prateek Nayak <kprateek.nayak@amd.com>
Cc: Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
Dave Hansen <dave.hansen@linux.intel.com>,
x86@kernel.org, linux-kernel@vger.kernel.org,
"H. Peter Anvin" <hpa@zytor.com>
Subject: Re: [PATCH 0/2] x86/cpu/cacheinfo: Simplify llc_id calculation on AMD platforms
Date: Thu, 21 Aug 2025 11:53:41 +0200 [thread overview]
Message-ID: <aKbspRcrEWBiox8c@lx-t490> (raw)
In-Reply-To: <20250821051910.7351-1-kprateek.nayak@amd.com>
Hi,
On Thu, 21 Aug 2025, K Prateek Nayak wrote:
>
> Reuse the available cacheinfo helpers instead of open-coding masks and
> shifts in cacheinfo_amd_init_llc_id().
>
> This series has been tested on top of tip:x86/cpu at commit 6a42c31ef324
> ("x86/cpu: Rename and move CPU model entry for Diamond Rapids") with no
> changes being observed in "/sys/kernel/debug/x86/topo/" on a 3rd
> Generation EPYC platform.
>
I had to merge the diff of the two patches to get what was going on.
My only comment would be:
> --- a/arch/x86/kernel/cpu/cacheinfo.c
> +++ b/arch/x86/kernel/cpu/cacheinfo.c
> ...
> +/*
> + * The max shared threads number comes from CPUID(0x4) EAX[25-14] with input
> + * ECX as cache index. Then right shift apicid by the number's order to get
> + * cache id for this cache node.
> + */
> +static unsigned int get_cache_id(u32 apicid, struct _cpuid4_info *id4)
> +{
> + unsigned long num_threads_sharing;
> + int index_msb;
> +
> + num_threads_sharing = 1 + id4->eax.split.num_threads_sharing;
> + index_msb = get_count_order(num_threads_sharing);
> + return apicid >> index_msb;
> +}
> +
...
> -/*
> - * The max shared threads number comes from CPUID(0x4) EAX[25-14] with input
> - * ECX as cache index. Then right shift apicid by the number's order to get
> - * cache id for this cache node.
> - */
> -static void get_cache_id(int cpu, struct _cpuid4_info *id4)
> -{
> - struct cpuinfo_x86 *c = &cpu_data(cpu);
> - unsigned long num_threads_sharing;
> - int index_msb;
> -
> - num_threads_sharing = 1 + id4->eax.split.num_threads_sharing;
> - index_msb = get_count_order(num_threads_sharing);
> - id4->id = c->topo.apicid >> index_msb;
> -}
> -
...
Since you don't write to 'id4' anymore, please make the pointer constant.
It helps with code comprehension: from a quick glance, one knows that the
function does not write to the passed structure.
Other than that, and possibly merging the two patches (if you want to):
Acked-by: Ahmed S. Darwish <darwi@linutronix.de>
Thanks!
next prev parent reply other threads:[~2025-08-21 9:53 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-08-21 5:19 [PATCH 0/2] x86/cpu/cacheinfo: Simplify llc_id calculation on AMD platforms K Prateek Nayak
2025-08-21 5:19 ` [PATCH 1/2] x86/cpu/cacheinfo: Convert get_cache_id() to use APIC ID and return Cache ID K Prateek Nayak
2025-08-22 14:48 ` [tip: x86/cpu] x86/cpu/cacheinfo: Simplify cacheinfo_amd_init_llc_id() using _cpuid4_info tip-bot2 for K Prateek Nayak
2025-08-21 5:19 ` [PATCH 2/2] " K Prateek Nayak
2025-08-21 9:29 ` [PATCH 0/2] x86/cpu/cacheinfo: Simplify llc_id calculation on AMD platforms Borislav Petkov
2025-08-21 9:53 ` Ahmed S. Darwish [this message]
2025-08-21 10:22 ` Borislav Petkov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aKbspRcrEWBiox8c@lx-t490 \
--to=darwi@linutronix.de \
--cc=bp@alien8.de \
--cc=dave.hansen@linux.intel.com \
--cc=hpa@zytor.com \
--cc=kprateek.nayak@amd.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=tglx@linutronix.de \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.