| Submitter | Tom Rini |
|---|---|
| Date | Sept. 26, 2013, 8:27 p.m. |
| Message ID | <1380227287-26057-3-git-send-email-trini@ti.com> |
| Download | mbox | patch |
| Permalink | /patch/278258/ |
| State | New |
| Delegated to: | Tom Rini |
| Headers | show |
Comments
Dear Tom Rini, In message <1380227287-26057-3-git-send-email-trini@ti.com> you wrote: > Switch the case of non-redundant non-embedded environment to use malloc > to allocate buffers, rather than place them on the stack, like the > redundant case does. What exactly would be the benefit of this change? It just adds code size and execution time and makes the code more complex, without any appearent advanteages? Best regards, Wolfgang Denk
On Sat, Oct 05, 2013 at 10:00:13PM +0200, Wolfgang Denk wrote: > Dear Tom Rini, > > In message <1380227287-26057-3-git-send-email-trini@ti.com> you wrote: > > Switch the case of non-redundant non-embedded environment to use malloc > > to allocate buffers, rather than place them on the stack, like the > > redundant case does. > > What exactly would be the benefit of this change? It just adds code > size and execution time and makes the code more complex, without any > appearent advanteages? The main advantage is that we can use this code in environments with less than CONFIG_ENV_SIZE worth of stack available. Arguably it makes the behaviour and code paths similar for redundant and non-redundant cases (but someone posted a patch to make the redundant case use the stack).
Dear Tom, In message <20131006204046.GN15917@bill-the-cat> you wrote: > > > In message <1380227287-26057-3-git-send-email-trini@ti.com> you wrote: > > > Switch the case of non-redundant non-embedded environment to use malloc > > > to allocate buffers, rather than place them on the stack, like the > > > redundant case does. > > > > What exactly would be the benefit of this change? It just adds code > > size and execution time and makes the code more complex, without any > > appearent advanteages? > > The main advantage is that we can use this code in environments with > less than CONFIG_ENV_SIZE worth of stack available. Arguably it makes > the behaviour and code paths similar for redundant and non-redundant > cases (but someone posted a patch to make the redundant case use the > stack). Instead of stack size you pay for that with additional size of the malloc() arena. As I just explained in the other mail [1], this is even worse to deal with and will result in less memoy available for other purposes. And yes, we definitely should use the stack for the redundant case as well. Do you want me to submit such a patch? [1] http://article.gmane.org/gmane.comp.boot-loaders.u-boot/171217 Best regards, Wolfgang Denk
Patch
diff --git a/common/env_mmc.c b/common/env_mmc.c index 65aafa9..0ab11d3 100644 --- a/common/env_mmc.c +++ b/common/env_mmc.c @@ -274,11 +274,18 @@ err: void env_relocate_spec(void) { #if !defined(ENV_IS_EMBEDDED) - ALLOC_CACHE_ALIGN_BUFFER(char, buf, CONFIG_ENV_SIZE); struct mmc *mmc = find_mmc_device(CONFIG_SYS_MMC_ENV_DEV); + env_t *ep; u32 offset; int ret; + ep = (env_t *)malloc(CONFIG_ENV_SIZE); + if (ep == NULL) { + puts("Can't allocate buffers for environment\n"); + ret = 1; + goto err; + } + if (init_mmc_for_env(mmc)) { ret = 1; goto err; @@ -289,12 +296,12 @@ void env_relocate_spec(void) goto fini; } - if (read_env(mmc, CONFIG_ENV_SIZE, offset, buf)) { + if (read_env(mmc, CONFIG_ENV_SIZE, offset, ep)) { ret = 1; goto fini; } - env_import(buf, 1); + env_import((char *)ep, 1); ret = 0; fini: @@ -302,6 +309,8 @@ fini: err: if (ret) set_default_env(NULL); + + free(ep); #endif } #endif /* CONFIG_ENV_OFFSET_REDUND */
Switch the case of non-redundant non-embedded environment to use malloc to allocate buffers, rather than place them on the stack, like the redundant case does. Signed-off-by: Tom Rini <trini@ti.com> --- common/env_mmc.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)