Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753675AbcLHMwN (ORCPT ); Thu, 8 Dec 2016 07:52:13 -0500 Received: from Galois.linutronix.de ([146.0.238.70]:45980 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751892AbcLHMwM (ORCPT ); Thu, 8 Dec 2016 07:52:12 -0500 Date: Thu, 8 Dec 2016 13:49:28 +0100 (CET) From: Thomas Gleixner To: Peter Zijlstra cc: LKML , x86@kernel.org, Borislav Petkov , "Charles (Chas) Williams" , "M. Vefa Bicakci" , Alok Kataria , Boris Ostrovsky Subject: Re: [PATCH] x86/smpboot: Make logical package management more robust In-Reply-To: <20161208110331.GK3107@twins.programming.kicks-ass.net> Message-ID: References: <20161208110331.GK3107@twins.programming.kicks-ass.net> User-Agent: Alpine 2.20 (DEB 67 2015-01-07) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2164 Lines: 57 On Thu, 8 Dec 2016, Peter Zijlstra wrote: > On Thu, Dec 08, 2016 at 10:04:25AM +0100, Thomas Gleixner wrote: > > @@ -289,16 +293,17 @@ int topology_update_package_map(unsigned > > if (test_and_set_bit(pkg, physical_package_map)) > > goto found; > > > > - if (logical_packages_frozen) { > > - physical_to_logical_pkg[pkg] = -1; > > - pr_warn("APIC(%x) Package %u exceeds logical package max\n", > > - apicid, pkg); > > + if (logical_packages >= __max_logical_packages) { > > + pr_warn("Package %u of CPU %u exceeds BIOS package data %u.\n", > > + logical_packages, cpu, __max_logical_packages); > > return -ENOSPC; > > } > > > > new = logical_packages++; > > - pr_info("APIC(%x) Converting physical %u to logical package %u\n", > > - apicid, pkg, new); > > + if (new != pkg) { > > + pr_info("CPU %u Converting physical %u to logical package %u\n", > > + cpu, pkg, new); > > + } > > This makes the print conditional on the phy<->logical mapping not > matching; I thought it was a concious decision to print everything in > the initial version. > > This way, if we have a 4 node system and nodes 1,2 are crossed we'll > only see: > > "Converting physical 2 to logical package 1" > "Converting physical 1 to logical package 2" > > And nothing on the other two nodes, which could be slightly confusing. Fair enough. I make it unconditional again. > > - if (logical_packages > __max_logical_packages) { > > - pr_warn("Detected more packages (%u), then computed by BIOS data (%u).\n", > > - logical_packages, __max_logical_packages); > > - logical_packages_frozen = true; > > - __max_logical_packages = logical_packages; > > So we'll never 'shrink' the initially computed max; which could result > in using more memory than strictly needed, otoh it makes physical > hotplug happier. Yes. I was debating that back and forth and at the end decided that making it simple and robust is a good tradeoff vs. the slightly higher memory consumption. Though on most systems that's a non issue as number of possible cpus/packages is the same as the actual available ones. The insane setups have to suffer - rightfully so. Thanks, tglx