[U-Boot] Early malloc() summary

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

Hi Tomas,

On 08/15/2012 10:00 PM, Tomas Hlavacek wrote:
> On Tue, Aug 14, 2012 at 3:54 PM, Graeme Russ <graeme.russ@gmail.com> wrote:
> 
>>> dm_malloc you mean? I'm not happy about it, maybe Graeme can pour in some crazy
>>> juice in our direction again?
>>
>> I don't like the idea of dm_malloc() either, but it may be the only way to
>> get this past Wolfgang in the initial pass...
> 
> I agree, I am going to do it like that.

Progress :)

>>>> Yes, this is the main question: Should I hack malloc() function or
>>>> does it make sense to have both early_malloc() and malloc() exposed to
>>>> DM cores/drivers?
>>>
>>> This is indeed the main question -- ideas ?
>>>
>>>> The first is better from the point of view of drivers - when you ask
>>>> for memory, you get it. But you have to check yourself whether you
>>>> need to relocate your pointers or not, though we can provide
>>>> "relocation chain" you can register your relocation routine into to
>>>> facilitate it. The later makes sense because this makes it explicit
>>>> that whenever you use early_malloc() you are responsible for
>>>> relocating your data on your own (again, we can provide some facility
>>>> for ir).
>>
>> And there is the crux of it. Two failure scenarios:
>>
>>  1) Write a driver which uses malloc() and fail to implement a relocation
>>     helper - Driver blows up after relocation
>>
>>  2) Write a driver using malloc() which you never thought to use prior to
>>     relocation and it blows up because someone used it pre-relocation or
>>     in SPL and didn't convert it to use early_malloc()
>>
>> Neither can be picked up by at build time...
>>
>>>> There is a third path possible: We can provide early_malloc() and say
>>>> wrapped_malloc() which can be the third function "give me memory, I do
>>>> not care whether it is early or not". So drivers and/or DM can choose
>>>> to use malloc routines working in early-only, late-only or both.
>>
>> Third path is dm_malloc() - Although ugly, it has a few nicities...
>>
>>  1) It wraps malloc() and early_malloc() around a gd->flags & GD_FLG_RELOC
>>     test
>>  2) We can pass a pointer to a driver_core struct (or whatever struct it
>>     is that holds the 'reloc' helper function pointer). We can't pick up
>>     misuse at compile time, but dm_malloc() can print a meaningful message
>>     if it is called pre-relocation with no relocation function. (We should
>>     add a flag to indicate that no relocation helper is required which may
>>     be the case for very simple drivers)
> 
> Yes, but it would prevent using dm_malloc(size_t size, driver *drv)
> for one-time buffers inside helper functions - strdup() for instance,

Hmm, I hadn't thought of that

> inside drivers in early stage. In that case we need
> dm_malloc_nocheck(size_t size) or we need to pass a pointer to the
> driver structure to each and every function call in driver which might
> want to call dm_malloc. Both seems impractical to me.

maybe something along the lines of:

static void *pre_reloc_malloc(size_t bytes)
{
  ...do magic...
  return pointer to malloc'd memory
}

void *early_malloc(size_t bytes, int (*reloc_helper)(void *))
{
  if (reloc_helper) {
    /*
     * Maybe one day we will register reloc_helper (if not already
     * registered). But for now, driver core will manage that
     */
  }

   return pre_reloc_malloc(bytes)
}

void *dm_malloc(struct driver_core *drv, size_t bytes)
{
  if (gd->flags & GD_FLG_RELOC) {
    return malloc(bytes);
  } else {
    if (!drv) {
      debug("dm_malloc requires a driver pointer!!!");
      return NULL;
    }

    /*
     * DM core deals with driver reloc functions, but we check
     * anyway
     */
    if (!drv->reloc && !(drv->flags & NO_RELOC_FUNC))
      debug("Early malloc with no reloc function!!!!");

    /*
     * One day this might be:
     *   return early_malloc(bytes, drv->reloc);
     * and the early malloc infrastructure will call all the
     * relocation helpers. But for now, driver core will be...
     */
    return early_malloc(bytes, dm_core_reloc);
  }
}

>>  3) We can see right away when driver developers forget to use it
> 
> Yes. And I could add a debug check into malloc() to verify we have the
> flag GD_FLG_RELOC set and yell when it is not.


If you want to do this, do so in a separate patch so it can be (n)ack'd
separately

>> Let's leave it at that for the time being - my other thought of registering
>> early_malloc relocation helpers can wait until someone other than DM needs
>> to use early_malloc(). Until then, DM can deal with managing the calls to
>> the relocation functions.
> 
> I think so. We can connect the DM function into the relocation chain
> when it is needed.

Regards,

Graeme