From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753651AbdKJSfp (ORCPT ); Fri, 10 Nov 2017 13:35:45 -0500 Received: from mx1.redhat.com ([209.132.183.28]:44486 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753537AbdKJSfo (ORCPT ); Fri, 10 Nov 2017 13:35:44 -0500 Subject: Re: [PATCH v5 2/3] x86/topology: Avoid wasting 128k for package id array To: Thomas Gleixner References: <20171105165428.32108-1-prarit@redhat.com> <20171105165428.32108-3-prarit@redhat.com> <372083ad-2a77-4646-bc67-dd4348a521d9@redhat.com> <20171110150112.GI8522@tassilo.jf.intel.com> Cc: Andi Kleen , linux-kernel@vger.kernel.org, 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 , He Chen , Mathias Krause , Tim Chen , Vitaly Kuznetsov From: Prarit Bhargava Message-ID: <6775a1f9-d4a1-c01e-ea48-596fd7932d61@redhat.com> Date: Fri, 10 Nov 2017 13:35:41 -0500 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: <20171110150112.GI8522@tassilo.jf.intel.com> 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, 10 Nov 2017 18:35:44 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/10/2017 10:01 AM, Andi Kleen wrote: >>> All of that works. There is no way to make sure that a lookup is fully >>> serialized against a concurrent update. Even if the lookup holds >>> cpu_read_lock() the new package might arrive right after the unlock. >>> >> >> Thanks Thomas. >> >> Andi, do you want to take a look at this? > > I was originally worried about races, that is why i tried to put > everything into cpu_data. But that didn't work out because something > clears it. Perhaps the right solution would be some extra per_cpu > data variables, and search for the first match. I suspect that would > be simpler. But if that doesn't work I guess something like Thomas' > example will work. > > I assume you will handle it, Prarit? I can, but let's discuss your paragraph above. I'd like to get Thomas' thoughts on the problem and see if he thinks your original idea is valid, or if he has a better suggestion. Thomas, what Andi was originally attempting to do (way back in v1) is store the logical_proc_id in cpu_data and drop the array completely. The problem is the procedure on a cpu up, smp_store_cpu_info() overwrites the cpu's cpu_data with boot_cpu_data (arch/x86/kernel/smpboot.c:402). AFAICT this is done because *new* cpus (ie, physical hotplug) should be initialized with some basic data from boot_cpu_data. ie) for an already existing cpu it looks like this is unnecessary as the data has already been initialized. Andi's idea would work and the code would become much cleaner, if smp_store_cpu_info() only overwrote cpu_data for new physically hotplugged cpus. Admittedly, I might be missing some path to that code so I'm looking for verification. What do you think? P. > > -Andi >