[U-Boot] [PATCHv4] [RFC] DM: early_malloc for DM added.

[Graeme Russ <graeme.russ@gmail.com>]

Hi Tomas

On Tue, Sep 18, 2012 at 5:13 PM, Tomas Hlavacek <tmshlvck@gmail.com> wrote:
> early_malloc for DM with support for more heaps and lightweight
> first heap on stack.

Technically, you are not putting the first heap on the stack - you are
sacrificing some early stack space to create the early heap

>
> Adaptation layer for seamless calling of early_malloc or dlmalloc from
> DM based on init stage added (dmmalloc() and related functions).
>
> Signed-off-by: Tomas Hlavacek <tmshlvck@gmail.com>
> ---
>  arch/arm/include/asm/config.h             |    3 +
>  arch/arm/include/asm/global_data.h        |    1 +
>  arch/arm/lib/board.c                      |    7 ++
>  arch/avr32/include/asm/global_data.h      |    1 +
>  arch/avr32/lib/board.c                    |    6 ++
>  arch/blackfin/include/asm/global_data.h   |    1 +
>  arch/blackfin/lib/board.c                 |    7 ++
>  arch/m68k/include/asm/global_data.h       |    1 +
>  arch/m68k/lib/board.c                     |    7 ++
>  arch/microblaze/include/asm/global_data.h |    1 +
>  arch/microblaze/lib/board.c               |    8 ++
>  arch/mips/include/asm/global_data.h       |    1 +
>  arch/mips/lib/board.c                     |    7 ++
>  arch/nds32/include/asm/global_data.h      |    1 +
>  arch/nds32/lib/board.c                    |    6 ++
>  arch/nios2/include/asm/global_data.h      |    1 +
>  arch/nios2/lib/board.c                    |    6 ++
>  arch/openrisc/include/asm/global_data.h   |    1 +
>  arch/openrisc/lib/board.c                 |    6 ++
>  arch/powerpc/include/asm/global_data.h    |    1 +
>  arch/powerpc/lib/board.c                  |    6 ++
>  arch/sandbox/include/asm/global_data.h    |    1 +
>  arch/sandbox/lib/board.c                  |    6 ++
>  arch/sh/include/asm/global_data.h         |    1 +
>  arch/sh/lib/board.c                       |    6 ++
>  arch/sparc/include/asm/global_data.h      |    1 +
>  arch/sparc/lib/board.c                    |    6 ++
>  arch/x86/include/asm/global_data.h        |    1 +
>  arch/x86/lib/board.c                      |   14 ++++
>  common/Makefile                           |    1 +
>  common/dmmalloc.c                         |  128 +++++++++++++++++++++++++++++
>  include/configs/zipitz2.h                 |   12 ++-

WTF! - Move this out into another patch

>  include/dmmalloc.h                        |   51 ++++++++++++
>  33 files changed, 306 insertions(+), 1 deletion(-)
>  create mode 100644 common/dmmalloc.c
>  create mode 100644 include/dmmalloc.h
>
> diff --git a/arch/arm/include/asm/config.h b/arch/arm/include/asm/config.h
> index c60dba2..8e2f67b 100644
> --- a/arch/arm/include/asm/config.h
> +++ b/arch/arm/include/asm/config.h
> @@ -23,4 +23,7 @@
>
>  #define CONFIG_LMB
>  #define CONFIG_SYS_BOOT_RAMDISK_HIGH
> +
> +#define CONFIG_SYS_EARLY_MALLOC
>  #endif
> +

Why are you adding this define to ARM and nothing else? Shouldn't it be an
all-in or none-in proposition?

(and why the stray eol white-space?)

> diff --git a/arch/arm/include/asm/global_data.h b/arch/arm/include/asm/global_data.h
> index c3ff789..8563d49 100644
> --- a/arch/arm/include/asm/global_data.h
> +++ b/arch/arm/include/asm/global_data.h
> @@ -84,6 +84,7 @@ typedef       struct  global_data {
>         unsigned long   post_log_res; /* success of POST test */
>         unsigned long   post_init_f_time; /* When post_init_f started */
>  #endif
> +       void            *early_heap_first; /* early heap for early_malloc */
>  } gd_t;

