From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751799AbdJWUAB (ORCPT ); Mon, 23 Oct 2017 16:00:01 -0400 Received: from mx1.redhat.com ([209.132.183.28]:58460 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751674AbdJWUAA (ORCPT ); Mon, 23 Oct 2017 16:00:00 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com B4D7280C1E Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=prarit@redhat.com Subject: Re: [PATCH 2/3 v3] x86/topology: Avoid wasting 128k for package id array To: Thomas Gleixner References: <20171019155713.27097-1-prarit@redhat.com> <20171019155713.27097-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 , He Chen , Mathias Krause , Tim Chen , Vitaly Kuznetsov From: Prarit Bhargava Message-ID: <5d202083-ae7c-1815-102a-fb06db6f6efd@redhat.com> Date: Mon, 23 Oct 2017 15:59:57 -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.26]); Mon, 23 Oct 2017 20:00:00 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10/20/2017 05:03 AM, Thomas Gleixner wrote: > On Thu, 19 Oct 2017, Prarit Bhargava wrote: >> static void remove_siblinginfo(int cpu) >> { >> - int sibling; >> + int phys_pkg_id, sibling; >> struct cpuinfo_x86 *c = &cpu_data(cpu); >> >> for_each_cpu(sibling, topology_core_cpumask(cpu)) { >> @@ -1529,6 +1526,12 @@ static void remove_siblinginfo(int cpu) >> cpumask_clear(topology_core_cpumask(cpu)); >> c->phys_proc_id = 0; >> c->cpu_core_id = 0; >> + >> + phys_pkg_id = c->phys_pkg_id; >> + c->phys_pkg_id = U16_MAX; > > This leaves c->logical_proc_set = 1, which is inconsistent at best. I have > no idea why we need this logical_proc_set flag at all. > >> + if (topology_phys_to_logical_pkg(phys_pkg_id) < 0) >> + logical_packages--; > > Now this has another issue. Depending on hotplug ordering the logical > package association can change across hotplug operations. I don't know it > that's an issue, but this needs to be analyzed before we merge that. > Thanks for making me look at this Thomas. Andi, it looks like this is unfortunately an issue. I have a reworked patchset that fixes by dynamically allocating a u16 logical_to_physical_package array that a) maps the logical to physical package IDs, b) is dynamically sized to the value of logical_packages, and, c) is of size logical_packages (not MAX_APICS) This will resolve the problem that Thomas has pointed out, and it would address the issue of wasting memory. I'm waiting for a 4S & 8S system to test the LTP hotplug tests on and then I'll post. Andi, I'll send it to you in private email for a quick review. P. > Thanks, > > tglx > >