[PATCH] dlmalloc: Add an option to default malloc to init

[PATCH] dlmalloc: Add an option to default malloc to init

[marek.bykowski at gmail.com]

From: Marek Bykowski <marek.bykowski@gmail.com>

If a system wants the malloc to get moved around from one to another
memory range it should call mem_malloc_init() with the updated memory
ranges. However setting aside the new memory alone isn't enough.
It should also bring the bins and static bookkeeping data to init.

One of the use cases could be SPL U-Boot that allocates first to
the (limited) static memory and then when needs more memory, and
the DDR memory becomes available, moves the pools around.

Note that defaulting the malloc to init discards all the memory
previously allocated.

Signed-off-by: Marek Bykowski <marek.bykowski@gmail.com>
---
 Kconfig           | 14 ++++++++++++++
 common/dlmalloc.c | 41 ++++++++++++++++++++++++++++++++++++++---
 2 files changed, 52 insertions(+), 3 deletions(-)

diff --git a/Kconfig b/Kconfig
index 1f0904f704..958553c90a 100644
--- a/Kconfig
+++ b/Kconfig
@@ -209,6 +209,20 @@ 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"
+	default n
+	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.
+
 config TOOLS_DEBUG
 	bool "Enable debug information for tools"
 	help
diff --git a/common/dlmalloc.c b/common/dlmalloc.c
index 6f12a18d54..8b2796e280 100644
--- a/common/dlmalloc.c
+++ b/common/dlmalloc.c
@@ -280,6 +280,7 @@ nextchunk-> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
 	    |             Unused space (may be 0 bytes long)                .
 	    .                                                               .
 	    .                                                               |
+
 nextchunk-> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
     `foot:' |             Size of chunk, in bytes                           |
 	    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
@@ -574,6 +575,10 @@ static void malloc_bin_reloc(void)
 static inline void malloc_bin_reloc(void) {}
 #endif
 
+#ifdef CONFIG_SYS_MALLOC_DEFAULT_TO_INIT
+static void malloc_init(void);
+#endif
+
 ulong mem_malloc_start = 0;
 ulong mem_malloc_end = 0;
 ulong mem_malloc_brk = 0;
@@ -604,6 +609,10 @@ void mem_malloc_init(ulong start, ulong size)
 	mem_malloc_end = start + size;
 	mem_malloc_brk = start;
 
+#ifdef CONFIG_SYS_MALLOC_DEFAULT_TO_INIT
+	malloc_init();
+#endif
+
 	debug("using memory %#lx-%#lx for malloc()\n", mem_malloc_start,
 	      mem_malloc_end);
 #ifdef CONFIG_SYS_MALLOC_CLEAR_ON_INIT
@@ -708,7 +717,36 @@ static unsigned int max_n_mmaps = 0;
 static unsigned long max_mmapped_mem = 0;
 #endif
 
+#ifdef CONFIG_SYS_MALLOC_DEFAULT_TO_INIT
+static void malloc_init(void)
+{
+	int i, j;
+
+	debug("bins (av_ array) are at %p\n", (void *)av_);
+
+	av_[0] = NULL; av_[1] = NULL;
+	for (i = 2, j = 2; i < NAV * 2 + 2; i += 2, j++) {
+		av_[i] = bin_at(j - 2);
+		av_[i + 1] = bin_at(j - 2);
+
+		/* Just print the first few bins so that
+		 * we can see there are alright.
+		 */
+		if (i < 10)
+			debug("av_[%d]=%lx av_[%d]=%lx\n",
+			      i, (ulong)av_[i],
+			      i + 1, (ulong)av_[i + 1]);
+	}
 
+	/* Init the static bookkeeping as well */
+	sbrk_base = (char *)(-1);
+	max_sbrked_mem = 0;
+	max_total_mem = 0;
+#ifdef DEBUG
+	memset((void *)&current_mallinfo, 0, sizeof(struct mallinfo));
+#endif
+}
+#endif
 
 /*
   Debugging support
@@ -1051,9 +1089,6 @@ static mchunkptr mremap_chunk(p, new_size) mchunkptr p; size_t new_size;
 
 #endif /* HAVE_MMAP */
 
-
-
-
 /*
   Extend the top-most chunk by obtaining memory from system.
   Main interface to sbrk (but see also malloc_trim).
-- 
2.21.0.896.g6a6c0f1

[PATCH] dlmalloc: Add an option to default malloc to init

[Tom Rini]

On Wed, Mar 18, 2020 at 01:14:43PM +0100, marek.bykowski at gmail.com wrote:

> From: Marek Bykowski <marek.bykowski@gmail.com>
> 
> If a system wants the malloc to get moved around from one to another
> memory range it should call mem_malloc_init() with the updated memory
> ranges. However setting aside the new memory alone isn't enough.
> It should also bring the bins and static bookkeeping data to init.
> 
> One of the use cases could be SPL U-Boot that allocates first to
> the (limited) static memory and then when needs more memory, and
> the DDR memory becomes available, moves the pools around.
> 
> Note that defaulting the malloc to init discards all the memory
> previously allocated.
> 
> Signed-off-by: Marek Bykowski <marek.bykowski@gmail.com>
> ---
>  Kconfig           | 14 ++++++++++++++
>  common/dlmalloc.c | 41 ++++++++++++++++++++++++++++++++++++++---
>  2 files changed, 52 insertions(+), 3 deletions(-)
> 
> diff --git a/Kconfig b/Kconfig
> index 1f0904f704..958553c90a 100644
> --- a/Kconfig
> +++ b/Kconfig
> @@ -209,6 +209,20 @@ 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"
> +	default n
> +	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.
> +
>  config TOOLS_DEBUG
>  	bool "Enable debug information for tools"
>  	help
> diff --git a/common/dlmalloc.c b/common/dlmalloc.c
> index 6f12a18d54..8b2796e280 100644
> --- a/common/dlmalloc.c
> +++ b/common/dlmalloc.c
> @@ -280,6 +280,7 @@ nextchunk-> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>  	    |             Unused space (may be 0 bytes long)                .
>  	    .                                                               .
>  	    .                                                               |
> +
>  nextchunk-> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>      `foot:' |             Size of chunk, in bytes                           |
>  	    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> @@ -574,6 +575,10 @@ static void malloc_bin_reloc(void)
>  static inline void malloc_bin_reloc(void) {}
>  #endif
>  
> +#ifdef CONFIG_SYS_MALLOC_DEFAULT_TO_INIT
> +static void malloc_init(void);
> +#endif
> +
>  ulong mem_malloc_start = 0;
>  ulong mem_malloc_end = 0;
>  ulong mem_malloc_brk = 0;
> @@ -604,6 +609,10 @@ void mem_malloc_init(ulong start, ulong size)
>  	mem_malloc_end = start + size;
>  	mem_malloc_brk = start;
>  
> +#ifdef CONFIG_SYS_MALLOC_DEFAULT_TO_INIT
> +	malloc_init();
> +#endif
> +
>  	debug("using memory %#lx-%#lx for malloc()\n", mem_malloc_start,
>  	      mem_malloc_end);
>  #ifdef CONFIG_SYS_MALLOC_CLEAR_ON_INIT
> @@ -708,7 +717,36 @@ static unsigned int max_n_mmaps = 0;
>  static unsigned long max_mmapped_mem = 0;
>  #endif
>  
> +#ifdef CONFIG_SYS_MALLOC_DEFAULT_TO_INIT
> +static void malloc_init(void)
> +{
> +	int i, j;
> +
> +	debug("bins (av_ array) are at %p\n", (void *)av_);
> +
> +	av_[0] = NULL; av_[1] = NULL;
> +	for (i = 2, j = 2; i < NAV * 2 + 2; i += 2, j++) {
> +		av_[i] = bin_at(j - 2);
> +		av_[i + 1] = bin_at(j - 2);
> +
> +		/* Just print the first few bins so that
> +		 * we can see there are alright.
> +		 */
> +		if (i < 10)
> +			debug("av_[%d]=%lx av_[%d]=%lx\n",
> +			      i, (ulong)av_[i],
> +			      i + 1, (ulong)av_[i + 1]);
> +	}
>  
> +	/* Init the static bookkeeping as well */
> +	sbrk_base = (char *)(-1);
> +	max_sbrked_mem = 0;
> +	max_total_mem = 0;
> +#ifdef DEBUG
> +	memset((void *)&current_mallinfo, 0, sizeof(struct mallinfo));
> +#endif
> +}
> +#endif
>  
>  /*
>    Debugging support

Can you please showcase using this feature somewhere?  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200320/30db8978/attachment.sig>

[PATCH] dlmalloc: Add an option to default malloc to init

[Marek]

>
> Can you please showcase using this feature somewhere?  Thanks!
>

Yes. First of all, sincere apology for such a massive delay in it.
I got snowed with the corporation work of mine.

So here is my go at showcasing it. We are in SPL U-Boot running code from
within the static memory:
arch/arm/cpu/armv8/start.S:reset vector
-> arch/arm/cpu/armv8/start.S:main()
-> arch/arm/lib/crt0_64.S:board_init_f()
-> /board/<my_board>/common/spl.c:board_init_f()

board_init_f() does mem_malloc_init(malloc_start_in_static, size). It needs
the malloc for SPI flash to load off some proprietary stuff.
Note the size of the static memory is limited, so is our malloc size.

Then along it does the DDR memory initialization. Now we have much greater memory
and can move our malloc allocator onto it. So again it does mem_malloc_init()
but this time with an address in the DDR memory and a much greater size.
And this is where this feature comes in hand. Just calling in mem_malloc_init()
with the updated start addr and the size isn't enough. It also mandates
resetting the malloc, namely defaulting its bins, and some bookkeeping.

I know there is an early malloc available I can use as the first one but that
requires managing the two mallocs. This patch proposes to use a single malloc
suite (dlmalloc) and use it freely around when and how it is needed.

Marek

[PATCH] dlmalloc: Add an option to default malloc to init

[Marek]

The previous message of mine went out of thread. It should be in thread now.

On Tue, Apr 21, 2020 at 02:21:36AM +0200, Marek wrote:
> >
> > Can you please showcase using this feature somewhere?  Thanks!
> >
> 
> Yes. First of all, sincere apology for such a massive delay in it.
> I got snowed with the corporation work of mine.
> 
> So here is my go at showcasing it. We are in SPL U-Boot running code from
> within the static memory:
> arch/arm/cpu/armv8/start.S:reset vector
> -> arch/arm/cpu/armv8/start.S:main()
> -> arch/arm/lib/crt0_64.S:board_init_f()
> -> /board/<my_board>/common/spl.c:board_init_f()
> 
> board_init_f() does mem_malloc_init(malloc_start_in_static, size). It needs
> the malloc for SPI flash to load off some proprietary stuff.
> Note the size of the static memory is limited, so is our malloc size.
> 
> Then along it does the DDR memory initialization. Now we have much greater memory
> and can move our malloc allocator onto it. So again it does mem_malloc_init()
> but this time with an address in the DDR memory and a much greater size.
> And this is where this feature comes in hand. Just calling in mem_malloc_init()
> with the updated start addr and the size isn't enough. It also mandates
> resetting the malloc, namely defaulting its bins, and some bookkeeping.
> 
> I know there is an early malloc available I can use as the first one but that
> requires managing the two mallocs. This patch proposes to use a single malloc
> suite (dlmalloc) and use it freely around when and how it is needed.
> 
> Marek

[PATCH] dlmalloc: Add an option to default malloc to init

[Marek]

>
> Can you please showcase using this feature somewhere?  Thanks!
>
> --
> Tom

Yes. First of all, sincere apology for such a massive delay in it.
I got snowed with the corporation work of mine.

So here is my go at showcasing it. We are in SPL U-Boot running code from
within the static memory:
arch/arm/cpu/armv8/start.S:reset vector
-> arch/arm/cpu/armv8/start.S:main()
-> arch/arm/lib/crt0_64.S:board_init_f()
-> /board/<my_board>/common/spl.c:board_init_f()

board_init_f() does mem_malloc_init(malloc_start_in_static, size). It needs
the malloc for SPI flash to load off some proprietary stuff.
Note the size of the static memory is limited, so is our malloc size.

Then along it does the DDR memory initialization. Now we have much greater memory
and can move our malloc allocator onto it. So again it does mem_malloc_init()
but this time with an address in the DDR memory and a much greater size.
And this is where this feature comes in hand. Just calling in mem_malloc_init()
with the updated start addr and the size isn't enough. It also mandates
resetting the malloc, namely defaulting its bins, and some bookkeeping.

I know there is an early malloc available I can use as the first one but that
requires managing the two mallocs. This patch proposes to use a single malloc
suite (dlmalloc) and use it freely around when and how it is needed.

Marek

[PATCH] dlmalloc: Add an option to default malloc to init

[Tom Rini]

On Tue, Apr 21, 2020 at 01:00:41PM +0200, Marek wrote:
> >
> > Can you please showcase using this feature somewhere?  Thanks!
> >
> > --
> > Tom
> 
> Yes. First of all, sincere apology for such a massive delay in it.
> I got snowed with the corporation work of mine.
> 
> So here is my go at showcasing it. We are in SPL U-Boot running code from
> within the static memory:
> arch/arm/cpu/armv8/start.S:reset vector
> -> arch/arm/cpu/armv8/start.S:main()
> -> arch/arm/lib/crt0_64.S:board_init_f()
> -> /board/<my_board>/common/spl.c:board_init_f()
> 
> board_init_f() does mem_malloc_init(malloc_start_in_static, size). It needs
> the malloc for SPI flash to load off some proprietary stuff.
> Note the size of the static memory is limited, so is our malloc size.
> 
> Then along it does the DDR memory initialization. Now we have much greater memory
> and can move our malloc allocator onto it. So again it does mem_malloc_init()
> but this time with an address in the DDR memory and a much greater size.
> And this is where this feature comes in hand. Just calling in mem_malloc_init()
> with the updated start addr and the size isn't enough. It also mandates
> resetting the malloc, namely defaulting its bins, and some bookkeeping.
> 
> I know there is an early malloc available I can use as the first one but that
> requires managing the two mallocs. This patch proposes to use a single malloc
> suite (dlmalloc) and use it freely around when and how it is needed.

OK, thanks.  Can you please re-send and include the above re-phrased to
make sense in a commit message, in the body of the commit as well so
it's documented more in-tree when this would be helpful?  Thanks again!

> 
> Marek

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200424/9d00aa93/attachment.sig>

[PATCH] dlmalloc: Add an option to default malloc to init

[Marek Bykowski]

> 
> OK, thanks.  Can you please re-send and include the above re-phrased to
> make sense in a commit message, in the body of the commit as well so
> it's documented more in-tree when this would be helpful?  Thanks again!
> 
> -- 
> Tom

Sure Tom. Will do that Monday/Tuesday.

Thanks,
Marek