From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1762965AbdJQPJh (ORCPT ); Tue, 17 Oct 2017 11:09:37 -0400 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:36734 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752721AbdJQPJf (ORCPT ); Tue, 17 Oct 2017 11:09:35 -0400 Subject: Re: [PATCH 2/2] powerpc/hotplug: Ensure nodes initialized for hotplug To: Michael Ellerman , linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org Cc: John Allen , Nathan Fontenot References: <87y3objlrk.fsf@concordia.ellerman.id.au> From: Michael Bringmann Organization: IBM Linux Technology Center Date: Tue, 17 Oct 2017 10:08:51 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.0 MIME-Version: 1.0 In-Reply-To: <87y3objlrk.fsf@concordia.ellerman.id.au> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit X-TM-AS-GCONF: 00 x-cbid: 17101715-0012-0000-0000-000015289DE3 X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00007906; HX=3.00000241; KW=3.00000007; PH=3.00000004; SC=3.00000237; SDB=6.00932465; UDB=6.00469574; IPR=6.00712755; BA=6.00005642; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009; ZB=6.00000000; ZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00017576; XFM=3.00000015; UTC=2017-10-17 15:08:59 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17101715-0013-0000-0000-00004FE92240 Message-Id: <240499bb-081e-fb9e-1083-bd4c72ef404c@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-10-17_11:,, 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-1707230000 definitions=main-1710170213 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10/16/2017 07:54 AM, Michael Ellerman wrote: > Michael Bringmann writes: > >> powerpc/hotplug: On systems like PowerPC which allow 'hot-add' of CPU, >> it may occur that the new resources are to be inserted into nodes >> that were not used for memory resources at bootup. Many different >> configurations of PowerPC resources may need to be supported depending >> upon the environment. > > Give me some detail please?! Configurations that demonstrated problems included 'memoryless' nodes that possessed only CPUs at boot, and nodes that contained neither CPUs nor memory at boot. The calculations in the kernel resulted in a different node use layout on many SAP HANA configured systems. > >> This patch fixes some problems encountered at > > What problems? The previous implementation collapsed all node assignments after affinity calculations to use only those nodes that had memory at boot. This resulted in calculation and configuration differences between the FSP code and the Linux kernel. > >> runtime with configurations that support memory-less nodes, but which >> allow CPUs to be added at and after boot. > > How does it fix those problems? The change involves completing the initialization of nodes that were not used at boot, but which were selected by VPHN affinity calculations during subsequent hotplug operations. Michael > >> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c >> index b385cd0..e811dd1 100644 >> --- a/arch/powerpc/mm/numa.c >> +++ b/arch/powerpc/mm/numa.c >> @@ -1325,6 +1325,17 @@ static long vphn_get_associativity(unsigned long cpu, >> return rc; >> } >> >> +static int verify_node_preparation(int nid) >> +{ > > I would not expect a function called "verify" ... > >> + if ((NODE_DATA(nid) == NULL) || >> + (NODE_DATA(nid)->node_spanned_pages == 0)) { >> + if (try_online_node(nid)) > > .. to do something like online a node. > >> + return first_online_node; >> + } >> + >> + return nid; >> +} >> + >> /* >> * Update the CPU maps and sysfs entries for a single CPU when its NUMA >> * characteristics change. This function doesn't perform any locking and is >> @@ -1433,9 +1444,11 @@ int numa_update_cpu_topology(bool cpus_locked) >> /* Use associativity from first thread for all siblings */ >> vphn_get_associativity(cpu, associativity); >> new_nid = associativity_to_nid(associativity); >> - if (new_nid < 0 || !node_online(new_nid)) >> + if (new_nid < 0 || !node_possible(new_nid)) >> new_nid = first_online_node; >> >> + new_nid = verify_node_preparation(new_nid); > > You're being called part-way through CPU hotplug here, are we sure it's > safe to go and do memory hotplug from there? What's the locking > situation? > > cheers > > -- Michael W. Bringmann Linux Technology Center IBM Corporation Tie-Line 363-5196 External: (512) 286-5196 Cell: (512) 466-0650 mwb@linux.vnet.ibm.com