early_heap_first is only used if CONFIG_SYS_EARLY_MALLOC is defined, so
wrap a #ifdef around it. This will also help to detect unintended usage
if CONFIG_SYS_EARLY_MALLOC is not defined.

>
>  /*
> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
> index 500e216..33e74da 100644
> --- a/arch/arm/lib/board.c
> +++ b/arch/arm/lib/board.c
> @@ -52,6 +52,7 @@
>  #include <fdtdec.h>
>  #include <post.h>
>  #include <logbuff.h>
> +#include <dmmalloc.h>
>
>  #ifdef CONFIG_BITBANGMII
>  #include <miiphy.h>
> @@ -273,6 +274,12 @@ void board_init_f(ulong bootflag)
>
>         memset((void *)gd, 0, sizeof(gd_t));
>
> +
> +#ifdef CONFIG_SYS_EARLY_MALLOC
> +       /* Initialize early_malloc */
> +       gd->early_heap_first = early_brk(CONFIG_SYS_EARLY_HEAP_SIZE);
> +#endif /* CONFIG_SYS_EARLY_MALLOC */
> +

Did you checkpatch? You've added an additional blank line (for a total of
two)

[snip]

> diff --git a/arch/x86/lib/board.c b/arch/x86/lib/board.c
> index 5f0b62c..b609dbe 100644
> --- a/arch/x86/lib/board.c
> +++ b/arch/x86/lib/board.c
> @@ -40,6 +40,8 @@
>  #include <asm/init_helpers.h>
>  #include <asm/init_wrappers.h>
>
> +#include <dmmalloc.h>
> +
>  /*
>   * Breath some life into the board...
>   *
> @@ -85,6 +87,17 @@
>  typedef int (init_fnc_t) (void);
>
>  /*
> + * Initialize early heap (when enabled by config).
> + */
> +static void early_malloc_init(void)
> +{
> +#ifdef CONFIG_SYS_EARLY_MALLOC
> +       /* Initialize early_malloc */
> +       gd->early_heap_first = early_brk(CONFIG_SYS_EARLY_HEAP_SIZE);
> +#endif /* CONFIG_SYS_EARLY_MALLOC */
> +}
> +
> +/*
>   * init_sequence_f is the list of init functions which are run when U-Boot
>   * is executing from Flash with a limited 'C' environment. The following
>   * limitations must be considered when implementing an '_f' function:
> @@ -99,6 +112,7 @@ init_fnc_t *init_sequence_f[] = {
>         cpu_init_f,
>         board_early_init_f,
>         env_init,
> +       early_malloc_init,

Put the #ifdef here - gc_sections will clean up the unused function

>         init_baudrate_f,
>         serial_init,
>         console_init_f,
> diff --git a/common/Makefile b/common/Makefile
> index 2a31c62..dfea4e8 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -188,6 +188,7 @@ COBJS-y += console.o
>  COBJS-y += dlmalloc.o
>  COBJS-y += memsize.o
>  COBJS-y += stdio.o
> +COBJS-y += dmmalloc.o

COBJS-$(CONFIG_SYS_EARLY_MALLOC)

or

COBJS-$(CONFIG_SYS_DM)

If DM mandates early_malloc (which I think it will) then we need to tie the
#defines together somehow

>  COBJS  := $(sort $(COBJS-y))
> diff --git a/common/dmmalloc.c b/common/dmmalloc.c
> new file mode 100644
> index 0000000..18f2d95
> --- /dev/null
> +++ b/common/dmmalloc.c
> @@ -0,0 +1,128 @@
> +/*
> + * (C) Copyright 2012
> + * Tomas Hlavacek (tmshlvck at gmail.com)
> + *
> + * See file CREDITS for list of people who contributed to this
> + * project.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
> + */
> +
> +#include <common.h> /* for ROUND_UP */
> +#include <asm/u-boot.h>
> +#include <asm/global_data.h> /* for gd_t and gd */
> +#include <asm/types.h> /* for phys_addr_t and size_addt_t */

Drop the comments - they are ugly

> +
> +#include <dmmalloc.h>
> +#include <malloc.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +#ifdef CONFIG_SYS_EARLY_MALLOC
> +static struct early_heap_header *def_early_brk(size_t size)
> +{
> +       struct early_heap_header *h =
> +               (struct early_heap_header *)CONFIG_SYS_EARLY_HEAP_ADDR;
> +
> +       h->free_space_pointer = (void *)(roundup(
> +                               (phys_addr_t)CONFIG_SYS_EARLY_HEAP_ADDR +
> +                               sizeof(struct early_heap_header),
> +                               sizeof(phys_addr_t)));
> +       h->free_bytes = size - roundup(sizeof(struct early_heap_header),
> +                               sizeof(phys_addr_t));
> +       h->next_early_heap = NULL;
> +
> +       return h;
> +}
> +
> +struct early_heap_header *early_brk(size_t size)
> +       __attribute__((weak, alias("def_early_brk")));
> +
> +
> +void *early_malloc(size_t size)
> +{
> +       phys_addr_t addr;
> +       struct early_heap_header *h;
> +
> +       /* Align size. */
> +       size = roundup(size, sizeof(phys_addr_t));
> +
> +       /* Choose early_heap with enough space. */
> +       h = gd->early_heap_first;
> +       while ((h->free_bytes < size) && (h->next_early_heap != NULL))
> +               h = h->next_early_heap;
> +
> +       if (h->free_bytes < size) {

You will get a NULL-pointer exception if no early heap has enough space

You also need to add some logic to allocate an additional early heap by
calling early_brk. If that call fails (returns NULL) then bail out

> +               debug("Early heap overflow. Heap %p, free %d, required %d.",
> +                       h, h->free_bytes, size);
> +               return NULL;
> +       }
> +
> +       /* Choose block beginning address and mark next free space. */
> +       addr = (phys_addr_t)h->free_space_pointer;
> +
> +       h->free_space_pointer += size;
> +       h->free_bytes -= size;
> +
> +       return (void *)addr;
> +}
> +
> +static int is_early_malloc_active(void)
> +{
> +       if (gd->flags & GD_FLG_RELOC)
> +               return 0;
> +
> +       return 1;

Hmmm, return ((gd->flags & GD_FLG_RELOC) == GD_FLG_RELOC); ?

I also prefer the semantics of not having 'is_'. "if blah" reads better
to me than "if is blah" - YMMV

> +}
> +
> +#endif /* CONFIG_SYS_EARLY_MALLOC */
> +
> +void *dmmalloc(size_t size)
> +{
> +#ifdef CONFIG_SYS_EARLY_MALLOC
> +       if (is_early_malloc_active())
> +               return early_malloc(size);
> +#endif /* CONFIG_SYS_EARLY_MALLOC */
> +       return malloc(size);
> +}

The argument over dmmalloc() versus transparent malloc() is far from over
IMNSHO, but for now, this will have to do :)

> +
> +void dmfree(void *ptr)
> +{
> +#ifdef CONFIG_SYS_EARLY_MALLOC
> +       if (is_early_malloc_active())
> +               return;
> +#endif /* CONFIG_SYS_EARLY_MALLOC */
> +       free(ptr);
> +}
> +
> +void *dmcalloc(size_t n, size_t elem_size)
> +{
> +#ifdef CONFIG_SYS_EARLY_MALLOC
> +       if (is_early_malloc_active())
> +               return NULL;
> +#endif /* CONFIG_SYS_EARLY_MALLOC */
> +       return calloc(n, elem_size);
> +}
> +
> +void *dmrealloc(void *oldmem, size_t bytes)
> +{
> +#ifdef CONFIG_SYS_EARLY_MALLOC
> +       if (is_early_malloc_active())
> +               return NULL;
> +#endif /* CONFIG_SYS_EARLY_MALLOC */
> +       return dmrealloc(oldmem, bytes);
> +}
> +

[snip]

Regards,

Graeme