[U-Boot] [PATCH] arm: mx6: Reduce SPL malloc pool size

[Przemyslaw Marczak]

Hello Marek,

On 11/25/2015 11:56 AM, Marek Vasut wrote:
> On Wednesday, November 25, 2015 at 11:40:36 AM, Przemyslaw Marczak wrote:
>> Hello Tim, Marek
>>
>> On 11/20/2015 10:40 PM, Tim Harvey wrote:
>>> On Fri, Nov 20, 2015 at 12:43 PM, Marek Vasut <marex at denx.de> wrote:
>>>> Using 50 MiB malloc pool in SPL is nonsense. Since the caches are not
>>>> enabled in SPL, it takes 2 seconds to init the pool and has no obvious
>>>> benefit. Reduce the size to 1 MiB.
>>>>
>>>> Signed-off-by: Marek Vasut <marex at denx.de>
>>>> Cc: Stefano Babic <sbabic at denx.de>
>>>> Cc: Tim Harvey <tharvey at gateworks.com>
>>>> ---
>>>>
>>>>    include/configs/imx6_spl.h | 6 +++---
>>>>    1 file changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/include/configs/imx6_spl.h b/include/configs/imx6_spl.h
>>>> index 1744f2c..43ce7fe 100644
>>>> --- a/include/configs/imx6_spl.h
>>>> +++ b/include/configs/imx6_spl.h
>>>> @@ -63,15 +63,15 @@
>>>>
>>>>    #if defined(CONFIG_MX6SX) || defined(CONFIG_MX6UL) ||
>>>>    defined(CONFIG_MX6SL) #define CONFIG_SPL_BSS_START_ADDR
>>>>    0x88200000
>>>>
>>>> -#define CONFIG_SPL_BSS_MAX_SIZE        0x100000        /* 1 MB */
>>>> +#define CONFIG_SPL_BSS_MAX_SIZE        0x100000                /* 1 MB
>>>> */
>>>>
>>>>    #define CONFIG_SYS_SPL_MALLOC_START    0x88300000
>>>>
>>>> -#define CONFIG_SYS_SPL_MALLOC_SIZE     0x3200000       /* 50 MB */
>>>> +#define CONFIG_SYS_SPL_MALLOC_SIZE     0x100000                /* 1 MB
>>>> */
>>>>
>>>>    #define CONFIG_SYS_TEXT_BASE           0x87800000
>>>>    #else
>>>>    #define CONFIG_SPL_BSS_START_ADDR      0x18200000
>>>>    #define CONFIG_SPL_BSS_MAX_SIZE                0x100000        /* 1 MB
>>>>    */ #define CONFIG_SYS_SPL_MALLOC_START    0x18300000
>>>>
>>>> -#define CONFIG_SYS_SPL_MALLOC_SIZE     0x3200000       /* 50 MB */
>>>> +#define CONFIG_SYS_SPL_MALLOC_SIZE     0x100000        /* 1 MB */
>>>>
>>>>    #define CONFIG_SYS_TEXT_BASE           0x17800000
>>>>    #endif
>>>>    #endif
>>>>
>>>> --
>>>> 2.1.4
>>>
>>> Acked-by: Tim Harvey <tharvey at gateworks.com>
>>>
>>> thanks for dropping 2 secs off our time to boot!
>>>
>>> Tim
>>
>> The boot time for SPL and U-Boot can be reduced more if
>> CONFIG_SYS_MALLOC_CLEAR_ON_INIT is unset (default is set).
>
> This might confuse DM, so I don't want this. DM checks if it's structures were
> already inited and if we don't clear the malloc area, they would be.
>

DM is safe - it uses calloc() for it's structures and also driver's 
static structures are zeroed. Only some private data which is not 
allocated by DM can be risky to use.

To be precise - we don't use malloc() to get pointer for zeroed memory, 
this is the job for the calloc().

So any code with bad use of malloc() should be fixed ASAP.

If you assume that clear the malloc pool at boot is safe to use, then 
I'm asking you, what about the sequence malloc/free/malloc for 
potentially the same area? You can't be sure that the second returned 
pointer after calling free() - points to the initially zeroed pool.

We don't clean the malloc pool for configs that we maintain and we 
didn't noticed any issues related to this. And the boot time is reduced 
significantly.

>> Then it doesn't matter what is the size of malloc pool but it's not
>> cleared - so needs check if malloc() calls should be changed to calloc().
>>
>> Best regards,
>
> Best regards,
> Marek Vasut
>

Best regards,
-- 
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com