| Message ID | 1424430379-12599-5-git-send-email-p.marczak@samsung.com |
|---|---|
| State | Changes Requested |
| Headers | show |
Hi Przemyslaw, On Fri, 20 Feb 2015 12:06:17 +0100 Przemyslaw Marczak <p.marczak@samsung.com> wrote: > This commit introduces new config: CONFIG_SYS_MALLOC_INIT_SKIP_ZEROING. > > Before this change, the all amount of memory reserved for the malloc, > was set to zero in mem_malloc_init(). When the malloc reserved memory > exceeds few MiB, then the boot process can slow down. > > So enabling this config, is an option to reduce the boot time. > > This option can be enabled by Kconfig. > > Note: > After enable this option, only calloc() will return the pointer to zeroed > memory area. Previously, without this option, the memory pointed to untouched > malloc memory region, was filled with zeros. So it means, that code with > malloc() calls should be reexamined. > > Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com> > Reviewed-by: Simon Glass <sjg@chromium.org> > > --- > Changes v3: > - squash the commit with the Kconfig option > --- > Kconfig | 26 +++++++++++++++++++------- > common/dlmalloc.c | 10 +++++++--- > 2 files changed, 26 insertions(+), 10 deletions(-) > > diff --git a/Kconfig b/Kconfig > index 75bab7f..87d4daf 100644 > --- a/Kconfig > +++ b/Kconfig > @@ -76,13 +76,25 @@ config SYS_MALLOC_F_LEN > initial serial device and any others that are needed. > > menuconfig EXPERT > - bool "Configure standard U-Boot features (expert users)" > - help > - This option allows certain base U-Boot options and settings > - to be disabled or tweaked. This is for specialized > - environments which can tolerate a "non-standard" U-Boot. > - Only use this if you really know what you are doing. > - > + bool "Configure standard U-Boot features (expert users)" > + help > + This option allows certain base U-Boot options and settings > + to be disabled or tweaked. This is for specialized > + environments which can tolerate a "non-standard" U-Boot. > + Only use this if you really know what you are doing. > + > +if EXPERT > + config SYS_MALLOC_INIT_SKIP_ZEROING > + bool "Skip memset at malloc init (reduce boot time)" > + help > + This avoids zeroing memory reserved for malloc at malloc init. > + Significant boot time reduction is visible for configs in which > + CONFIG_SYS_MALLOC_LEN value, has more than few MiB. > + Useful for bzip2, bmp logo. > + Warning: > + When enabling this, please check if malloc calls, maybe > + should be replaced by calloc - if expects zeroed memory. > +endif > endmenu # General setup > > menu "Boot images" > diff --git a/common/dlmalloc.c b/common/dlmalloc.c > index 6453ee9..63f68ed 100644 > --- a/common/dlmalloc.c > +++ b/common/dlmalloc.c > @@ -1535,9 +1535,9 @@ void mem_malloc_init(ulong start, ulong size) > > debug("using memory %#lx-%#lx for malloc()\n", mem_malloc_start, > mem_malloc_end); > - > - memset((void *)mem_malloc_start, 0, size); > - > +#ifndef CONFIG_SYS_MALLOC_INIT_SKIP_ZEROING > + memset((void *)mem_malloc_start, 0x0, size); > +#endif > malloc_bin_reloc(); > } > > @@ -2948,10 +2948,12 @@ Void_t* cALLOc(n, elem_size) size_t n; size_t elem_size; > > > /* check if expand_top called, in which case don't need to clear */ > +#ifndef CONFIG_SYS_MALLOC_INIT_SKIP_ZEROING > #if MORECORE_CLEARS > mchunkptr oldtop = top; > INTERNAL_SIZE_T oldtopsize = chunksize(top); > #endif > +#endif > Void_t* mem = mALLOc (sz); > > if ((long)n < 0) return NULL; > @@ -2977,6 +2979,7 @@ Void_t* cALLOc(n, elem_size) size_t n; size_t elem_size; > > csz = chunksize(p); > > +#ifndef CONFIG_SYS_MALLOC_INIT_SKIP_ZEROING > #if MORECORE_CLEARS > if (p == oldtop && csz > oldtopsize) > { > @@ -2984,6 +2987,7 @@ Void_t* cALLOc(n, elem_size) size_t n; size_t elem_size; > csz = oldtopsize; > } > #endif > +#endif > > MALLOC_ZERO(mem, csz - SIZE_SZ); > return mem; You are adding only "ifndef" conditionals. IMHO, Isn't "#ifdef CONFIG_SYS_MALLOC_INIT_ZEROING" better? (Generally speaking, CONFIG options that disable a feature are not preferable.) You also need to add "default y" to the Kconfig and add "# CONIFG_SYS_MALLOC_INIT_ZEROING is not set" to your defconfig. Best Regards Masahiro Yamada
Hello, On 02/20/2015 01:52 PM, Masahiro Yamada wrote: > Hi Przemyslaw, > > > On Fri, 20 Feb 2015 12:06:17 +0100 > Przemyslaw Marczak <p.marczak@samsung.com> wrote: > >> This commit introduces new config: CONFIG_SYS_MALLOC_INIT_SKIP_ZEROING. >> >> Before this change, the all amount of memory reserved for the malloc, >> was set to zero in mem_malloc_init(). When the malloc reserved memory >> exceeds few MiB, then the boot process can slow down. >> >> So enabling this config, is an option to reduce the boot time. >> >> This option can be enabled by Kconfig. >> >> Note: >> After enable this option, only calloc() will return the pointer to zeroed >> memory area. Previously, without this option, the memory pointed to untouched >> malloc memory region, was filled with zeros. So it means, that code with >> malloc() calls should be reexamined. >> >> Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com> >> Reviewed-by: Simon Glass <sjg@chromium.org> >> >> --- >> Changes v3: >> - squash the commit with the Kconfig option >> --- >> Kconfig | 26 +++++++++++++++++++------- >> common/dlmalloc.c | 10 +++++++--- >> 2 files changed, 26 insertions(+), 10 deletions(-) >> >> diff --git a/Kconfig b/Kconfig >> index 75bab7f..87d4daf 100644 >> --- a/Kconfig >> +++ b/Kconfig >> @@ -76,13 +76,25 @@ config SYS_MALLOC_F_LEN >> initial serial device and any others that are needed. >> >> menuconfig EXPERT >> - bool "Configure standard U-Boot features (expert users)" >> - help >> - This option allows certain base U-Boot options and settings >> - to be disabled or tweaked. This is for specialized >> - environments which can tolerate a "non-standard" U-Boot. >> - Only use this if you really know what you are doing. >> - >> + bool "Configure standard U-Boot features (expert users)" >> + help >> + This option allows certain base U-Boot options and settings >> + to be disabled or tweaked. This is for specialized >> + environments which can tolerate a "non-standard" U-Boot. >> + Only use this if you really know what you are doing. >> + >> +if EXPERT >> + config SYS_MALLOC_INIT_SKIP_ZEROING >> + bool "Skip memset at malloc init (reduce boot time)" >> + help >> + This avoids zeroing memory reserved for malloc at malloc init. >> + Significant boot time reduction is visible for configs in which >> + CONFIG_SYS_MALLOC_LEN value, has more than few MiB. >> + Useful for bzip2, bmp logo. >> + Warning: >> + When enabling this, please check if malloc calls, maybe >> + should be replaced by calloc - if expects zeroed memory. >> +endif >> endmenu # General setup >> >> menu "Boot images" >> diff --git a/common/dlmalloc.c b/common/dlmalloc.c >> index 6453ee9..63f68ed 100644 >> --- a/common/dlmalloc.c >> +++ b/common/dlmalloc.c >> @@ -1535,9 +1535,9 @@ void mem_malloc_init(ulong start, ulong size) >> >> debug("using memory %#lx-%#lx for malloc()\n", mem_malloc_start, >> mem_malloc_end); >> - >> - memset((void *)mem_malloc_start, 0, size); >> - >> +#ifndef CONFIG_SYS_MALLOC_INIT_SKIP_ZEROING >> + memset((void *)mem_malloc_start, 0x0, size); >> +#endif >> malloc_bin_reloc(); >> } >> >> @@ -2948,10 +2948,12 @@ Void_t* cALLOc(n, elem_size) size_t n; size_t elem_size; >> >> >> /* check if expand_top called, in which case don't need to clear */ >> +#ifndef CONFIG_SYS_MALLOC_INIT_SKIP_ZEROING >> #if MORECORE_CLEARS >> mchunkptr oldtop = top; >> INTERNAL_SIZE_T oldtopsize = chunksize(top); >> #endif >> +#endif >> Void_t* mem = mALLOc (sz); >> >> if ((long)n < 0) return NULL; >> @@ -2977,6 +2979,7 @@ Void_t* cALLOc(n, elem_size) size_t n; size_t elem_size; >> >> csz = chunksize(p); >> >> +#ifndef CONFIG_SYS_MALLOC_INIT_SKIP_ZEROING >> #if MORECORE_CLEARS >> if (p == oldtop && csz > oldtopsize) >> { >> @@ -2984,6 +2987,7 @@ Void_t* cALLOc(n, elem_size) size_t n; size_t elem_size; >> csz = oldtopsize; >> } >> #endif >> +#endif >> >> MALLOC_ZERO(mem, csz - SIZE_SZ); >> return mem; > > > You are adding only "ifndef" conditionals. > > IMHO, > Isn't "#ifdef CONFIG_SYS_MALLOC_INIT_ZEROING" better? > (Generally speaking, CONFIG options that disable a feature are not preferable.) > > You also need to add "default y" to the Kconfig > and add "# CONIFG_SYS_MALLOC_INIT_ZEROING is not set" to your defconfig. > > > Best Regards > Masahiro Yamada > Ok, will change this to positive. Best regards,
diff --git a/Kconfig b/Kconfig index 75bab7f..87d4daf 100644 --- a/Kconfig +++ b/Kconfig @@ -76,13 +76,25 @@ config SYS_MALLOC_F_LEN initial serial device and any others that are needed. menuconfig EXPERT - bool "Configure standard U-Boot features (expert users)" - help - This option allows certain base U-Boot options and settings - to be disabled or tweaked. This is for specialized - environments which can tolerate a "non-standard" U-Boot. - Only use this if you really know what you are doing. - + bool "Configure standard U-Boot features (expert users)" + help + This option allows certain base U-Boot options and settings + to be disabled or tweaked. This is for specialized + environments which can tolerate a "non-standard" U-Boot. + Only use this if you really know what you are doing. + +if EXPERT + config SYS_MALLOC_INIT_SKIP_ZEROING + bool "Skip memset at malloc init (reduce boot time)" + help + This avoids zeroing memory reserved for malloc at malloc init. + Significant boot time reduction is visible for configs in which + CONFIG_SYS_MALLOC_LEN value, has more than few MiB. + Useful for bzip2, bmp logo. + Warning: + When enabling this, please check if malloc calls, maybe + should be replaced by calloc - if expects zeroed memory. +endif endmenu # General setup menu "Boot images" diff --git a/common/dlmalloc.c b/common/dlmalloc.c index 6453ee9..63f68ed 100644 --- a/common/dlmalloc.c +++ b/common/dlmalloc.c @@ -1535,9 +1535,9 @@ void mem_malloc_init(ulong start, ulong size) debug("using memory %#lx-%#lx for malloc()\n", mem_malloc_start, mem_malloc_end); - - memset((void *)mem_malloc_start, 0, size); - +#ifndef CONFIG_SYS_MALLOC_INIT_SKIP_ZEROING + memset((void *)mem_malloc_start, 0x0, size); +#endif malloc_bin_reloc(); } @@ -2948,10 +2948,12 @@ Void_t* cALLOc(n, elem_size) size_t n; size_t elem_size; /* check if expand_top called, in which case don't need to clear */ +#ifndef CONFIG_SYS_MALLOC_INIT_SKIP_ZEROING #if MORECORE_CLEARS mchunkptr oldtop = top; INTERNAL_SIZE_T oldtopsize = chunksize(top); #endif +#endif Void_t* mem = mALLOc (sz); if ((long)n < 0) return NULL; @@ -2977,6 +2979,7 @@ Void_t* cALLOc(n, elem_size) size_t n; size_t elem_size; csz = chunksize(p); +#ifndef CONFIG_SYS_MALLOC_INIT_SKIP_ZEROING #if MORECORE_CLEARS if (p == oldtop && csz > oldtopsize) { @@ -2984,6 +2987,7 @@ Void_t* cALLOc(n, elem_size) size_t n; size_t elem_size; csz = oldtopsize; } #endif +#endif MALLOC_ZERO(mem, csz - SIZE_SZ); return mem;