[4/4] [RFC] ppc64 abstract discontig

[Dave Hansen]

Nice patch set.  Every line we move under CONFIG_DISCONTIG is another
line we can easily kill later.  Some tiny comments below.

On Thu, 2005-05-19 at 17:47 +0100, Andy Whitcroft wrote: 
> @@ -499,6 +511,7 @@ static void __init dump_numa_topology(vo
>  
>  		printk(KERN_INFO "Node %d Memory:", node);
>  
> +#if 0
>  		count = 0;
>  
>  		for (i = 0; i < lmb_end_of_DRAM(); i += MEMORY_INCREMENT) {
> @@ -515,6 +528,7 @@ static void __init dump_numa_topology(vo
>  
>  		if (count > 0)
>  			printk("-0x%lx", i);
> +#endif
>  		printk("\n");
>  	}
>  	return;

Just kill this code section if it's really not needed.

As it stands now, this patch series looks like it adds quite a bit of
code.  

 arch/ppc64/mm/numa.c               |   44 +++++++++++++++++++++++++++++------
 current/arch/ppc64/Kconfig         |    4 +++
 current/arch/ppc64/mm/numa.c       |   32 ++++++++++++-------------
 current/include/asm-ppc64/mmzone.h |    2 -
 include/asm-ppc64/mmzone.h         |   46 ++++++++++++++++++++-----------------
 5 files changed, 83 insertions(+), 45 deletions(-)

That should change pretty significantly once you kill that hunk.

> diff -upN reference/include/asm-ppc64/mmzone.h current/include/asm-ppc64/mmzone.h
> --- reference/include/asm-ppc64/mmzone.h
> +++ current/include/asm-ppc64/mmzone.h
> @@ -30,7 +30,9 @@ extern struct pglist_data *node_data[];
>   */
>  
>  extern int numa_cpu_lookup_table[];
> +#ifdef CONFIG_DISCONTIGMEM
>  extern char *numa_memory_lookup_table;
> +#endif
>  extern cpumask_t numa_cpumask_lookup_table[];
>  extern int nr_cpus_in_node[];

How about renaming that variable to not have "numa_" in it if it isn't
actually numa-dependent?

Another patch on top of these 4 to do

	s/numa(_memory_lookup_table)/discontig$1/

Would be nice.

> @@ -62,8 +64,8 @@ static inline int pfn_to_nid(unsigned lo
>  {
>  	int nid;
>  
> -	nid = numa_memory_lookup_table[pfn >>
> -					(MEMORY_INCREMENT_SHIFT - PAGE_SHIFT)];
> +	nid = numa_memory_lookup_table[pfn >> (MEMORY_INCREMENT_SHIFT -
> +								PAGE_SHIFT)];

A little pet peeve of mine is when there is lots of shift arithmetic
going on.  What does FOO_SHIFT - BAR_SHIFT really *mean*?

If you're going to modify this, can you fix it to something like?

	unsigned long pages_per_table_entry = 
			MEMORY_INCREMENT_SHIFT - PAGE_SHIFT;

-- Dave