[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <54cbcf15-3493-f94f-fa1f-771dec4925ca@linux.vnet.ibm.com>
Date: Wed, 15 Nov 2017 11:38:04 -0600
From: Michael Bringmann <mwb@...ux.vnet.ibm.com>
To: Michael Ellerman <mpe@...erman.id.au>,
linuxppc-dev@...ts.ozlabs.org, linux-kernel@...r.kernel.org
Cc: John Allen <jallen@...ux.vnet.ibm.com>,
Nathan Fontenot <nfont@...ux.vnet.ibm.com>,
Tyrel Datwyler <tyreld@...ux.vnet.ibm.com>,
Thomas Falcon <tlfalcon@...ux.vnet.ibm.com>
Subject: Re: [PATCH V2 2/3] pseries/findnodes: Find nodes with memory for
memoryless nodes
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 <mwb@...ux.vnet.ibm.com> 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@...ux.vnet.ibm.com
Powered by blists - more mailing lists