diff mbox series

[v2,3/4] malloc: Don't statically initialize av_ if using malloc_init

Message ID 20230808225320.310926-4-sean.anderson@seco.com
State Changes Requested
Delegated to: Tom Rini
Headers show
Series [v2,1/4] common: Only mark malloc initialized after mem_malloc_init | expand

Commit Message

Sean Anderson Aug. 8, 2023, 10:53 p.m. UTC 
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@seco.com>
---

Changes in v2:
- Fix cALLOc condition

 Kconfig           | 18 +++++++-----------
 common/dlmalloc.c |  9 +++++++--
 2 files changed, 14 insertions(+), 13 deletions(-)

Comments

Simon Glass Aug. 9, 2023, 2:03 a.m. UTC | #1 
On Tue, 8 Aug 2023 at 16:53, Sean Anderson <sean.anderson@seco.com> 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@seco.com>
> ---
>
> Changes in v2:
> - Fix cALLOc condition
>
>  Kconfig           | 18 +++++++-----------
>  common/dlmalloc.c |  9 +++++++--
>  2 files changed, 14 insertions(+), 13 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>
Tom Rini Aug. 10, 2023, 10:11 p.m. UTC | #2 
On Tue, Aug 08, 2023 at 06:53:19PM -0400, 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@seco.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

As-is, this leads to a failure to build on ls1012aqds_tfa_SECURE_BOOT
and a number of other platforms.  For v3, please throw this at CI,
either in Azure or GitLab via the clock tree, thanks.

In specifics, I've thrown this and the fls/ffs series at my lab and it
seems to cause mx6cuboxi to fail to boot (a Pi 3 in 32bit mode, and
am335x_evm are booting fine as are the Pi 3 in 64bit mode, libretech-cc,
pine64_plus, am65x_evm_a53, and j721e_evm_a72).  I did a size comparison
build again and the whole log is at:
https://gist.github.com/trini/2e88e94743821586ef742a936c45c389
as there's some oddities again (like SPL growing).
diff mbox series

Patch

diff --git a/Kconfig b/Kconfig
index 70efb41cc66..4b32286b69d 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 30c78ae976b..0985fd8de2a 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
 };
 
 #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_VAL(SYS_MALLOC_F_LEN) ||
+      (gd->flags & GD_FLG_FULL_MALLOC_INIT))
+    oldtopsize = chunksize(top);
 #endif
 #endif
   Void_t* mem = mALLOc (sz);