public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Marek Vasut <marex@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [U-Boot-DM] [PATCH 1/1] early_malloc() introduced to ARM architecture
Date: Sun, 29 Jul 2012 08:30:18 +0200	[thread overview]
Message-ID: <201207290830.18824.marex@denx.de> (raw)
In-Reply-To: <CAEB7QLDEd+Nhcfci9ZS5456Sq=EzDc2HQ_RpWVNZn8vpC+iVYA@mail.gmail.com>

Dear Tomas Hlavacek,

> Hello Marek,
> 
> On Sat, Jul 28, 2012 at 10:36 PM, Marek Vasut <marex@denx.de> wrote:
> > I think we should still mark early patches as RFC.
> 
> Sure...
> 
> > > +#include <earlymalloc.h>
> > 
> > Do we need early_malloc.h at all? malloc.h won't cut it?
> 
> My intention was to keep the early_malloc in separate instance from
> real malloc.

early_malloc.c is probably fine ... once you manage to properly wrap the calls 
to early_malloc()/dlmalloc() into malloc() and switch based on the GD flags. 
It's just the header file that I consider redundant.

> Although this is proof-of-concept / RFC for early_malloc,
> it does not contain adaptation layer for switching
> malloc/early_malloc, because it seriously break things.

Why/how?

> I wanted
> comments on the basic idea first.
> 
> > +    early_malloc_init();
> > So this basically flips a bit, do it the other way and you don't need it.
> 
> Well yes. I was told to support more stages of early_mallocator
> deactivation during DM core relocation.

Ok, add more flags maybe? Can you elaborate on the reason?

> So this is why I have plenty
> of flags (currently using one).

Still, can you not wrap it into GD flags ?

> Now I know at least about two stages
> and three states. Actually intended lifecycle of the early_malloc is:
> 
> 1) init:

What's "init" ... you mean board_init_f() ?

> malloc() calls are switched to early_malloc()

Ok, but you must wrap this into malloc() call

> other calls (=free(), realloc()...) are switched to dummy.

realloc() can work too, why not ? So can calloc() etc.

> 2) relocation done

board_init_r() ?

> real malloc initialized, before core relocation:
> malloc() switched to dlmalloc
> other calls to dummy (this is special
> mode for core relocation, which prevents calling dlmaloc free() on
> space allocated by early_malloc().

I see ... ok, se please check if the GD flags can't be used, I think they can.

> 3) Full switch to dlmalloc.
> 
> Sure we can make it simpler, if it is OK with prospective core
> relocation function.
> 
> > No, we want this wrapped into malloc() call, so the drivers can be inited
> > indifferent of time.
> 
> Sure. My intention was not to hack current malloc but rather create
> adaptation layer. Maybe not a good idea... Anyway I can put simple
> switches like
> 
> if(!(gd->flags & GD_FLG_RELOC))
> 	return early_malloc();
> 
> directly into malloc() functions.

You can actually switch the mallocator so the functions exported by it are 
prefixed with dl ... and then just create some stuff that's called malloc() 
free() etc. and does the switching.

> > > +
> > > +typedef struct early_malloc_spc {
> > > +        char            flags;
> > > +        short           size;
> > > +        char            heap[EARLY_HEAP_SIZE];
> > > +} em_spc_t;
> > 
> > Drop the typedef please.
> 
> Actually I don't follow what is the problem here with the typedef?

http://lwn.net/Articles/2241/ ... but I'd like to gain opinions from other 
people on this one.

> > > diff --git a/lib/asm-offsets.c b/lib/asm-offsets.c
> > > index c88f5d4..6e39826 100644
> > > --- a/lib/asm-offsets.c
> > > +++ b/lib/asm-offsets.c
> > > @@ -23,7 +23,7 @@ int main(void)
> > > 
> > >  {
> > >  
> > >      /* Round up to make sure size gives nice stack alignment */
> > >      DEFINE(GENERATED_GBL_DATA_SIZE,
> > > 
> > > -        (sizeof(struct global_data) + 15) & ~15);
> > > +        (sizeof(struct global_data_extended) + 15) & ~15);
> > 
> > And if you adjusted the global data to be the "container of both, you
> > won't need
> > this here either.
> 
> Sure. My intention was to keep early_malloc heap in separate structure

Of course. My point was to do it the other way. But looking at it one more time, 
this seems correct.

> (containing the GD structure) in order not to relocate it but rather
> relocate cores by ourselves, which was our plan. It would be simpler
> to have it in the end of standard GD structure, because we can drop
> multi-stage early_malloc turn-off and perhaps we are not going to need
> to do relocation of DM structures by hand.
> 
> Tomas
> 
> 
> --
> Tom?? Hlav??ek <tmshlvck@gmail.com>

Best regards,
Marek Vasut

      reply	other threads:[~2012-07-29  6:30 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CAEB7QLAf3ha5wp50+2R7_vhMh7bEU8EtHx-QvoRz3fXy+AsJtQ@mail.gmail.com>
2012-07-28 20:36 ` [U-Boot] [U-Boot-DM] [PATCH 1/1] early_malloc() introduced to ARM architecture Marek Vasut
2012-07-29  6:15   ` Tomas Hlavacek
2012-07-29  6:30     ` Marek Vasut [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201207290830.18824.marex@denx.de \
    --to=marex@denx.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox