From: Tom Rini <trini@ti.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 0/3] arm: reduce .bss section clear time
Date: Thu, 12 Feb 2015 11:07:44 -0500 [thread overview]
Message-ID: <20150212160744.GF7086@bill-the-cat> (raw)
In-Reply-To: <20150205105100.435e62fc@amdc2363>
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@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
-------------- 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/20150212/18211374/attachment.sig>
next prev parent reply other threads:[~2015-02-12 16:07 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-01-28 12:55 [U-Boot] [PATCH 0/3] arm: reduce .bss section clear time Przemyslaw Marczak
2015-01-28 12:55 ` [U-Boot] [PATCH 1/3] exynos: config: enable arch memcpy and arch memset Przemyslaw Marczak
2015-01-28 12:55 ` [U-Boot] [PATCH 2/3] arm: relocation: clear .bss section with arch memset if defined Przemyslaw Marczak
2015-02-01 2:38 ` Albert ARIBAUD
2015-02-02 17:04 ` Bill Pringlemeir
2015-02-02 17:25 ` Tom Rini
2015-02-02 17:28 ` Pantelis Antoniou
2015-02-02 17:36 ` Tom Rini
2015-02-12 15:37 ` Tom Rini
2015-02-13 16:23 ` Przemyslaw Marczak
2015-01-28 12:55 ` [U-Boot] [PATCH 3/3] dfu: mmc: file buffer: remove static allocation Przemyslaw Marczak
2015-01-28 13:12 ` [U-Boot] [PATCH 0/3] arm: reduce .bss section clear time Stefan Roese
2015-01-28 14:10 ` Przemyslaw Marczak
2015-01-28 14:18 ` Pantelis Antoniou
2015-01-28 14:30 ` Przemyslaw Marczak
2015-01-28 14:34 ` Pantelis Antoniou
2015-01-29 15:26 ` Przemyslaw Marczak
2015-01-29 16:48 ` Przemyslaw Marczak
2015-02-02 8:46 ` Lukasz Majewski
2015-02-02 18:15 ` Simon Glass
2015-02-05 9:51 ` Lukasz Majewski
2015-02-12 16:07 ` Tom Rini [this message]
2015-02-13 15:48 ` Przemyslaw Marczak
2015-02-13 18:13 ` Tom Rini
2015-02-13 16:15 ` Przemyslaw Marczak
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150212160744.GF7086@bill-the-cat \
--to=trini@ti.com \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox