Hi Tomas
On Tue, Sep 18, 2012 at 5:13 PM, Tomas Hlavacek <tmshl...@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 <tmshl...@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 (tmshl...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
