linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Brice Goglin <Brice.Goglin@inria.fr>
To: Len Brown <lenb@kernel.org>, x86@kernel.org
Cc: linux-kernel@vger.kernel.org, Len Brown <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
Date: Sun, 24 Feb 2019 11:04:26 +0100	[thread overview]
Message-ID: <3ac80787-f282-ce94-77dc-f0e6ea9d4431@inria.fr> (raw)
In-Reply-To: <07d2908dc72bf964b27380999e1c826587d69136.1550545163.git.len.brown@intel.com>

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



  parent reply	other threads:[~2019-02-24 10:04 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-19  3:40 [PATCH 0/11] multi-die/package support Len Brown
2019-02-19  3:40 ` [PATCH 01/11] x86 topology: fix doc typo Len Brown
2019-02-19  3:40   ` [PATCH 02/11] topolgy: simplify cputopology.txt formatting and wording Len Brown
     [not found]     ` <9108bd98-e9f4-fee3-80c7-72d540c48291@infradead.org>
2019-02-19 20:33       ` [linux-drivers-review] " Brown, Len
2019-02-19  3:40   ` [PATCH 03/11] x86 topology: Add CPUID.1F multi-die/package support Len Brown
2019-02-19 16:49     ` Liang, Kan
2019-02-19 19:27       ` Brown, Len
2019-02-20  2:59     ` Like Xu
2019-02-20  6:10       ` Len Brown
2019-02-20 10:55     ` Peter Zijlstra
2019-02-20 15:08       ` Len Brown
2019-02-26 13:54         ` Peter Zijlstra
2019-02-28 15:59           ` Len Brown
2019-02-28 17:56             ` Peter Zijlstra
2019-02-24 10:04     ` Brice Goglin [this message]
2019-02-25  5:31       ` Like Xu
2019-02-25  8:08       ` Brown, Len
2019-02-19  3:40   ` [PATCH 04/11] cpu topology: export die_id Len Brown
2019-02-19  3:40   ` [PATCH 05/11] x86 topology: export die_siblings Len Brown
2019-02-19 16:56     ` Liang, Kan
2019-02-19 18:43       ` Brown, Len
2019-02-19 19:33         ` Liang, Kan
2019-02-20 10:58           ` Peter Zijlstra
2019-02-20 21:52     ` Brice Goglin
2019-02-21  7:41       ` Len Brown
2019-02-21  8:38         ` Brice Goglin
2019-02-19  3:40   ` [PATCH 06/11] x86 topology: define topology_unique_die_id() Len Brown
2019-02-19  3:40   ` [PATCH 07/11] powercap/intel_rapl: simplify rapl_find_package() Len Brown
2019-02-19  9:11     ` Rafael J. Wysocki
2019-02-19  3:40   ` [PATCH 08/11] powercap/intel_rapl: Support multi-die/package Len Brown
2019-02-19  9:10     ` Rafael J. Wysocki
2019-02-20 11:02     ` Peter Zijlstra
2019-02-21  5:44       ` Len Brown
2019-02-26  4:41         ` Len Brown
2019-02-26  6:55           ` Zhang Rui
2019-02-19  3:40   ` [PATCH 09/11] powercap/intel_rapl: update rapl domain name and debug messages Len Brown
2019-02-19  9:10     ` Rafael J. Wysocki
2019-02-19  3:40   ` [PATCH 10/11] thermal/x86_pkg_temp_thermal: Support multi-die/package Len Brown
2019-02-19  3:40   ` [PATCH 11/11] hwmon/coretemp: " Len Brown
2019-02-19 16:46     ` Guenter Roeck

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=3ac80787-f282-ce94-77dc-f0e6ea9d4431@inria.fr \
    --to=brice.goglin@inria.fr \
    --cc=len.brown@intel.com \
    --cc=lenb@kernel.org \
    --cc=like.xu@linux.intel.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).