From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754833AbcCSP5L (ORCPT ); Sat, 19 Mar 2016 11:57:11 -0400 Received: from mail.skyhub.de ([78.46.96.112]:59098 "EHLO mail.skyhub.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754626AbcCSP5K (ORCPT ); Sat, 19 Mar 2016 11:57:10 -0400 Date: Sat, 19 Mar 2016 16:56:51 +0100 From: Borislav Petkov To: Thomas Gleixner Cc: Peter Zijlstra , LKML , Ingo Molnar , aherrmann@suse.com, jencce.kernel@gmail.com, Rui Huang Subject: Re: [PATCH 2/3] x86/topology: Fix AMD core count Message-ID: <20160319155651.GA6570@nazgul.tnic> References: <20160318150345.146716865@infradead.org> <20160318150538.551407299@infradead.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, Mar 19, 2016 at 10:24:59AM +0100, Thomas Gleixner wrote: > On Fri, 18 Mar 2016, Peter Zijlstra wrote: > > It turns out AMD gets x86_max_cores wrong when there are compute > > units. > > > > The issue is that Linux assumes: > > > > nr_logical_cpus = nr_cores * nr_siblings > > > > But AMD reports its CU unit as 2 cores, but then sets num_smp_siblings > > to 2 as well. > > > > Cc: Ingo Molnar > > Cc: Borislav Petkov > > Cc: Thomas Gleixner > > Cc: Andreas Herrmann > > Reported-by: Xiong Zhou > > Fixes: 1f12e32f4cd5 ("x86/topology: Create logical package id") > > Signed-off-by: Peter Zijlstra (Intel) > > Link: http://lkml.kernel.org/r/20160317095220.GO6344@twins.programming.kicks-ass.net > > --- > > arch/x86/kernel/cpu/amd.c | 8 ++++---- > > arch/x86/kernel/smpboot.c | 11 ++++++----- > > 2 files changed, 10 insertions(+), 9 deletions(-) > > > > --- a/arch/x86/kernel/cpu/amd.c > > +++ b/arch/x86/kernel/cpu/amd.c > > @@ -313,9 +313,9 @@ static void amd_get_topology(struct cpui > > node_id = ecx & 7; > > > > /* get compute unit information */ > > - smp_num_siblings = ((ebx >> 8) & 3) + 1; > > + cores_per_cu = smp_num_siblings = ((ebx >> 8) & 3) + 1; > > + c->x86_max_cores /= smp_num_siblings; > > Unfortunately that will break stuff in event/amd/core.c, ras/mce_amd_inj.c > which rely on the AMD interpretation of c->x86_max_cores. What is the events/amd/core.c check even mean? We alloc an amd_nb per core but not per thread? The mce_amd_inj.c thing we could fix probably like this: --- diff --git a/arch/x86/ras/mce_amd_inj.c b/arch/x86/ras/mce_amd_inj.c index 55d38cfa46c2..b5a917d943c8 100644 --- a/arch/x86/ras/mce_amd_inj.c +++ b/arch/x86/ras/mce_amd_inj.c @@ -203,12 +203,7 @@ static void trigger_thr_int(void *info) static u32 get_nbc_for_node(int node_id) { - struct cpuinfo_x86 *c = &boot_cpu_data; - u32 cores_per_node; - - cores_per_node = c->x86_max_cores / amd_get_nodes_per_socket(); - - return cores_per_node * node_id; + return boot_cpu_data.x86_max_cores * node_id; } static void toggle_nb_mca_mst_cpu(u16 nid) --- -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. --