Hi Tomas,
On 08/15/2012 10:00 PM, Tomas Hlavacek wrote:
> On Tue, Aug 14, 2012 at 3:54 PM, Graeme Russ <graeme.r...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
