On 10/1/23 21:16, Simon Glass wrote:
Hi Sean,

On Thu, 28 Sept 2023 at 08:45, Sean Anderson <sean.ander...@seco.com> wrote:

In my efforts to get SPL to fit into flash after some changes I made, I
noticed that av_ is one of the largest variables in SPL. As it turns
out, we can generate it at runtime, and the code is already there. This
has the potential to save 1-2k across the board, for some (very) minor
boot time increase.

This series is based on [1], since this makes checking for SYS_MALLOC_F
easier. Passing CI at [2].

To measure the boot time difference, I applied the following patch:

---
  common/board_r.c | 5 +++++
  common/spl/spl.c | 4 ++++
  2 files changed, 9 insertions(+)

diff --git a/common/board_r.c b/common/board_r.c
index 58a5986aa54..ca624b20d46 100644
--- a/common/board_r.c
+++ b/common/board_r.c
@@ -194,6 +194,7 @@ static int initr_barrier(void)
         return 0;
  }

+static ulong malloc_begin, malloc_end;
  static int initr_malloc(void)
  {
         ulong malloc_start;
@@ -208,8 +209,10 @@ static int initr_malloc(void)
          * reserve_noncached().
          */
         malloc_start = gd->relocaddr - TOTAL_MALLOC_LEN;
+       malloc_begin = timer_get_boot_us();

Perhaps this would be better done with bootstage, since then the
timing can be enabled / disabled, and reported along with other
timings.

I'll try that out next time.

         mem_malloc_init((ulong)map_sysmem(malloc_start, TOTAL_MALLOC_LEN),
                         TOTAL_MALLOC_LEN);
+       malloc_end = timer_get_boot_us();
         gd->flags |= GD_FLG_FULL_MALLOC_INIT;
         return 0;
  }
@@ -570,6 +573,8 @@ static int dm_announce(void)

  static int run_main_loop(void)
  {
+       printf("malloc_init took %luus (%lu %lu)\n", malloc_end - malloc_begin,
+              malloc_begin, malloc_end);
  #ifdef CONFIG_SANDBOX
         sandbox_main_loop_init();
  #endif
diff --git a/common/spl/spl.c b/common/spl/spl.c
index d74acec10b5..b34d1f4b4e6 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -755,7 +755,9 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
         spl_set_bd();

  #if defined(CONFIG_SYS_SPL_MALLOC)
+       ulong malloc_begin = timer_get_boot_us();
         mem_malloc_init(SYS_SPL_MALLOC_START, CONFIG_SYS_SPL_MALLOC_SIZE);
+       ulong malloc_end = timer_get_boot_us();
         gd->flags |= GD_FLG_FULL_MALLOC_INIT;
  #endif
         if (!(gd->flags & GD_FLG_SPL_INIT)) {
@@ -817,6 +819,8 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
         spl_image.boot_device = BOOT_DEVICE_NONE;
         board_boot_order(spl_boot_list);

+       printf("malloc_init took %luus (%lu %lu)\n", malloc_end - malloc_begin,
+              malloc_begin, malloc_end);

debug() ?

Well, this is not going to be applied, so I used the easiest thing :)

--Sean

         ret = boot_from_devices(&spl_image, spl_boot_list,
                                 ARRAY_SIZE(spl_boot_list));
         if (ret) {
--
2.25.1

I found that MALLOC_CLEAR_ON_INIT dominated the mem_malloc_init time
(taking around 150 ms in SPL on my board). After disabling it, I found
that MALLOC_RUNTIME_INIT took around 5 us on average.

[1] https://lore.kernel.org/u-boot/20230926141514.2101787-1-...@chromium.org/
[2] https://source.denx.de/u-boot/custodians/u-boot-clk/-/pipelines/17900

Changes in v3:
- Use CONFIG_IS_ENABLED in conditionals
- Don't enable SPL_SYS_MALLOC_RUNTIME_INIT if we are short on BSS

Changes in v2:
- Only mark malloc initialized after mem_malloc_init
- Fix cALLOc condition

Sean Anderson (4):
   common: Only mark malloc initialized after mem_malloc_init
   malloc: Don't use ifdefs for SYS_MALLOC_DEFAULT_TO_INIT
   malloc: Don't statically initialize av_ if using malloc_init
   malloc: Enable SYS_MALLOC_RUNTIME_INIT by default in SPL

  Kconfig           | 27 +++++++++++++++++----------
  common/board_r.c  |  3 ++-
  common/dlmalloc.c | 16 ++++++++--------
  3 files changed, 27 insertions(+), 19 deletions(-)

--
2.35.1.1320.gc452695387.dirty


REgards,
SImon

Reply via email to