Re: [PATCH] mm: swap the definition of CONFIG_SPARSEMEM_VMEMMAP and CONFIG_SPARSEMEM

[Arnd Bergmann]

On Fri, Nov 4, 2022, at 04:28, zhijun.han wrote:
> CONFIG_SPARSEMEM_VMEMMAP depends on CONFIG_SPARSEMEM
> When CONFIG_SPARSEMEM_VMEMMAP is enabled,
> CONFIG_SPARSEMEM will be enabled too.
> Causes __pfn_to_page and __page_to_pfn to be overwritten
>
> Signed-off-by: zhijun.han <hanzj.it@xxxxxxxxx>

I'm sorry, but I don't follow the logic here. Your explanation
makes sense, but from your patch it appears that your are
introducing the problem that you explain, instead of fixing it.

> --- a/include/asm-generic/memory_model.h
> +++ b/include/asm-generic/memory_model.h
> @@ -19,12 +19,6 @@
>  #define __page_to_pfn(page)	((unsigned long)((page) - mem_map) + \
>  				 ARCH_PFN_OFFSET)
> 
> -#elif defined(CONFIG_SPARSEMEM_VMEMMAP)
> -
> -/* memmap is virtually contiguous.  */
> -#define __pfn_to_page(pfn)	(vmemmap + (pfn))
> -#define __page_to_pfn(page)	(unsigned long)((page) - vmemmap)
> -
>  #elif defined(CONFIG_SPARSEMEM)
>  /*
>   * Note: section's mem_map is encoded to reflect its start_pfn.

What I see in the existing code is that checking
CONFIG_SPARSEMEM_VMEMMAP first catches only the case where
both SPARSEMEM and SPARSEMEM_VMEMMAP are set, so the third
elseif case is not used. When you swap them around, we
always get the definition for the SPARSEMEM case, since the
third elseif is never reached.

     Arnd