From: Przemyslaw Marczak <p.marczak@samsung.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 0/3] arm: reduce .bss section clear time
Date: Fri, 13 Feb 2015 17:15:38 +0100 [thread overview]
Message-ID: <54DE232A.6060605@samsung.com> (raw)
In-Reply-To: <CAPnjgZ098d6A2tXv53qgismujbp29vP8pOYTqO1QgpDgGP+=ew@mail.gmail.com>
Hello Simon,
On 02/02/2015 07:15 PM, Simon Glass wrote:
> 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.
>
> Regards,
> Simon
>
Filling the malloc area with some pattern was a good idea to find out,
why my trats2 had some issue after skip the memset with zeros in malloc
init.
And actually the issue was not in malloc call, but it was in calloc.
The present implementation assumes that memory reserved for malloc
is zeroed at init. And the calloc do the check, how much of the
allocated memory is fresh(doesn't require zeroing).
After skip this fresh memory check, the calloc works fine.
Anyway, I think that this should be optional and tested on every config,
before enable.
I would like to test something and will send the updated patch set on
Monday.
Best regards,
--
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com
prev parent reply other threads:[~2015-02-13 16:15 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
2015-02-13 15:48 ` Przemyslaw Marczak
2015-02-13 18:13 ` Tom Rini
2015-02-13 16:15 ` Przemyslaw Marczak [this message]
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=54DE232A.6060605@samsung.com \
--to=p.marczak@samsung.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