[U-Boot] [PATCH 0/3] arm: reduce .bss section clear time

[Tom Rini]

On Fri, Feb 13, 2015 at 04:48:15PM +0100, Przemyslaw Marczak wrote:
> Hello,
> 
> On 02/12/2015 05:07 PM, Tom Rini wrote:
> >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.majewski at 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.
> >
> 
> The present malloc implementation, which includes the memset at
> init, could be little confusing. Because of this memset. the "few"
> first calls of malloc will always return a pointer to the zeroed
> memory region. Of course, the only calloc do the right job and set
> with zeros the memory region, which was used by any previous malloc
> call.
> 
> So, maybe some code expects zeroed memory also when using malloc.
> 
> In this case, I would like to skip this memset as an option.
> 
> This also requires skip some checking in calloc code.
> 
> I will send the patch with such option for Kconfig.

There might be a few things which are depending on this odd behviour but
they shouldn't.  Most things are zeroing out twice since it's normal to
malloc+memset if you care about contents being zero.  The only real
hang-up I see is that we're half way to release, more or less.  Maybe we
do this early post v2015.04 release to give more time to catch the odd
cases?  And spend some time now auditing.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150213/2eea5fcb/attachment.sig>