On 04/05/2013 06:17 AM, Wolfgang Denk wrote:
Dear Tom Rini,
In message 20130403153014.GF7035@bill-the-cat you wrote:
On Fri, Mar 22, 2013 at 11:26:21AM -0000, Rob Herring wrote:
From: Rob Herring rob.herring@calxeda.com
Most of the various environment functions create CONFIG_ENV_SIZE buffers on the stack. At least on ARM and PPC which have 4KB stacks, this can overflow the stack if we have large environment sizes. So move all the buffers off the stack to static buffers.
Signed-off-by: Rob Herring rob.herring@calxeda.com
Applied to u-boot/master, thanks!
I'm unhappy about this.
The patch makes no sense to me, and in addition it causes build warnings for some boards (PPC: debris, kvme080):
env_nvram.c: In function 'env_init': env_nvram.c:124:16: warning: pointer targets in initialization differ in signedness [-Wpointer-sign]
I tried to explain this before, but eventually you missed my remarks, so here they go again:
The functiuons we are talking about run after relocation, i. e. when we have a full standard C runtime environment. In this situation we can assume to have virtually unlimited stack space available - actually limited only by the RAM size.
Moving the buffers from the stack into BSS is bad, as this way we permanently lose the space for these buffers, nut we need them only for a very short time, so we are wasting lots of memory.
If some board have for some reasons unreasonable small stack sizes, these need to be fixed. Rob refers to LMB stack space in his previous message - if there are indeed such small stack sizes used there, this is a problem that needs to be fixed elsewhere, but NOT by adapting all the rest of the U-Boot code to an artifical small stack.
The stack size limit only comes into play when bootm runs and starts moving initrd and dtb to high addresses below the stack. At that point, the stack size does become limited because only 4KB (recently increase from 1KB) of space is reserved. So I had this in mind when I started debugging my environment getting corrupted and saw large buffers on the stack. My problem was ultimately that blank lines in mvenvimage input files are not handled correctly giving intermittent issues with the env import. I'm still not clear why the issue was intermittent, but I think mkenvimage should skip/remove blank lines. I still need to make a fix for that.
I hereby request to revert that commit.
That's fine with me.
Rob
Best regards,
Wolfgang Denk