Re: [PATCH 1/2] ARM: Remove redundant ARCH_SPARSEMEM_DEFAULT setting

[Florian Fainelli <f.fainelli@gmail.com>]

On 5/7/2020 1:29 PM, Mike Rapoport wrote:
> On Thu, May 07, 2020 at 11:30:39AM +0100, Russell King - ARM Linux admin wrote:
>> On Wed, May 06, 2020 at 04:50:08PM -0700, Florian Fainelli wrote:
>>> From: Kevin Cernekee <cernekee@gmail.com>
>>>
>>> If ARCH_SPARSEMEM_ENABLE=y and ARCH_{FLATMEM,DISCONTIGMEM}_ENABLE=n,
>>> then the logic in mm/Kconfig already makes CONFIG_SPARSEMEM the only
>>> choice.  This is true for all of the existing ARM users of
>>> ARCH_SPARSEMEM_ENABLE.
>>>
>>> Forcing ARCH_SPARSEMEM_DEFAULT=y if ARCH_SPARSEMEM_ENABLE=y prevents
>>> us from ever defaulting to FLATMEM, so we should remove this setting.
>>
>> No explanation why that is desirable.
>>
>>> -config ARCH_SPARSEMEM_DEFAULT
>>> -	def_bool ARCH_SPARSEMEM_ENABLE
>>> -
>>
>> What this basically says is ARCH_SPARSEMEM_ENABLE=ARCH_SPARSEMEM_DEFAULT,
>> which is exactly what we want for the non-multiplatform boards that
>> select ARCH_SPARSEMEM_ENABLE - we _want_ them to default to sparsemem
>> because that is what the platform requires.
> 
> The ARCH_SPARSEMEM_DEFAULT knob has no effect unless manual selection
> of the memory model is enabled, i.e. arch Kconfig sets
> ARCH_SELECT_MEMORY_MODEL to 'y'.
> 
> Otherwise, ARCH_SPARSEMEM_ENABLE is sufficient to select SPARSEMEM over
> FLATMEM:
> 
> config SPARSEMEM
> 	def_bool y
> 	depends on (!SELECT_MEMORY_MODEL && ARCH_SPARSEMEM_ENABLE) || SPARSEMEM_MANUAL
> 
> config FLATMEM
> 	def_bool y
> 	depends on (!DISCONTIGMEM && !SPARSEMEM) || FLATMEM_MANUAL
> 
>> For example, with RiscPC, which selects ARCH_SPARSEMEM_ENABLE, we have
>> four banks of memory at 0x10000000, 0x14000000, 0x18000000 and
>> 0x1c000000.  These correspond with the two memory slots - the first two
>> for the first slot, and the second two for the second slot.  Each slot
>> has two banks.  The size of each memory bank depends on the size of the
>> module.
>>
>> Flatmem is completely unable to work with this setup if all banks are
>> populated, and the first bank does not contain enough memory to allocate
>> the struct page array.  So, sparsemem is the only option there.
>>
>> Hence, for these platforms, we want sparsemem and only sparsemem, not
>> flatmem.
>>
>> So, this patch which makes it possible to select flatmem is completely
>> out of the question for these platforms.
> 
> This patch alone won't make it possible to select flatmem for these
> platforms, actually, as of now it changes nothing.
> 
> The removal of ARCH_SPARSEMEM_DEFAULT will have an effect only with
> addtion of ARCH_SELECT_MEMORY_MODEL.
> 
> If I understood Florian's intention correctly, the goal was to allow
> manual selection of the memory model for multiplatform builds that today
> implicitly use flatmem.

That is correct.

> 
> I think that the patch below would achieve that without changing the
> current defaults and without forcing flatmem on the platforms that
> already explicitly select sparsemem.

Agree, with your patch, we have the following:

mutli_v7_defonfig -> FLATMEM
ep93xx_defconfig -> SPARSEMEM
rpc_defconfig -> SPARSEMEM
assabet_defconfig -> SPARSEMEM

with the bonus that when you run menuconfig you can now select sparsemem
for a multi-platform build, which was the intention.

Tested-by: Florian Fainelli <f.fainelli@gmail.com>


> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 66a04f6f4775..b6eb1a28ca27 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -312,6 +312,9 @@ choice
>  config ARCH_MULTIPLATFORM
>  	bool "Allow multiple platforms to be selected"
>  	depends on MMU
> +	select ARCH_FLATMEM_ENABLE
> +	select ARCH_SPARSEMEM_ENABLE
> +	select ARCH_SELECT_MEMORY_MODEL
>  	select ARM_HAS_SG_CHAIN
>  	select ARM_PATCH_PHYS_VIRT
>  	select AUTO_ZRELADDR
> @@ -1515,11 +1518,14 @@ config OABI_COMPAT
>  config ARCH_HAS_HOLES_MEMORYMODEL
>  	bool
>  
> +config ARCH_FLATMEM_ENABLE
> +	bool
> +
>  config ARCH_SPARSEMEM_ENABLE
>  	bool
>  
> -config ARCH_SPARSEMEM_DEFAULT
> -	def_bool ARCH_SPARSEMEM_ENABLE
> +config ARCH_SELECT_MEMORY_MODEL
> +	bool
>  
>  config HAVE_ARCH_PFN_VALID
>  	def_bool ARCH_HAS_HOLES_MEMORYMODEL || !SPARSEMEM
> 
>> -- 
>> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
>> FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up
> 

-- 
Florian

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel