On Thu, Feb 05, 2015 at 10:51:00AM +0100, Lukasz Majewski wrote: > Hi Simon, > > > Hi Lukasz, > > > > On 2 February 2015 at 01:46, Lukasz Majewski <l.majew...@samsung.com> > > wrote: > > > Dear All, > > > > > >> And the next is interesting. > > >> odroid_defconfig has more than 80MB for malloc (we need about > > >> 64mb for the DFU now, to be able write 32MB file). > > >> > > >> This is the CONFIG_SYS_MALLOC_LEN. And the memory area for malloc > > >> is set to 0 in function mem_malloc_init(). So for this config that > > >> function sets more than 80MB to zero. > > >> > > >> This is not good, because we shouldn't expect zeroed memory > > >> returned by malloc pointer. This is a job for calloc. > > >> > > >> Especially if some command expects zeroed memory after malloc, > > >> probably after few next calls - it can crash... > > > > > > I think that the above excerpt is _really_ important and should be > > > discussed. > > > > > > I've "cut" it from the original post, so it won't get lost between > > > the lines. > > > > > > It seems really strange, that malloc() area is cleared after > > > relocation. Which means that all "first" malloc'ed buffers get > > > implicitly zeroed. > > > > > > Przemek is right here that this zeroing shouldn't be performed. > > > > > > I'm also concerned about potential bugs, which show up (or even > > > worse - won't show up soon) after this change. > > > > > > Hence, I would like to ask directly the community about the possible > > > solutions. > > > > > > Please look at: ./common/dlmalloc.c mem_alloc_init() function [1]. > > > > > > On the one hand removing memset() at [1] speeds up booting time and > > > makes malloc() doing what is is supposed to do. > > > > > > On the other hand there might be in space some boards, which rely on > > > this memset and without it some wired things may start to happening. > > > > I think removing it is a good idea. It was one optimisation that I did > > for boot time in the Chromium tree. If you do it now (and Tom agrees) > > then there is plenty of time to test for this release cycle. You could > > go further and add a test CONFIG which fills it with some other > > non-zero value. > > Tom, is such approach acceptable for you?
I was thinking at first we should default to a poisoned value. But given what we're seeing with generic board updates (lots of boards aren't even build-tested at every release which isn't really a surprise), I think the "funky" boards which may exist are probably not going to be seen for a while anyhow so we'd have to default to a poison for a long while. So yes, lets just add a CONFIG option (and Kconfig line) to optionally do it and default to no memset. ... but I just audited everyone doing "malloc (" and found a few things to fixup so we really do want to take a poke around. -- Tom
signature.asc
Description: Digital signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot