Re: [U-Boot] [PATCH] arm: mx6: Reduce SPL malloc pool size
Hi Marek, On 01/12/2015 00:31, Marek Vasut wrote: >> >> 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. 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 -- = 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: sba...@denx.de = ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] arm: mx6: Reduce SPL malloc pool size
On Tuesday, December 01, 2015 at 03:13:17 PM, Tim Harvey wrote: > On Mon, Nov 30, 2015 at 3:31 PM, Marek Vasut wrote: > >> 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. > > Marek, Hi Tim, > 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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] arm: mx6: Reduce SPL malloc pool size
On Mon, Nov 30, 2015 at 3:31 PM, Marek Vasut wrote: >> >> 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. 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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] arm: mx6: Reduce SPL malloc pool size
On Monday, November 30, 2015 at 07:56:55 PM, Tim Harvey wrote: > On Wed, Nov 25, 2015 at 5:32 AM, Marek Vasut wrote: > > On Wednesday, November 25, 2015 at 02:15:13 PM, Przemyslaw Marczak wrote: > >> On 11/25/2015 01:16 PM, Marek Vasut wrote: > >> > On Wednesday, November 25, 2015 at 01:00:41 PM, Przemyslaw Marczak wrote: > >> >> 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 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 > >> >> Cc: Stefano Babic > >> >> Cc: Tim Harvey > >> >> --- > >> >> > >> >> 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 > >> >> 0x8820 > >> >> > >> >> -#define CONFIG_SPL_BSS_MAX_SIZE0x10/* 1 MB > >> >> */ +#define CONFIG_SPL_BSS_MAX_SIZE0x10 > >> >> /* 1 MB */ > >> >> > >> >> #define CONFIG_SYS_SPL_MALLOC_START0x8830 > >> >> > >> >> -#define CONFIG_SYS_SPL_MALLOC_SIZE 0x320 /* 50 MB > >> >> */ +#define CONFIG_SYS_SPL_MALLOC_SIZE 0x10 > >> >> /* 1 MB */ > >> >> > >> >> #define CONFIG_SYS_TEXT_BASE 0x8780 > >> >> #else > >> >> #define CONFIG_SPL_BSS_START_ADDR 0x1820 > >> >> #define CONFIG_SPL_BSS_MAX_SIZE0x10 > >> >> /* 1 MB */ #define CONFIG_SYS_SPL_MALLOC_START0x1830 > >> >> > >> >> -#define CONFIG_SYS_SPL_MALLOC_SIZE 0x320 /* 50 MB > >> >> */ +#define CONFIG_SYS_SPL_MALLOC_SIZE 0x10/* 1 > >> >> MB */ > >> >> > >> >> #define CONFIG_SYS_TEXT_BASE 0x1780 > >> >> #endif > >> >> #endif > >> >> > >> >> -- > >> >> 2.1.4 > >> > > >> > Acked-by: Tim Harvey > >> > > >> > 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. > >> > > >> > Do you use DM in SPL ? > >> > >> No, we don't use the SPL - for the boards which we maintain. > > > > OK > > > >> > 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 ;-) > > Perhaps the use case for a >1MB malloc area is falcon mode. While a > 1MB area certainly works for l
Re: [U-Boot] [PATCH] arm: mx6: Reduce SPL malloc pool size
On Wed, Nov 25, 2015 at 5:32 AM, Marek Vasut wrote: > On Wednesday, November 25, 2015 at 02:15:13 PM, Przemyslaw Marczak wrote: >> On 11/25/2015 01:16 PM, Marek Vasut wrote: >> > On Wednesday, November 25, 2015 at 01:00:41 PM, Przemyslaw Marczak wrote: >> >> 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 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 >> >> Cc: Stefano Babic >> >> Cc: Tim Harvey >> >> --- >> >> >> >> 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 >> >> 0x8820 >> >> >> >> -#define CONFIG_SPL_BSS_MAX_SIZE0x10/* 1 MB */ >> >> +#define CONFIG_SPL_BSS_MAX_SIZE0x10/* 1 >> >> MB */ >> >> >> >> #define CONFIG_SYS_SPL_MALLOC_START0x8830 >> >> >> >> -#define CONFIG_SYS_SPL_MALLOC_SIZE 0x320 /* 50 MB */ >> >> +#define CONFIG_SYS_SPL_MALLOC_SIZE 0x10/* 1 >> >> MB */ >> >> >> >> #define CONFIG_SYS_TEXT_BASE 0x8780 >> >> #else >> >> #define CONFIG_SPL_BSS_START_ADDR 0x1820 >> >> #define CONFIG_SPL_BSS_MAX_SIZE0x10 >> >> /* 1 MB */ #define CONFIG_SYS_SPL_MALLOC_START0x1830 >> >> >> >> -#define CONFIG_SYS_SPL_MALLOC_SIZE 0x320 /* 50 MB */ >> >> +#define CONFIG_SYS_SPL_MALLOC_SIZE 0x10/* 1 MB */ >> >> >> >> #define CONFIG_SYS_TEXT_BASE 0x1780 >> >> #endif >> >> #endif >> >> >> >> -- >> >> 2.1.4 >> > >> > Acked-by: Tim Harvey >> > >> > 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. >> > >> > Do you use DM in SPL ? >> >> No, we don't use the SPL - for the boards which we maintain. > > OK > >> > 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 ;-) 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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] arm: mx6: Reduce SPL malloc pool size
On Wednesday, November 25, 2015 at 02:15:13 PM, Przemyslaw Marczak wrote: > On 11/25/2015 01:16 PM, Marek Vasut wrote: > > On Wednesday, November 25, 2015 at 01:00:41 PM, Przemyslaw Marczak wrote: > >> 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 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 > >> Cc: Stefano Babic > >> Cc: Tim Harvey > >> --- > >> > >> 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 > >> 0x8820 > >> > >> -#define CONFIG_SPL_BSS_MAX_SIZE0x10/* 1 MB */ > >> +#define CONFIG_SPL_BSS_MAX_SIZE0x10/* 1 > >> MB */ > >> > >> #define CONFIG_SYS_SPL_MALLOC_START0x8830 > >> > >> -#define CONFIG_SYS_SPL_MALLOC_SIZE 0x320 /* 50 MB */ > >> +#define CONFIG_SYS_SPL_MALLOC_SIZE 0x10/* 1 > >> MB */ > >> > >> #define CONFIG_SYS_TEXT_BASE 0x8780 > >> #else > >> #define CONFIG_SPL_BSS_START_ADDR 0x1820 > >> #define CONFIG_SPL_BSS_MAX_SIZE0x10 > >> /* 1 MB */ #define CONFIG_SYS_SPL_MALLOC_START0x1830 > >> > >> -#define CONFIG_SYS_SPL_MALLOC_SIZE 0x320 /* 50 MB */ > >> +#define CONFIG_SYS_SPL_MALLOC_SIZE 0x10/* 1 MB */ > >> > >> #define CONFIG_SYS_TEXT_BASE 0x1780 > >> #endif > >> #endif > >> > >> -- > >> 2.1.4 > > > > Acked-by: Tim Harvey > > > > 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. > > > > Do you use DM in SPL ? > > No, we don't use the SPL - for the boards which we maintain. OK > > 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 ;-) ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] arm: mx6: Reduce SPL malloc pool size
On 11/25/2015 01:16 PM, Marek Vasut wrote: On Wednesday, November 25, 2015 at 01:00:41 PM, Przemyslaw Marczak wrote: 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 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 Cc: Stefano Babic Cc: Tim Harvey --- 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 0x8820 -#define CONFIG_SPL_BSS_MAX_SIZE0x10/* 1 MB */ +#define CONFIG_SPL_BSS_MAX_SIZE0x10/* 1 MB */ #define CONFIG_SYS_SPL_MALLOC_START0x8830 -#define CONFIG_SYS_SPL_MALLOC_SIZE 0x320 /* 50 MB */ +#define CONFIG_SYS_SPL_MALLOC_SIZE 0x10/* 1 MB */ #define CONFIG_SYS_TEXT_BASE 0x8780 #else #define CONFIG_SPL_BSS_START_ADDR 0x1820 #define CONFIG_SPL_BSS_MAX_SIZE0x10/* 1 MB */ #define CONFIG_SYS_SPL_MALLOC_START0x1830 -#define CONFIG_SYS_SPL_MALLOC_SIZE 0x320 /* 50 MB */ +#define CONFIG_SYS_SPL_MALLOC_SIZE 0x10/* 1 MB */ #define CONFIG_SYS_TEXT_BASE 0x1780 #endif #endif -- 2.1.4 Acked-by: Tim Harvey 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. Do you use DM in SPL ? No, we don't use the SPL - for the boards which we maintain. 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, -- Przemyslaw Marczak Samsung R&D Institute Poland Samsung Electronics p.marc...@samsung.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] arm: mx6: Reduce SPL malloc pool size
On Wednesday, November 25, 2015 at 01:00:41 PM, Przemyslaw Marczak wrote: > 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 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 > Cc: Stefano Babic > Cc: Tim Harvey > --- > > 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 > 0x8820 > > -#define CONFIG_SPL_BSS_MAX_SIZE0x10/* 1 MB */ > +#define CONFIG_SPL_BSS_MAX_SIZE0x10/* 1 > MB */ > > #define CONFIG_SYS_SPL_MALLOC_START0x8830 > > -#define CONFIG_SYS_SPL_MALLOC_SIZE 0x320 /* 50 MB */ > +#define CONFIG_SYS_SPL_MALLOC_SIZE 0x10/* 1 > MB */ > > #define CONFIG_SYS_TEXT_BASE 0x8780 > #else > #define CONFIG_SPL_BSS_START_ADDR 0x1820 > #define CONFIG_SPL_BSS_MAX_SIZE0x10/* 1 > MB */ #define CONFIG_SYS_SPL_MALLOC_START0x1830 > > -#define CONFIG_SYS_SPL_MALLOC_SIZE 0x320 /* 50 MB */ > +#define CONFIG_SYS_SPL_MALLOC_SIZE 0x10/* 1 MB */ > > #define CONFIG_SYS_TEXT_BASE 0x1780 > #endif > #endif > > -- > 2.1.4 > >>> > >>> Acked-by: Tim Harvey > >>> > >>> 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. 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 . ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] arm: mx6: Reduce SPL malloc pool size
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 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 Cc: Stefano Babic Cc: Tim Harvey --- 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 0x8820 -#define CONFIG_SPL_BSS_MAX_SIZE0x10/* 1 MB */ +#define CONFIG_SPL_BSS_MAX_SIZE0x10/* 1 MB */ #define CONFIG_SYS_SPL_MALLOC_START0x8830 -#define CONFIG_SYS_SPL_MALLOC_SIZE 0x320 /* 50 MB */ +#define CONFIG_SYS_SPL_MALLOC_SIZE 0x10/* 1 MB */ #define CONFIG_SYS_TEXT_BASE 0x8780 #else #define CONFIG_SPL_BSS_START_ADDR 0x1820 #define CONFIG_SPL_BSS_MAX_SIZE0x10/* 1 MB */ #define CONFIG_SYS_SPL_MALLOC_START0x1830 -#define CONFIG_SYS_SPL_MALLOC_SIZE 0x320 /* 50 MB */ +#define CONFIG_SYS_SPL_MALLOC_SIZE 0x10/* 1 MB */ #define CONFIG_SYS_TEXT_BASE 0x1780 #endif #endif -- 2.1.4 Acked-by: Tim Harvey 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.marc...@samsung.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] arm: mx6: Reduce SPL malloc pool size
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 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 > >> Cc: Stefano Babic > >> Cc: Tim Harvey > >> --- > >> > >> 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 > >> 0x8820 > >> > >> -#define CONFIG_SPL_BSS_MAX_SIZE0x10/* 1 MB */ > >> +#define CONFIG_SPL_BSS_MAX_SIZE0x10/* 1 MB > >> */ > >> > >> #define CONFIG_SYS_SPL_MALLOC_START0x8830 > >> > >> -#define CONFIG_SYS_SPL_MALLOC_SIZE 0x320 /* 50 MB */ > >> +#define CONFIG_SYS_SPL_MALLOC_SIZE 0x10/* 1 MB > >> */ > >> > >> #define CONFIG_SYS_TEXT_BASE 0x8780 > >> #else > >> #define CONFIG_SPL_BSS_START_ADDR 0x1820 > >> #define CONFIG_SPL_BSS_MAX_SIZE0x10/* 1 MB > >> */ #define CONFIG_SYS_SPL_MALLOC_START0x1830 > >> > >> -#define CONFIG_SYS_SPL_MALLOC_SIZE 0x320 /* 50 MB */ > >> +#define CONFIG_SYS_SPL_MALLOC_SIZE 0x10/* 1 MB */ > >> > >> #define CONFIG_SYS_TEXT_BASE 0x1780 > >> #endif > >> #endif > >> > >> -- > >> 2.1.4 > > > > Acked-by: Tim Harvey > > > > 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. > 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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] arm: mx6: Reduce SPL malloc pool size
Hello Tim, Marek On 11/20/2015 10:40 PM, Tim Harvey wrote: On Fri, Nov 20, 2015 at 12:43 PM, Marek Vasut 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 Cc: Stefano Babic Cc: Tim Harvey --- 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 0x8820 -#define CONFIG_SPL_BSS_MAX_SIZE0x10/* 1 MB */ +#define CONFIG_SPL_BSS_MAX_SIZE0x10/* 1 MB */ #define CONFIG_SYS_SPL_MALLOC_START0x8830 -#define CONFIG_SYS_SPL_MALLOC_SIZE 0x320 /* 50 MB */ +#define CONFIG_SYS_SPL_MALLOC_SIZE 0x10/* 1 MB */ #define CONFIG_SYS_TEXT_BASE 0x8780 #else #define CONFIG_SPL_BSS_START_ADDR 0x1820 #define CONFIG_SPL_BSS_MAX_SIZE0x10/* 1 MB */ #define CONFIG_SYS_SPL_MALLOC_START0x1830 -#define CONFIG_SYS_SPL_MALLOC_SIZE 0x320 /* 50 MB */ +#define CONFIG_SYS_SPL_MALLOC_SIZE 0x10/* 1 MB */ #define CONFIG_SYS_TEXT_BASE 0x1780 #endif #endif -- 2.1.4 Acked-by: Tim Harvey 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, -- Przemyslaw Marczak Samsung R&D Institute Poland Samsung Electronics p.marc...@samsung.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] arm: mx6: Reduce SPL malloc pool size
On Fri, Nov 20, 2015 at 12:43 PM, Marek Vasut 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 > Cc: Stefano Babic > Cc: Tim Harvey > --- > 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 0x8820 > -#define CONFIG_SPL_BSS_MAX_SIZE0x10/* 1 MB */ > +#define CONFIG_SPL_BSS_MAX_SIZE0x10/* 1 MB */ > #define CONFIG_SYS_SPL_MALLOC_START0x8830 > -#define CONFIG_SYS_SPL_MALLOC_SIZE 0x320 /* 50 MB */ > +#define CONFIG_SYS_SPL_MALLOC_SIZE 0x10/* 1 MB */ > #define CONFIG_SYS_TEXT_BASE 0x8780 > #else > #define CONFIG_SPL_BSS_START_ADDR 0x1820 > #define CONFIG_SPL_BSS_MAX_SIZE0x10/* 1 MB */ > #define CONFIG_SYS_SPL_MALLOC_START0x1830 > -#define CONFIG_SYS_SPL_MALLOC_SIZE 0x320 /* 50 MB */ > +#define CONFIG_SYS_SPL_MALLOC_SIZE 0x10/* 1 MB */ > #define CONFIG_SYS_TEXT_BASE 0x1780 > #endif > #endif > -- > 2.1.4 > Acked-by: Tim Harvey thanks for dropping 2 secs off our time to boot! Tim ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] arm: mx6: Reduce SPL malloc pool size
On 20/11/2015 21:43, Marek Vasut 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. You're absolutely right ! Nice catch ! > > Signed-off-by: Marek Vasut > Cc: Stefano Babic > Cc: Tim Harvey > --- > 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 0x8820 > -#define CONFIG_SPL_BSS_MAX_SIZE0x10/* 1 MB */ > +#define CONFIG_SPL_BSS_MAX_SIZE0x10 /* 1 MB */ > #define CONFIG_SYS_SPL_MALLOC_START0x8830 > -#define CONFIG_SYS_SPL_MALLOC_SIZE 0x320 /* 50 MB */ > +#define CONFIG_SYS_SPL_MALLOC_SIZE 0x10 /* 1 MB */ > #define CONFIG_SYS_TEXT_BASE 0x8780 > #else > #define CONFIG_SPL_BSS_START_ADDR0x1820 > #define CONFIG_SPL_BSS_MAX_SIZE 0x10/* 1 MB */ > #define CONFIG_SYS_SPL_MALLOC_START 0x1830 > -#define CONFIG_SYS_SPL_MALLOC_SIZE 0x320 /* 50 MB */ > +#define CONFIG_SYS_SPL_MALLOC_SIZE 0x10/* 1 MB */ > #define CONFIG_SYS_TEXT_BASE 0x1780 > #endif > #endif > Tested-by: Stefano Babic Best regards, Stefano Babic -- = 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: sba...@denx.de = ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
