arm: mx6: Reduce SPL malloc pool size
PATCH 1/1 arm: mx6: Reduce SPL malloc pool size
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.
--- include/configs/imx6_spl.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
include/configs/imx6_spl.h +3 -3
@@ -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
Signature · 1 line
2.1.4
PATCH 1/1 arm: mx6: Reduce SPL malloc pool size
Quoted 3 lines · Marek Vasut
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.
You're absolutely right ! Nice catch !
Quoted 22 lines
Signed-off-by Marek Vasut <[email protected]>Cc Stefano Babic <[email protected]>Cc Tim Harvey <[email protected]>--- include/configs/imx6_spl.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)include/configs/imx6_spl.h +2
-2@@ -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 */
include/configs/imx6_spl.h line 77
Quoted 10 lines
include/configs/imx6_spl.h +1
-1#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
Best regards,
Stefano Babic
Signature · 5 lines
===================================================================== DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de =====================================================================
PATCH 1/1 arm: mx6: Reduce SPL malloc pool size
In reply to Marek VasutQuoted 35 lines · Marek Vasut
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 <[email protected]>Cc Stefano Babic <[email protected]>Cc Tim Harvey <[email protected]>--- include/configs/imx6_spl.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)include/configs/imx6_spl.h +3
-4@@ -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 --
Quoted 2 lines
2.1.4
thanks for dropping 2 secs off our time to boot!
Tim
PATCH 1/1 arm: mx6: Reduce SPL malloc pool size
Hello Tim, Marek
Quoted 41 lines · Tim Harvey
Quoted 35 lines · Marek Vasut
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 <[email protected]>Cc Stefano Babic <[email protected]>Cc Tim Harvey <[email protected]>--- include/configs/imx6_spl.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)include/configs/imx6_spl.h +3
-4@@ -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 --Quoted 2 lines
2.1.4
Acked-by Tim Harvey <[email protected]>
Quoted 4 lines
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).
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,
Signature · 4 lines
Przemyslaw Marczak Samsung R&D Institute Poland Samsung Electronics p.marczak at samsung.com
PATCH 1/1 arm: mx6: Reduce SPL malloc pool size
Quoted 54 lines · Przemyslaw Marczak
Hello Tim, Marek
Quoted 51 lines · Tim Harvey
Quoted 46 lines · Marek Vasut
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 <[email protected]>Cc Stefano Babic <[email protected]>Cc Tim Harvey <[email protected]>---
include/configs/imx6_spl.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)include/configs/imx6_spl.h +2
-2@@ -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 0x18300000include/configs/imx6_spl.h +1
-2-#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 --Quoted 1 line
2.1.4
Acked-by Tim Harvey <[email protected]>
Quoted 6 lines
Quoted 3 lines
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_INITis 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.
Quoted 4 lines
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 tocalloc().Best regards,
Best regards,
Marek Vasut
PATCH 1/1 arm: mx6: Reduce SPL malloc pool size
Hello Marek,
Quoted 57 lines · Marek Vasut
Quoted 56 lines · Przemyslaw Marczak
Hello Tim, Marek
Quoted 53 lines · Tim Harvey
Quoted 46 lines · Marek Vasut
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 <[email protected]>Cc Stefano Babic <[email protected]>Cc Tim Harvey <[email protected]>---
include/configs/imx6_spl.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)include/configs/imx6_spl.h +2
-2@@ -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 0x18300000include/configs/imx6_spl.h +1
-2-#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 --Quoted 1 line
2.1.4
Acked-by Tim Harvey <[email protected]>thanks for dropping 2 secs off our time to boot!
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
Tim
The boot time for SPL and U-Boot can be reduced more if
CONFIG_SYS_MALLOC_CLEAR_ON_INITis 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.
Quoted 8 lines
Quoted 4 lines
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 tocalloc().Best regards,
Best regards,
Marek Vasut
Best regards,
Signature · 4 lines
Przemyslaw Marczak Samsung R&D Institute Poland Samsung Electronics p.marczak at samsung.com
PATCH 1/1 arm: mx6: Reduce SPL malloc pool size
Quoted 78 lines · Przemyslaw Marczak
Hello Marek,
Quoted 57 lines · Marek Vasut
Quoted 56 lines · Przemyslaw Marczak
Hello Tim, Marek
Quoted 53 lines · Tim Harvey
Quoted 46 lines · Marek Vasut
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 <[email protected]>Cc Stefano Babic <[email protected]>Cc Tim Harvey <[email protected]>---
include/configs/imx6_spl.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)include/configs/imx6_spl.h +1
-1@@ -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 0x88300000include/configs/imx6_spl.h +1
-1-#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 0x18300000include/configs/imx6_spl.h +1
-2-#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 --Quoted 1 line
2.1.4
Acked-by Tim Harvey <[email protected]>thanks for dropping 2 secs off our time to boot!
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
Tim
The boot time for SPL and U-Boot can be reduced more if
CONFIG_SYS_MALLOC_CLEAR_ON_INITis 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 thecalloc().So any code with bad use of
malloc()should be fixed ASAP.
Quoted 8 lines
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.
Do you use DM in SPL ?
I just checked and I see it's probably only the GD which needs to be cleared out, so we should indeed be safe. In which case, patch is welcome to enable CONFIG_SYS_MALLOC_CLEAR_ON_INIT .
PATCH 1/1 arm: mx6: Reduce SPL malloc pool size
Quoted 84 lines · Marek Vasut
Quoted 83 lines · Przemyslaw Marczak
Hello Marek,
Quoted 57 lines · Marek Vasut
Quoted 56 lines · Przemyslaw Marczak
Hello Tim, Marek
Quoted 53 lines · Tim Harvey
Quoted 46 lines · Marek Vasut
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 <[email protected]>Cc Stefano Babic <[email protected]>Cc Tim Harvey <[email protected]>---
include/configs/imx6_spl.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)include/configs/imx6_spl.h +1
-1@@ -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 0x88300000include/configs/imx6_spl.h +1
-1-#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 0x18300000include/configs/imx6_spl.h +1
-2-#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 --Quoted 1 line
2.1.4
Acked-by Tim Harvey <[email protected]>thanks for dropping 2 secs off our time to boot!
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
Tim
The boot time for SPL and U-Boot can be reduced more if
CONFIG_SYS_MALLOC_CLEAR_ON_INITis 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 thecalloc().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.
Quoted 5 lines
Quoted 3 lines
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.
Do you use DM in SPL ?
No, we don't use the SPL - for the boards which we maintain.
Quoted 5 lines
I just checked and I see it's probably only the GD which needs to be cleared out, so we should indeed be safe. In which case, patch is welcome to enable
CONFIG_SYS_MALLOC_CLEAR_ON_INIT.
Yes I can see also for the Exynos SPL, that GD is manually cleared.
But disabling the default CONFIG_SYS_MALLOC_CLEAR_ON_INIT, will help you in reducing the U-Boot init time.
Just try and if you can easy measure the real difference for enable/disable this config - share the results - it can give us an interesting conclusions.
Best regards,
Signature · 4 lines
Przemyslaw Marczak Samsung R&D Institute Poland Samsung Electronics p.marczak at samsung.com
PATCH 1/1 arm: mx6: Reduce SPL malloc pool size
Quoted 85 lines · Przemyslaw Marczak
Quoted 84 lines · Marek Vasut
Quoted 83 lines · Przemyslaw Marczak
Hello Marek,
Quoted 57 lines · Marek Vasut
Quoted 56 lines · Przemyslaw Marczak
Hello Tim, Marek
Quoted 53 lines · Tim Harvey
Quoted 46 lines · Marek Vasut
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 <[email protected]>Cc Stefano Babic <[email protected]>Cc Tim Harvey <[email protected]>---
include/configs/imx6_spl.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)include/configs/imx6_spl.h +1
-1@@ -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 0x88300000include/configs/imx6_spl.h +1
-1-#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 0x18300000include/configs/imx6_spl.h +1
-2-#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 --Quoted 1 line
2.1.4
Acked-by Tim Harvey <[email protected]>thanks for dropping 2 secs off our time to boot!
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
Tim
The boot time for SPL and U-Boot can be reduced more if
CONFIG_SYS_MALLOC_CLEAR_ON_INITis 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 thecalloc().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.
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
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.
Do you use DM in SPL ?
No, we don't use the SPL - for the boards which we maintain.
OK
Quoted 11 lines
Quoted 3 lines
I just checked and I see it's probably only the GD which needs to be cleared out, so we should indeed be safe. In which case, patch is welcome to enable
CONFIG_SYS_MALLOC_CLEAR_ON_INIT.Yes I can see also for the Exynos SPL, that GD is manually cleared.
But disabling the default
CONFIG_SYS_MALLOC_CLEAR_ON_INIT, will help you in reducing the U-Boot init time.
Just try and if you can easy measure the real difference for enable/disable this config - share the results - it can give us an interesting conclusions.
Sure, thanks for the hint, but I still don't see any reason for having 50 MiB malloc area in SPL ;-)
PATCH 1/1 arm: mx6: Reduce SPL malloc pool size
Quoted 102 lines · Marek Vasut
Quoted 85 lines · Przemyslaw Marczak
Quoted 84 lines · Marek Vasut
Quoted 83 lines · Przemyslaw Marczak
Hello Marek,
Quoted 57 lines · Marek Vasut
Quoted 56 lines · Przemyslaw Marczak
Hello Tim, Marek
Quoted 53 lines · Tim Harvey
Quoted 46 lines · Marek Vasut
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 <[email protected]>Cc Stefano Babic <[email protected]>Cc Tim Harvey <[email protected]>---
include/configs/imx6_spl.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)include/configs/imx6_spl.h +1
-1@@ -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 0x88300000include/configs/imx6_spl.h +1
-1-#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 0x18300000include/configs/imx6_spl.h +1
-2-#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 --Quoted 1 line
2.1.4
Acked-by Tim Harvey <[email protected]>thanks for dropping 2 secs off our time to boot!
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
Tim
The boot time for SPL and U-Boot can be reduced more if
CONFIG_SYS_MALLOC_CLEAR_ON_INITis 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 thecalloc().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.Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
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.
Do you use DM in SPL ?
No, we don't use the SPL - for the boards which we maintain.
OK
Quoted 6 lines
Quoted 3 lines
I just checked and I see it's probably only the GD which needs to be cleared out, so we should indeed be safe. In which case, patch is welcome to enable
CONFIG_SYS_MALLOC_CLEAR_ON_INIT.Yes I can see also for the Exynos SPL, that GD is manually cleared.
Quoted 8 lines
Quoted 5 lines
But disabling the default
CONFIG_SYS_MALLOC_CLEAR_ON_INIT, will help you in reducing the U-Boot init time.
Just try and if you can easy measure the real difference for enable/disable this config - share the results - it can give us an interesting conclusions.Sure, thanks for the hint, but I still don't see any reason for having 50 MiB malloc area in SPL ;-)
Perhaps the use case for a >1MB malloc area is falcon mode. While a 1MB area certainly works for loading and bootstrapping to u-boot.img I have used anywhere from 1MB to 8MB images for falcon mode.
Tim
PATCH 1/1 arm: mx6: Reduce SPL malloc pool size
Quoted 8 lines · Tim Harvey
Quoted 7 lines · Marek Vasut
Quoted 6 lines · Przemyslaw Marczak
Quoted 5 lines · Marek Vasut
Quoted 4 lines · Przemyslaw Marczak
Hello Marek,
Quoted 1 line · Marek Vasut
On Wednesday, November 25, 2015 at 11:40:36 AM, Przemyslaw Marczak
Quoted 102 lines · wrote:
Quoted 102 lines
Quoted 80 lines
Quoted 80 lines
Quoted 80 lines
Quoted 57 lines
Quoted 57 lines
Hello Tim, Marek
Quoted 54 lines · Tim Harvey
Quoted 42 lines · Marek Vasut
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 <[email protected]>Cc Stefano Babic <[email protected]>Cc Tim Harvey <[email protected]>---
include/configs/imx6_spl.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)(quoted diff) +0
-0
b/include/configs/imx6_spl.hindex 1744f2c..43ce7fe 100644include/configs/imx6_spl.h +0
-3@@ -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_SIZE0x100000/* 1
MB */Quoted 6 lines
#define CONFIG_SYS_TEXT_BASE 0x17800000 #endif #endif--
2.1.4Acked-by Tim Harvey <[email protected]>thanks for dropping 2 secs off our time to boot!
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
Tim
The boot time for SPL and U-Boot can be reduced more if
CONFIG_SYS_MALLOC_CLEAR_ON_INITis 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 thecalloc().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.Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
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.
Do you use DM in SPL ?
No, we don't use the SPL - for the boards which we maintain.
OK
Quoted 11 lines
Quoted 3 lines
I just checked and I see it's probably only the GD which needs to be cleared out, so we should indeed be safe. In which case, patch is welcome to enable
CONFIG_SYS_MALLOC_CLEAR_ON_INIT.Yes I can see also for the Exynos SPL, that GD is manually cleared.
But disabling the default
CONFIG_SYS_MALLOC_CLEAR_ON_INIT, will help you in reducing the U-Boot init time.
Just try and if you can easy measure the real difference for enable/disable this config - share the results - it can give us an interesting conclusions.
Quoted 6 lines
Quoted 2 lines
Sure, thanks for the hint, but I still don't see any reason for having 50 MiB malloc area in SPL ;-)
Perhaps the use case for a >1MB malloc area is falcon mode. While a 1MB area certainly works for loading and bootstrapping to u-boot.img I have used anywhere from 1MB to 8MB images for falcon mode.
Does the falcon mode load the image into malloc'd area or directly into RAM location ? If it's the former, then yeah, large malloc pool makes sense and MALLOC_CLEAR_ON_INIT makes sense as well I guess.
PATCH 1/1 arm: mx6: Reduce SPL malloc pool size
Quoted 8 lines · Marek Vasut
Quoted 4 lines
Perhaps the use case for a >1MB malloc area is falcon mode. While a 1MB area certainly works for loading and bootstrapping to u-boot.img I have used anywhere from 1MB to 8MB images for falcon mode.
Does the falcon mode load the image into malloc'd area or directly into RAM location ? If it's the former, then yeah, large malloc pool makes sense and
MALLOC_CLEAR_ON_INITmakes sense as well I guess.
Marek,
I checked and Falcon mode does load images directly into RAM so large malloc still doesn't make sense and should be reduced. I think 1MB is a fine value.
Tim
PATCH 1/1 arm: mx6: Reduce SPL malloc pool size
Quoted 10 lines · Tim Harvey
Quoted 7 lines · Marek Vasut
Quoted 3 lines
Perhaps the use case for a >1MB malloc area is falcon mode. While a 1MB area certainly works for loading and bootstrapping to u-boot.img I have used anywhere from 1MB to 8MB images for falcon mode.
Does the falcon mode load the image into malloc'd area or directly into RAM location ? If it's the former, then yeah, large malloc pool makes sense and
MALLOC_CLEAR_ON_INITmakes sense as well I guess.Marek,
Hi Tim,
Quoted 3 lines
I checked and Falcon mode does load images directly into RAM so large malloc still doesn't make sense and should be reduced. I think 1MB is a fine value.
Thank you!
Best regards,
Marek Vasut
PATCH 1/1 arm: mx6: Reduce SPL malloc pool size
In reply to Marek VasutHi Marek,
Quoted 8 lines · Marek Vasut
Quoted 4 lines
Perhaps the use case for a >1MB malloc area is falcon mode. While a 1MB area certainly works for loading and bootstrapping to u-boot.img I have used anywhere from 1MB to 8MB images for falcon mode.
Does the falcon mode load the image into malloc'd area or directly into RAM location ? If it's the former, then yeah, large malloc pool makes sense and
MALLOC_CLEAR_ON_INITmakes sense as well I guess.
No, it copies directly into the RAM. Large malloc area are not needed for Falcon Mode, and I have not idea who needs a large area. More as instantiating SPI or I2C is generally not needed.
Best regards,
Stefano Babic
Signature · 5 lines
===================================================================== DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de =====================================================================