From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1424662AbdEYWFw (ORCPT ); Thu, 25 May 2017 18:05:52 -0400 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:54575 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1163123AbdEYWFp (ORCPT ); Thu, 25 May 2017 18:05:45 -0400 Subject: Re: [PATCH V2 1/2] powerpc/numa: Update CPU topology when VPHN enabled To: Michael Bringmann , linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org, Paul Mackerras References: <6d5f8438-094b-41b4-a95b-97f89c4d6a08@linux.vnet.ibm.com> Cc: Tyrel Datwyler , Sahil Mehta , Rashmica Gupta , Reza Arbab , Ingo Molnar , John Allen , Shailendra Singh , Andrew Donnellan , Bharata B Rao , Nathan Fontenot , Thomas Gleixner , Sebastian Andrzej Siewior , "Aneesh Kumar K.V" From: Tyrel Datwyler Date: Thu, 25 May 2017 15:05:31 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <6d5f8438-094b-41b4-a95b-97f89c4d6a08@linux.vnet.ibm.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 x-cbid: 17052522-0028-0000-0000-000007AB391A X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00007117; HX=3.00000241; KW=3.00000007; PH=3.00000004; SC=3.00000212; SDB=6.00865575; UDB=6.00429835; IPR=6.00645398; BA=6.00005375; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009; ZB=6.00000000; ZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00015583; XFM=3.00000015; UTC=2017-05-25 22:05:38 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17052522-0029-0000-0000-000035EB7935 Message-Id: X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-05-25_17:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1703280000 definitions=main-1705250401 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 05/25/2017 10:35 AM, Michael Bringmann wrote: > > powerpc/numa: Correct the currently broken capability to set the > topology for shared CPUs in LPARs. At boot time for shared CPU > lpars, the topology for each shared CPU is set to node zero, however, > this is now updated correctly using the Virtual Processor Home Node > (VPHN) capabilities information provided by the pHyp. The VPHN handling > in Linux is disabled, if PRRN handling is present. > > Signed-off-by: Michael Bringmann > --- > arch/powerpc/mm/numa.c | 19 ++++++++++++++++++- > arch/powerpc/platforms/pseries/dlpar.c | 2 ++ > arch/powerpc/platforms/pseries/hotplug-cpu.c | 3 ++- > 3 files changed, 22 insertions(+), 2 deletions(-) > > diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c > index 371792e..15c2dd5 100644 > --- a/arch/powerpc/mm/numa.c > +++ b/arch/powerpc/mm/numa.c > @@ -29,6 +29,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -42,6 +43,8 @@ > #include > > static int numa_enabled = 1; > +static int topology_inited; > +static int topology_update_needed; > > static char *cmdline __initdata; > > @@ -1321,8 +1324,11 @@ int arch_update_cpu_topology(void) > struct device *dev; > int weight, new_nid, i = 0; > > - if (!prrn_enabled && !vphn_enabled) > + if (!prrn_enabled && !vphn_enabled) { > + if (!topology_inited) > + topology_update_needed = 1; > return 0; > + } > > weight = cpumask_weight(&cpu_associativity_changes_mask); > if (!weight) > @@ -1361,6 +1367,8 @@ int arch_update_cpu_topology(void) > cpumask_andnot(&cpu_associativity_changes_mask, > &cpu_associativity_changes_mask, > cpu_sibling_mask(cpu)); > + pr_info("Assoc chg gives same node %d for cpu%d\n", > + new_nid, cpu); > cpu = cpu_last_thread_sibling(cpu); > continue; > } > @@ -1377,6 +1385,9 @@ int arch_update_cpu_topology(void) > cpu = cpu_last_thread_sibling(cpu); > } > > + if (i) > + updates[i-1].next = NULL; > + > pr_debug("Topology update for the following CPUs:\n"); > if (cpumask_weight(&updated_cpus)) { > for (ud = &updates[0]; ud; ud = ud->next) { > @@ -1423,6 +1434,7 @@ int arch_update_cpu_topology(void) > > out: > kfree(updates); > + topology_update_needed = 0; > return changed; > } > > @@ -1600,6 +1612,11 @@ static int topology_update_init(void) > if (!proc_create("powerpc/topology_updates", 0644, NULL, &topology_ops)) > return -ENOMEM; > > + topology_inited = 1; > + if (topology_update_needed) > + bitmap_fill(cpumask_bits(&cpu_associativity_changes_mask), > + nr_cpumask_bits); > + > return 0; > } > device_initcall(topology_update_init); > diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c > index bda18d8..5106263 100644 > --- a/arch/powerpc/platforms/pseries/dlpar.c > +++ b/arch/powerpc/platforms/pseries/dlpar.c > @@ -592,6 +592,8 @@ static ssize_t dlpar_show(struct class *class, struct class_attribute *attr, > > static int __init pseries_dlpar_init(void) > { > + arch_update_cpu_topology(); > + > pseries_hp_wq = alloc_workqueue("pseries hotplug workqueue", > WQ_UNBOUND, 1); > return sysfs_create_file(kernel_kobj, &class_attr_dlpar.attr); > diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c > index 7bc0e91..b5eff35 100644 > --- a/arch/powerpc/platforms/pseries/hotplug-cpu.c > +++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c > @@ -619,7 +619,8 @@ static int dlpar_cpu_remove_by_index(u32 drc_index) > } > > rc = dlpar_cpu_remove(dn, drc_index); > - of_node_put(dn); > + if (rc) > + of_node_put(dn); This if statement is unnecessary. A reference to dn is grabbed with cpu_drc_index_to_dn() earlier in the function. So, regardless of whether dlpar_cpu_remove() succeeds or fails we still need to release the reference we took. I suspect that maybe you threw this in there to prevent the of_node underflow that was being seen during dlpar remove of cpus. A fix for that should already be upstream. Either way this hunk doesn't seem to have anything to do with the rest of this VPHN patch, and should have been a separate patch with its own change log explaining the reasoning. -Tyrel > return rc; > } > >