On 2019/2/24 18:04, Brice Goglin wrote:
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.

I don't think the intel MCP topolohgy has this global uniqueness assumption for die_id according to CPUID.1F SDM.

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




Reply via email to