From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756098AbdKCOyq (ORCPT ); Fri, 3 Nov 2017 10:54:46 -0400 Received: from mx1.redhat.com ([209.132.183.28]:43040 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755519AbdKCOxe (ORCPT ); Fri, 3 Nov 2017 10:53:34 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 815D933A18A Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=prarit@redhat.com Subject: Re: [PATCH 2/3 v4] x86/topology: Avoid wasting 128k for package id array To: Thomas Gleixner References: <20171025120940.15721-1-prarit@redhat.com> <20171025120940.15721-3-prarit@redhat.com> Cc: linux-kernel@vger.kernel.org, Andi Kleen , Ingo Molnar , "H. Peter Anvin" , x86@kernel.org, Peter Zijlstra , Dave Hansen , Piotr Luc , Kan Liang , Borislav Petkov , Stephane Eranian , Arvind Yadav , Andy Lutomirski , Christian Borntraeger , "Kirill A. Shutemov" , Tom Lendacky , Mathias Krause , Tim Chen , Vitaly Kuznetsov From: Prarit Bhargava Message-ID: Date: Fri, 3 Nov 2017 10:53:31 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Fri, 03 Nov 2017 14:53:33 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/01/2017 12:56 PM, Thomas Gleixner wrote: > On Wed, 25 Oct 2017, Prarit Bhargava wrote: >> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h >> index b390ff76e58f..f4ab1edf4e24 100644 >> --- a/arch/x86/include/asm/processor.h >> +++ b/arch/x86/include/asm/processor.h >> @@ -124,8 +124,10 @@ struct cpuinfo_x86 { >> u16 booted_cores; >> /* Physical processor id: */ >> u16 phys_proc_id; >> - /* Logical processor id: */ >> + /* Logical processor (package) id: */ >> u16 logical_proc_id; >> + /* Physical package ID */ >> + u16 phys_pkg_id; > > How is this new field used aside of being written to and how is it > different from phys_proc_id? AFAICT, it's the same as all callers to > topology_update_package_map() are handing in cpu_data->phys_proc_id. > I've removed this in v5. >> +/** >> + * topology_phys_to_logical_pkg - Map a physical package id to a logical >> + * >> + * Returns logical package id or -1 if not found >> + */ >> +int topology_phys_to_logical_pkg(unsigned int phys_pkg) >> +{ >> + int log_pkg; >> + >> + for (log_pkg = 0; log_pkg < logical_packages; log_pkg++) >> + if (logical_to_physical_pkg_map[log_pkg] == phys_pkg) >> + return log_pkg; >> + >> + return -1; >> +} >> +EXPORT_SYMBOL(topology_phys_to_logical_pkg); > > .... > >> + >> + /* Allocate and copy a new array */ >> + ltp_pkg_map_new = kmalloc(logical_packages * sizeof(u16), GFP_KERNEL); >> + BUG_ON(!ltp_pkg_map_new); >> + if (logical_to_physical_pkg_map) { >> + memcpy(ltp_pkg_map_new, logical_to_physical_pkg_map, >> + logical_packages * sizeof(u16)); >> + kfree(logical_to_physical_pkg_map); >> } >> - physical_to_logical_pkg[pkg] = new; >> + logical_to_physical_pkg_map = ltp_pkg_map_new; > > This lacks serialization and is therefore broken against a concurrent > topology_phys_to_logical_pkg() call for obvious reasons. The current user > is probably safe, but this really needs to be fixed now. I'm using a spin_lock_irq in v5. I am finishing testing on 4S & 8S systems now. P. > > Thanks, > > tglx >