From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0a-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com [148.163.158.5]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 3ygZTM6Kr3zDrST for ; Tue, 21 Nov 2017 03:50:15 +1100 (AEDT) Received: from pps.filterd (m0098419.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.21/8.16.0.21) with SMTP id vAKGnSnR084441 for ; Mon, 20 Nov 2017 11:50:12 -0500 Received: from e31.co.us.ibm.com (e31.co.us.ibm.com [32.97.110.149]) by mx0b-001b2d01.pphosted.com with ESMTP id 2ebydq68f2-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Mon, 20 Nov 2017 11:50:12 -0500 Received: from localhost by e31.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 20 Nov 2017 09:50:11 -0700 Subject: Re: [PATCH V7 3/3] hotplug/cpu: Fix crash with memoryless nodes To: Michael Bringmann , linuxppc-dev@lists.ozlabs.org Cc: Michael Ellerman , John Allen , Tyrel Datwyler , Thomas Falcon References: <3445a368-d2d4-f908-5f0e-df7bbd659e07@linux.vnet.ibm.com> From: Nathan Fontenot Date: Mon, 20 Nov 2017 10:50:06 -0600 MIME-Version: 1.0 In-Reply-To: <3445a368-d2d4-f908-5f0e-df7bbd659e07@linux.vnet.ibm.com> Content-Type: text/plain; charset=utf-8 Message-Id: <08960615-15b4-1a14-a10b-9f0053c8163e@linux.vnet.ibm.com> List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On 11/16/2017 11:28 AM, Michael Bringmann wrote: > On powerpc systems with shared configurations of CPUs and memory and > memoryless nodes at boot, an event ordering problem was observed on > a SLES12 build platforms with the hot-add of CPUs to the memoryless > nodes. > > * The most common error occurred when the memory SLAB driver attempted > to reference the memoryless node to which a CPU was being added > before the kernel had finished initializing all of the data structures > for the CPU and exited 'device_online' under DLPAR/hot-add. > > Normally the memoryless node would be initialized through the call > path device_online ... arch_update_cpu_topology ... find_cpu_nid > ... try_online_node. This patch ensures that the powerpc node will > be initialized as early as possible, even if it was memoryless and > CPU-less at the point when we are trying to hot-add a new CPU to it. > > Signed-off-by: Michael Bringmann > --- > Changes in V7: > -- Make function find_cpu_nid() externally visible/usable so that > it may be used from hotplug-cpu.c > --- > arch/powerpc/mm/numa.c | 3 ++- > arch/powerpc/platforms/pseries/hotplug-cpu.c | 3 +++ > 2 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c > index 163f4cc..d6d4f7c 100644 > --- a/arch/powerpc/mm/numa.c > +++ b/arch/powerpc/mm/numa.c > @@ -1310,7 +1310,7 @@ static long vphn_get_associativity(unsigned long cpu, > return rc; > } > > -static inline int find_cpu_nid(int cpu) > +int find_cpu_nid(int cpu) > { > __be32 associativity[VPHN_ASSOC_BUFSIZE] = {0}; > int new_nid; > @@ -1343,6 +1343,7 @@ static inline int find_cpu_nid(int cpu) > #endif > } > > + printk(KERN_INFO "%s:%d cpu %d nid %d\n", __FUNCTION__, __LINE__, cpu, new_nid); This seems like a more likely pr_debug statement. > return new_nid; > } > > diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c > index a7d14aa7..df8c732 100644 > --- a/arch/powerpc/platforms/pseries/hotplug-cpu.c > +++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c > @@ -340,6 +340,8 @@ static void pseries_remove_processor(struct device_node *np) > cpu_maps_update_done(); > } > > +extern int find_cpu_nid(int cpu); > + > static int dlpar_online_cpu(struct device_node *dn) > { > int rc = 0; > @@ -364,6 +366,7 @@ static int dlpar_online_cpu(struct device_node *dn) > != CPU_STATE_OFFLINE); > cpu_maps_update_done(); > timed_topology_update(1); > + find_cpu_nid(cpu); We don't use the returned node from this call, so I'm not sure why it gets called. Perhaps its the possible call to try_online_node() that may get called in find_cpu_nid(), if so perhpas naming the routine something slightly different would be good, like find_and_online_cpu_nid? -Nathan > rc = device_online(get_cpu_device(cpu)); > if (rc) > goto out; >