Re: [PATCH] sparsemem fix

[Franck Bui-Huu <vagabon.xyz@gmail.com>]

Atsushi Nemoto wrote:
> 1. MIPS should select SPARSEMEM_STATIC since allocating bootmem in
>    memory_present() will corrupt bootmap area.
> 2. pfn_valid() for SPARSEMEM is defined in linux/mmzone.h
> 
> Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
> 
> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index f151a7e..879a19c 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -1690,6 +1690,7 @@ config ARCH_DISCONTIGMEM_ENABLE
>  
>  config ARCH_SPARSEMEM_ENABLE
>  	bool
> +	select SPARSEMEM_STATIC
>  
>  config NUMA
>  	bool "NUMA Support"
> diff --git a/include/asm-mips/page.h b/include/asm-mips/page.h
> index 6b97744..6ed1151 100644
> --- a/include/asm-mips/page.h
> +++ b/include/asm-mips/page.h
> @@ -138,16 +138,14 @@ #define __va(x)			((void *)((unsigned lo
>  
>  #define pfn_to_kaddr(pfn)	__va((pfn) << PAGE_SHIFT)
>  
> -#ifndef CONFIG_SPARSEMEM
> -#ifndef CONFIG_NEED_MULTIPLE_NODES
> -#define pfn_valid(pfn)		((pfn) < max_mapnr)
> -#endif
> -#endif
> -
>  #ifdef CONFIG_FLATMEM
>  
>  #define pfn_valid(pfn)		((pfn) < max_mapnr)
>  

In flatmem case, I would define pfn_valid like:

#define pfn_valid(pfn)          ((pfn) >= ARCH_PFN_OFFSET && (pfn) < max_mapnr)

> +#elif defined(CONFIG_SPARSEMEM)
> +
> +/* pfn_valid is defined in linux/mmzone.h */
> +
>  #elif defined(CONFIG_NEED_MULTIPLE_NODES)

why not using:

#elif defined(CONFIG_DISCONTIGMEM) || defined(CONFIG_NUMA)

hence, we would have all memory model cases.

For now it seems to be implemented only in sgi-ip27 machine. Maybe we should
make things clear by adding:

#ifdef CONFIG_SGI_IP27
#define pfn_valid	[...]
#else
#error discontigmem model is only supported by sgi-ip27 platforms.
#error Please try to implement a generic solution if you plan to
#error use this memory model. Good luck ;)
#endif /* CONFIG_SGI_IP27 */

>  
>  #define pfn_valid(pfn)							\
> @@ -159,8 +157,6 @@ ({									\
>  	            : 0);						\
>  })
>  
> -#else
> -#error Provide a definition of pfn_valid
>  #endif

maybe this would be better too ?

#else

#error Unknow memory model, provide a definition of pfn_valid

#endif

>  
>  #define virt_to_page(kaddr)	pfn_to_page(__pa(kaddr) >> PAGE_SHIFT)
> 
>