[U-Boot] [U-BOOT] [PATCH] env: reduce the stack footprint for the env buf

[Lei Wen]

On Mon, Feb 7, 2011 at 4:59 PM, Wolfgang Denk <wd at denx.de> wrote:
> Dear Lei Wen,
>
> In message <AANLkTi=KNpJ02SHCp5FYqukYdMAXUN+LgVpK7O-6Veex at mail.gmail.com> you wrote:
>>
>> The measurement is simple, for I don't change other part, the heap
>> size and etc part
>> size, but only move the temp buffer to the heap area instead of stack area.
>> So this reduce the large memory need in the env_relocate_spec() function.
>> So that the stack would not touch those memory since most function
>> don't cost so much
>> stack as env_relocate_spec() does...
>
> You don't provide any real measurement results here.  Also, you did
> not report what happens when you - for example - reduce the size of
> your malloc arena by the size...
>
>> The heap area is already enough for the env_relocate_spec() in our
>> platform. Maybe
>> you concern is for those platform don't allocate enough heap size for
>> the env?...
>> Also fragmentation problem I am not figure out clearly... Temporarily
>> use large heap then free
>> would make uboot unable to malloc enough memory in latter allocate?...
>
> To sum it up:
>
> - Your patch increases the code size, and the code complexity, and
>  increases the risk of memory leaks.
>
> - Your patch requires on many systems to increase the size of the
>  malloc arena, with the result that the available memory at runtim
>  becomes lower.
>
> - Your patch fails to adjust the malloc arena size on the affected
>  boards (and eventually any board may be affected, so all will have
>  to be checked and verified).
>
> - You claim that this patch helps to use less memory on your system,
>  but you fail to prove this with actual numbers.
>
>
> So we have a number of disadvantages for all users, a lot of effort
> for all users (they all need to re-test their boards), and a claimed
> but otherwise unproven advantage for a simgle user (you).
>
> I hereby reject your patch.  If you really think it, you can still
> maintain it as a local, out-of-tree patch.

Understand...
It is true for this patch that don't consider for other platform...

Thanks,
Lei