Re: NUMA aware slab allocator V3

[Matthew Dobson]

Dave Hansen wrote:
>>+#ifdef CONFIG_NUMA
>>+#define NUMA_NODES MAX_NUMNODES
>>+#define NUMA_NODE_ID numa_node_id()
>>+#else
>>+#define NUMA_NODES 1
>>+#define NUMA_NODE_ID 0
>> #endif
> 
> 
> I think numa_node_id() should always do what you want.  It is never
> related to discontig nodes, and #defines down to the same thing you have
> in the end, anyway:
>         
>         #define numa_node_id()       (cpu_to_node(_smp_processor_id()))
>         
>         asm-i386/topology.h
>         #ifdef CONFIG_NUMA
>         ...
>         static inline int cpu_to_node(int cpu)
>         {
>                 return cpu_2_node[cpu];
>         }
>         
>         asm-generic/topology.h:
>         #ifndef cpu_to_node
>         #define cpu_to_node(cpu)        (0)
>         #endif
> 
> As for the MAX_NUMNODES, I'd just continue to use it, instead of a new
> #define.  There is no case where there can be more NUMA nodes than
> DISCONTIG nodes, and this assumption appears in plenty of other code.
> 
> I'm cc'ing Matt Dobson, who's touched this MAX_NUMNODES business a lot
> more recently than I.
> 
> -- Dave


You're right, Dave.  The series of #defines at the top resolve to the same
thing as numa_node_id().  Adding the above #defines will serve only to
obfuscate the code.

Another thing that will really help, Christoph, would be replacing all your
open-coded for (i = 0; i < MAX_NUMNODES/NR_CPUS; i++) loops.  We have
macros that make that all nice and clean and (should?) do the right thing
for various combinations of SMP/DISCONTIG/NUMA/etc.  Use those and if they
DON'T do the right thing, please let me know and we'll fix them ASAP.

for_each_cpu(i)
for_each_online_cpu(i)
for_each_node(i)
for_each_online_node(i)

Those 4 macros should replace all your open-coded loops, Christoph.

-Matt
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/