From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S932739AbcHKMsu (ORCPT <rfc822;w@1wt.eu>);
	Thu, 11 Aug 2016 08:48:50 -0400
Received: from merlin.infradead.org ([205.233.59.134]:36304 "EHLO
	merlin.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S932537AbcHKMsr (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Thu, 11 Aug 2016 08:48:47 -0400
Date: Thu, 11 Aug 2016 14:48:39 +0200
From: Peter Zijlstra <peterz@infradead.org>
To: Jiri Olsa <jolsa@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>, Andi Kleen <andi@firstfloor.org>,
        linux-kernel@vger.kernel.org, Andi Kleen <ak@linux.intel.com>,
        x86@kernel.org, Ingo Molnar <mingo@kernel.org>,
        Frank Ramsay <framsay@redhat.com>, Prarit Bhargava <prarit@redhat.com>
Subject: Re: [PATCH] x86/smp: Fix __max_logical_packages value setup
Message-ID: <20160811124839.GQ6879@twins.programming.kicks-ass.net>
References: <20160803162358.GA10890@krava>
 <20160810135417.GP30192@twins.programming.kicks-ass.net>
 <20160810140033.GA23798@krava>
 <20160810141538.GA28551@krava>
 <20160810155205.GR30192@twins.programming.kicks-ass.net>
 <20160810161417.GA11369@krava>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20160810161417.GA11369@krava>
User-Agent: Mutt/1.5.23.1 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Wed, Aug 10, 2016 at 06:14:18PM +0200, Jiri Olsa wrote:
> > Maybe one nit, the variable is no longer used for a max, so maybe rename
> > it too?

> diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h
> index cf75871d2f81..c28010088651 100644
> --- a/arch/x86/include/asm/topology.h
> +++ b/arch/x86/include/asm/topology.h
> @@ -118,8 +118,8 @@ extern const struct cpumask *cpu_coregroup_mask(int cpu);
>  #define topology_core_cpumask(cpu)		(per_cpu(cpu_core_map, cpu))
>  #define topology_sibling_cpumask(cpu)		(per_cpu(cpu_sibling_map, cpu))
>  
> -extern unsigned int __max_logical_packages;
> -#define topology_max_packages()			(__max_logical_packages)
> +extern unsigned int logical_packages;
> +#define topology_max_packages()			(logical_packages)
>  
>  extern int __max_smt_threads;
>  

Aaahh, I missed this bit yesterday..


Imagine a machine with physical hotplug, where we boot with half the
sockets populated (say 2), then topology_max_packages() will return 2
when we run the uncore driver init.

That driver will allocate resources based on 2.

Then we physically hotplug the remaining sockets (another 2), which will
result in topology_max_packages() to now return 4.

When we run the cpuhotplug hook, it will try and access resources based
on 4, which were not allocated.


blergh..