From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932837AbdKORia (ORCPT ); Wed, 15 Nov 2017 12:38:30 -0500 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:43712 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753038AbdKORiX (ORCPT ); Wed, 15 Nov 2017 12:38:23 -0500 Subject: Re: [PATCH V2 2/3] pseries/findnodes: Find nodes with memory for memoryless nodes To: Michael Ellerman , linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org Cc: John Allen , Nathan Fontenot , Tyrel Datwyler , Thomas Falcon References: <87vajbldlw.fsf@concordia.ellerman.id.au> From: Michael Bringmann Organization: IBM Linux Technology Center Date: Wed, 15 Nov 2017 11:38:04 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <87vajbldlw.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: 17111517-0012-0000-0000-0000154C9A65 X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00008067; HX=3.00000241; KW=3.00000007; PH=3.00000004; SC=3.00000240; SDB=6.00946309; UDB=6.00477661; IPR=6.00726606; BA=6.00005692; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009; ZB=6.00000000; ZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00018032; XFM=3.00000015; UTC=2017-11-15 17:38:08 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17111517-0013-0000-0000-0000504726FE Message-Id: <54cbcf15-3493-f94f-fa1f-771dec4925ca@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-11-15_09:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501 malwarescore=0 suspectscore=0 phishscore=0 bulkscore=0 spamscore=0 clxscore=1011 lowpriorityscore=0 impostorscore=0 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1709140000 definitions=main-1711150235 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hello: Sorry for the out-of-date description. This entire patch has been removed / eliminated from subsequent patch sets. All changes to correct powerpc memoryless nodes will be confined to powerpc-specific code. Regards, Michael On 10/19/2017 03:56 AM, Michael Ellerman wrote: > Hi Michael, > > Michael Bringmann writes: >> pseries/findnodes: On pseries systems which allow 'hot-add' of > > This isn't a powerpc or pseries patch, so the subject/prefix is wrong. > > Also because you're changing generic code you need to provide an > explanation that makes sense in general, across all architectures, not > just in terms of what the pseries platform does. > >> resources, we may boot configurations that have CPUs, but no memory >> associated to a node by the affinity calculations. > > This is called a "memory-less node" and is understood by the generic > code. > >> Previously, the >> software took a shortcut to collapse initialization and references > > What software? What shortcut? > >> to such memoryless nodes with other nodes that did have memory >> associated with them at boot. This patch is based on fixes that > > What fixes? > >> allow the proper initialization and distinguishment of memoryless >> and memory-plus nodes after NUMA initialization. > > What exactly is unproper about the current code? > >> It extends the >> use of the 'node_to_mem_node()' API from 'topology.h' to modules > > The term "modules" has a specific meaning in Linux which is not correct > here. We would just say "in two functions" or "in two files". > >> that are allocating node-specific memory at boot, and allows such >> references to find available memory in another node. > > >> diff --git a/block/blk-mq-cpumap.c b/block/blk-mq-cpumap.c >> index 9f8cffc..a27a31f 100644 >> --- a/block/blk-mq-cpumap.c >> +++ b/block/blk-mq-cpumap.c >> @@ -73,7 +73,8 @@ int blk_mq_hw_queue_to_node(unsigned int *mq_map, unsigned int index) >> >> for_each_possible_cpu(i) { >> if (index == mq_map[i]) >> - return local_memory_node(cpu_to_node(i)); >> + return local_memory_node( >> + node_to_mem_node(cpu_to_node(i))); > > What is this trying to do? > > local_memory_node() is supposed to return a "local" node for nodes with > no memory. > > And in fact the comment says: > > * Used for initializing percpu 'numa_mem' > > Which is what we do: > > set_numa_mem(local_memory_node(numa_cpu_lookup_table[cpu])); > > And is what's returned by node_to_mem_node(): > > static inline void set_numa_mem(int node) > { > this_cpu_write(_numa_mem_, node); > _node_numa_mem_[numa_node_id()] = node; > } > > static inline int node_to_mem_node(int node) > { > return _node_numa_mem_[node]; > } > > So your change effectively ends up doing: > > return local_memory_node(local_memory_node(cpu_to_node(i))); > > Which doesn't look right. > > > 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