[PATCH 2/3] malloc: Don't statically initialize av_ if using malloc_init

[Heinrich Schuchardt]

On 8/1/23 00:33, Sean Anderson wrote:
> When we enable malloc_init, there is no need to statically initialize
> av_, since we are going to do it manually. This lets us move av_ to
> .bss, saving around 1-2k of data (depending on the pointer size).
> 
> cALLOc must be adjusted to not access top before malloc_init.
> 
> While we're at it, rename/reword the Kconfig to better describe what
> this option does.
> 
> Signed-off-by: Sean Anderson <sean.anderson at seco.com>
> ---
> 
>   Kconfig           | 18 +++++++-----------
>   common/dlmalloc.c |  9 +++++++--
>   2 files changed, 14 insertions(+), 13 deletions(-)
> 
> diff --git a/Kconfig b/Kconfig
> index 70efb41cc6..4b32286b69 100644
> --- a/Kconfig
> +++ b/Kconfig
> @@ -372,18 +372,14 @@ if EXPERT
>   	  When disabling this, please check if malloc calls, maybe
>   	  should be replaced by calloc - if one expects zeroed memory.
>   
> -config SYS_MALLOC_DEFAULT_TO_INIT
> -	bool "Default malloc to init while reserving the memory for it"
> +config SYS_MALLOC_RUNTIME_INIT
> +	bool "Initialize malloc's internal data at runtime"
>   	help
> -	  It may happen that one needs to move the dynamic allocation
> -	  from one to another memory range, eg. when moving the malloc
> -	  from the limited static to a potentially large dynamic (DDR)
> -	  memory.
> -
> -	  If so then on top of setting the updated memory aside one
> -	  needs to bring the malloc init.
> -
> -	  If such a scenario is sought choose yes.
> +         Initialize malloc's internal data structures at runtime, rather than
> +         at compile-time. This is necessary if relocating the malloc arena
> +         from a smaller static memory to a large DDR memory. It can also
> +         reduce the size of U-Boot by letting malloc's data reside in .bss
> +         instead of .data.
>   
>   config TOOLS_DEBUG
>   	bool "Enable debug information for tools"
> diff --git a/common/dlmalloc.c b/common/dlmalloc.c
> index 30c78ae976..8a1daae5ec 100644
> --- a/common/dlmalloc.c
> +++ b/common/dlmalloc.c
> @@ -556,6 +556,7 @@ typedef struct malloc_chunk* mbinptr;
>   #define IAV(i)  bin_at(i), bin_at(i)
>   
>   static mbinptr av_[NAV * 2 + 2] = {
> +#if !CONFIG_IS_ENABLED(SYS_MALLOC_RUNTIME_INIT)
>    NULL, NULL,
>    IAV(0),   IAV(1),   IAV(2),   IAV(3),   IAV(4),   IAV(5),   IAV(6),   IAV(7),
>    IAV(8),   IAV(9),   IAV(10),  IAV(11),  IAV(12),  IAV(13),  IAV(14),  IAV(15),
> @@ -573,6 +574,7 @@ static mbinptr av_[NAV * 2 + 2] = {
>    IAV(104), IAV(105), IAV(106), IAV(107), IAV(108), IAV(109), IAV(110), IAV(111),
>    IAV(112), IAV(113), IAV(114), IAV(115), IAV(116), IAV(117), IAV(118), IAV(119),
>    IAV(120), IAV(121), IAV(122), IAV(123), IAV(124), IAV(125), IAV(126), IAV(127)
> +#endif
>   };

With this patch booting qemu-riscv64_spl_defconfig with 
SYS_MALLOC_RUNTIME_INIT=y fails without output from main U-Boot.

After removing the #if above, main U-Boot output provides some output 
but driver binding fails.

Without this patch and with SYS_MALLOC_DEFAULT_TO_INIT=y booting succeeds.

>   
>   #ifdef CONFIG_NEEDS_MANUAL_RELOC
> @@ -623,7 +625,7 @@ void mem_malloc_init(ulong start, ulong size)
>   	mem_malloc_end = start + size;
>   	mem_malloc_brk = start;
>   
> -	if (CONFIG_IS_ENABLED(SYS_MALLOC_DEFAULT_TO_INIT))
> +	if (CONFIG_IS_ENABLED(SYS_MALLOC_RUNTIME_INIT))
>   		malloc_init();
>   
>   	debug("using memory %#lx-%#lx for malloc()\n", mem_malloc_start,
> @@ -2151,7 +2153,10 @@ Void_t* cALLOc(n, elem_size) size_t n; size_t elem_size;
>   #ifdef CONFIG_SYS_MALLOC_CLEAR_ON_INIT
>   #if MORECORE_CLEARS
>     mchunkptr oldtop = top;
> -  INTERNAL_SIZE_T oldtopsize = chunksize(top);
> +  INTERNAL_SIZE_T oldtopsize;
> +  if (CONFIG_IS_ENABLED(SYS_MALLOC_RUNTIME_INIT) &&
> +      !(gd->flags & GD_FLG_FULL_MALLOC_INIT))
> +    oldtopsize = chunksize(top);

malloc_simple needs a value for oldtopsize and this is before 
GD_FLG_FULL_MALLOC_INIT is set.

This change worked for me:

#ifdef CONFIG_SYS_MALLOC_CLEAR_ON_INIT
#if MORECORE_CLEARS
   mchunkptr oldtop = top;
   if (CONFIG_IS_ENABLED(SYS_MALLOC_RUNTIME_INIT) &&
       !(gd->flags & GD_FLG_FULL_MALLOC_INIT))
         malloc_init();
   INTERNAL_SIZE_T oldtopsize = chunksize(top);
#endif
#endif

You don't want to call malloc_init() twice. So some flag should be added 
indicating if malloc_init() has been invoked.

Best regards

Heinrich

>   #endif
>   #endif
>     Void_t* mem = mALLOc (sz);