Messages in this thread Patch in this message | | | From | "Brown, Len" <> | Subject | RE: [PATCH 03/11] x86 topology: Add CPUID.1F multi-die/package support | Date | Mon, 25 Feb 2019 08:08:24 +0000 |
| |
Hi Brice,
Yes, you re-discovered the bug that Kan Liang pointed out last week.
I have updated this patch set, and the latest for testing is in my git tree on kernel.org or
https://github.com/lenb/linux.git x86
Note that I took your advice and left the core_siblings with its original definition, And created package_threads as a synonym. I will e-mail out the patch set again When I do 2 more things:
1. add core_threads map to sysfs 2. replace unique_die_id with logical_die_id -- turns out it is useful for same reason as logical_package_id.
Thanks, -Len
-----Original Message----- From: Brice Goglin [mailto:Brice.Goglin@inria.fr] Sent: Sunday, February 24, 2019 5:04 AM To: Len Brown <lenb@kernel.org>; x86@kernel.org Cc: linux-kernel@vger.kernel.org; Brown, Len <len.brown@intel.com>; linux-doc@vger.kernel.org; Like Xu <like.xu@linux.intel.com> Subject: Re: [PATCH 03/11] x86 topology: Add CPUID.1F multi-die/package support
Le 19/02/2019 à 04:40, Len Brown a écrit : > diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c > index ccd1f2a8e557..4250a87f57db 100644 > --- a/arch/x86/kernel/smpboot.c > +++ b/arch/x86/kernel/smpboot.c > @@ -393,6 +393,7 @@ static bool match_smt(struct cpuinfo_x86 *c, struct cpuinfo_x86 *o) > int cpu1 = c->cpu_index, cpu2 = o->cpu_index; > > if (c->phys_proc_id == o->phys_proc_id && > + c->cpu_die_id == o->cpu_die_id && > per_cpu(cpu_llc_id, cpu1) == per_cpu(cpu_llc_id, cpu2)) { > if (c->cpu_core_id == o->cpu_core_id) > return topology_sane(c, o, "smt"); @@ -404,6 +405,7 @@ static > bool match_smt(struct cpuinfo_x86 *c, struct cpuinfo_x86 *o) > } > > } else if (c->phys_proc_id == o->phys_proc_id && > + c->cpu_die_id == o->cpu_die_id && > c->cpu_core_id == o->cpu_core_id) { > return topology_sane(c, o, "smt"); > } > @@ -461,7 +463,7 @@ static bool match_llc(struct cpuinfo_x86 *c, struct cpuinfo_x86 *o) > */ > static bool match_die(struct cpuinfo_x86 *c, struct cpuinfo_x86 *o) > { > - if (c->phys_proc_id == o->phys_proc_id) > + if (c->cpu_die_id == o->cpu_die_id) > return true; > return false; > }
Hello Len,
I am testing your patches in a VM with CPUID.1f QEMU patches (author Like Xu is CC'ed), booted to get 2 packages with 1 NUMA node each and 2 dies each:
-smp cpus=16,threads=2,cores=2,dies=2,sockets=2 -numa node,cpus=0-7,nodeid=0 -numa node,cpus=8-15,nodeid=1
sysfs files expose wrong information: core_siblings contains threads of the local die AND of die with same number in other processors, eg cpu 0-3 and 8-11 instead of 0-3 only.
The issue is that you seem to assume that die ids will always be unique across multiple packages. Is this a valid assumption? If so, QEMU patches should be fixed. If not, I fixed the issue by changing match_die() to check both phys_proc_id and cpu_die_id:
static bool match_die(struct cpuinfo_x86 *c, struct cpuinfo_x86 *o) { if (c->phys_proc_id == o->phys_proc_id && c->cpu_die_id == o->cpu_die_id) return true; return false; }
Thanks Brice
| |