| Message ID | 1526637792-33233-1-git-send-email-ley.foon.tan@intel.com |
|---|---|
| State | Accepted |
| Commit | ee038c58d5196dc2eb2be7e08e766c50a7bc2619 |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [U-Boot] malloc: Use malloc simple before malloc is fully initialized in memalign() | expand |
On 05/18/2018 12:03 PM, Ley Foon Tan wrote: > Follow implementation in mALLOc(). Check GD_FLG_FULL_MALLOC_INIT flag and use > malloc_simple if GD_FLG_FULL_MALLOC_INIT is unset. Adjust the malloc bytes > to align with the requested alignment. > > The original memalign() function will access mchunkptr struct to adjust the > alignment if there is misalignment happen, but mchunkptr struct is not being > initialized before full malloc is initialized. This cause the system crash. > > Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com> Reviewed-by: Marek Vasut <marek.vasut@gmail.com>
Hi Ley, On 18 May 2018 at 04:03, Ley Foon Tan <ley.foon.tan@intel.com> wrote: > Follow implementation in mALLOc(). Check GD_FLG_FULL_MALLOC_INIT flag and use > malloc_simple if GD_FLG_FULL_MALLOC_INIT is unset. Adjust the malloc bytes > to align with the requested alignment. > > The original memalign() function will access mchunkptr struct to adjust the > alignment if there is misalignment happen, but mchunkptr struct is not being > initialized before full malloc is initialized. This cause the system crash. > > Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com> > --- > common/dlmalloc.c | 7 +++++++ > 1 files changed, 7 insertions(+), 0 deletions(-) > > diff --git a/common/dlmalloc.c b/common/dlmalloc.c > index b395eef..edaad29 100644 > --- a/common/dlmalloc.c > +++ b/common/dlmalloc.c > @@ -1891,6 +1891,13 @@ Void_t* mEMALIGn(alignment, bytes) size_t alignment; size_t bytes; > > if ((long)bytes < 0) return NULL; > > +#if CONFIG_VAL(SYS_MALLOC_F_LEN) How about: if (IS_ENABLED(CONFIG_SYS_MALLOC_F)) ? > + if (!(gd->flags & GD_FLG_FULL_MALLOC_INIT)) { > + nb = roundup(bytes, alignment); > + return malloc_simple(nb); > + } > +#endif > + > /* If need less alignment than we give anyway, just relay to malloc */ > > if (alignment <= MALLOC_ALIGNMENT) return mALLOc(bytes); > -- > 1.7.1 > Regards, Simon
On Sat, May 19, 2018 at 10:37 PM, Simon Glass <sjg@chromium.org> wrote: > Hi Ley, > > On 18 May 2018 at 04:03, Ley Foon Tan <ley.foon.tan@intel.com> wrote: >> Follow implementation in mALLOc(). Check GD_FLG_FULL_MALLOC_INIT flag and use >> malloc_simple if GD_FLG_FULL_MALLOC_INIT is unset. Adjust the malloc bytes >> to align with the requested alignment. >> >> The original memalign() function will access mchunkptr struct to adjust the >> alignment if there is misalignment happen, but mchunkptr struct is not being >> initialized before full malloc is initialized. This cause the system crash. >> >> Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com> >> --- >> common/dlmalloc.c | 7 +++++++ >> 1 files changed, 7 insertions(+), 0 deletions(-) >> >> diff --git a/common/dlmalloc.c b/common/dlmalloc.c >> index b395eef..edaad29 100644 >> --- a/common/dlmalloc.c >> +++ b/common/dlmalloc.c >> @@ -1891,6 +1891,13 @@ Void_t* mEMALIGn(alignment, bytes) size_t alignment; size_t bytes; >> >> if ((long)bytes < 0) return NULL; >> >> +#if CONFIG_VAL(SYS_MALLOC_F_LEN) > > How about: > > if (IS_ENABLED(CONFIG_SYS_MALLOC_F)) I think this is the reason it uses #if CONFIG_VAL(SYS_MALLOC_F_LEN), same for malloc(). "spl: make SPL and normal u-boot stage use independent SYS_MALLOC_F_LEN" http://git.denx.de/?p=u-boot.git;a=commit;h=f1896c45cb2f7d8dbed27e784a6459a129fc0762 > > ? > >> + if (!(gd->flags & GD_FLG_FULL_MALLOC_INIT)) { >> + nb = roundup(bytes, alignment); >> + return malloc_simple(nb); >> + } >> +#endif >> + >> /* If need less alignment than we give anyway, just relay to malloc */ >> >> if (alignment <= MALLOC_ALIGNMENT) return mALLOc(bytes); >> -- >> 1.7.1 >> > > Regards, > Simon Regards Ley Foon
Hi, On 23 May 2018 at 00:32, Ley Foon Tan <lftan.linux@gmail.com> wrote: > On Sat, May 19, 2018 at 10:37 PM, Simon Glass <sjg@chromium.org> wrote: >> Hi Ley, >> >> On 18 May 2018 at 04:03, Ley Foon Tan <ley.foon.tan@intel.com> wrote: >>> Follow implementation in mALLOc(). Check GD_FLG_FULL_MALLOC_INIT flag and use >>> malloc_simple if GD_FLG_FULL_MALLOC_INIT is unset. Adjust the malloc bytes >>> to align with the requested alignment. >>> >>> The original memalign() function will access mchunkptr struct to adjust the >>> alignment if there is misalignment happen, but mchunkptr struct is not being >>> initialized before full malloc is initialized. This cause the system crash. >>> >>> Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com> >>> --- >>> common/dlmalloc.c | 7 +++++++ >>> 1 files changed, 7 insertions(+), 0 deletions(-) >>> >>> diff --git a/common/dlmalloc.c b/common/dlmalloc.c >>> index b395eef..edaad29 100644 >>> --- a/common/dlmalloc.c >>> +++ b/common/dlmalloc.c >>> @@ -1891,6 +1891,13 @@ Void_t* mEMALIGn(alignment, bytes) size_t alignment; size_t bytes; >>> >>> if ((long)bytes < 0) return NULL; >>> >>> +#if CONFIG_VAL(SYS_MALLOC_F_LEN) >> >> How about: >> >> if (IS_ENABLED(CONFIG_SYS_MALLOC_F)) > > I think this is the reason it uses #if CONFIG_VAL(SYS_MALLOC_F_LEN), > same for malloc(). > > "spl: make SPL and normal u-boot stage use independent SYS_MALLOC_F_LEN" > > http://git.denx.de/?p=u-boot.git;a=commit;h=f1896c45cb2f7d8dbed27e784a6459a129fc0762 So how about if (CONFIG_IS_ENABLED(SYS_MALLOC_F_LEN) Or you could use #if if you need to To me it seems better to use the setting itself (i.e. whether the pre-reloc malloc() is enabled) rather than one of its parameters (the size of the region). > > >> >> ? >> >>> + if (!(gd->flags & GD_FLG_FULL_MALLOC_INIT)) { >>> + nb = roundup(bytes, alignment); >>> + return malloc_simple(nb); >>> + } >>> +#endif >>> + >>> /* If need less alignment than we give anyway, just relay to malloc */ >>> >>> if (alignment <= MALLOC_ALIGNMENT) return mALLOc(bytes); >>> -- >>> 1.7.1 Regards, Simon
On Thu, May 24, 2018 at 12:33 AM, Simon Glass <sjg@chromium.org> wrote: > Hi, > > On 23 May 2018 at 00:32, Ley Foon Tan <lftan.linux@gmail.com> wrote: >> On Sat, May 19, 2018 at 10:37 PM, Simon Glass <sjg@chromium.org> wrote: >>> Hi Ley, >>> >>> On 18 May 2018 at 04:03, Ley Foon Tan <ley.foon.tan@intel.com> wrote: >>>> Follow implementation in mALLOc(). Check GD_FLG_FULL_MALLOC_INIT flag and use >>>> malloc_simple if GD_FLG_FULL_MALLOC_INIT is unset. Adjust the malloc bytes >>>> to align with the requested alignment. >>>> >>>> The original memalign() function will access mchunkptr struct to adjust the >>>> alignment if there is misalignment happen, but mchunkptr struct is not being >>>> initialized before full malloc is initialized. This cause the system crash. >>>> >>>> Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com> >>>> --- >>>> common/dlmalloc.c | 7 +++++++ >>>> 1 files changed, 7 insertions(+), 0 deletions(-) >>>> >>>> diff --git a/common/dlmalloc.c b/common/dlmalloc.c >>>> index b395eef..edaad29 100644 >>>> --- a/common/dlmalloc.c >>>> +++ b/common/dlmalloc.c >>>> @@ -1891,6 +1891,13 @@ Void_t* mEMALIGn(alignment, bytes) size_t alignment; size_t bytes; >>>> >>>> if ((long)bytes < 0) return NULL; >>>> >>>> +#if CONFIG_VAL(SYS_MALLOC_F_LEN) >>> >>> How about: >>> >>> if (IS_ENABLED(CONFIG_SYS_MALLOC_F)) >> >> I think this is the reason it uses #if CONFIG_VAL(SYS_MALLOC_F_LEN), >> same for malloc(). >> >> "spl: make SPL and normal u-boot stage use independent SYS_MALLOC_F_LEN" >> >> http://git.denx.de/?p=u-boot.git;a=commit;h=f1896c45cb2f7d8dbed27e784a6459a129fc0762 > > So how about > > if (CONFIG_IS_ENABLED(SYS_MALLOC_F_LEN) > > Or you could use #if if you need to Tested both #if (CONFIG_IS_ENABLED(SYS_MALLOC_F_LEN)) and if (CONFIG_IS_ENABLED(SYS_MALLOC_F_LEN)), both are not working. It will not go into this code. > > To me it seems better to use the setting itself (i.e. whether the > pre-reloc malloc() is enabled) rather than one of its parameters (the > size of the region). > >> >> >>> >>> ? >>> >>>> + if (!(gd->flags & GD_FLG_FULL_MALLOC_INIT)) { >>>> + nb = roundup(bytes, alignment); >>>> + return malloc_simple(nb); >>>> + } >>>> +#endif >>>> + >>>> /* If need less alignment than we give anyway, just relay to malloc */ >>>> >>>> if (alignment <= MALLOC_ALIGNMENT) return mALLOc(bytes); >>>> -- >>>> 1.7.1 > Regards, > Simon Regards Ley Foon
Hi, On 24 May 2018 at 21:24, Ley Foon Tan <lftan.linux@gmail.com> wrote: > > On Thu, May 24, 2018 at 12:33 AM, Simon Glass <sjg@chromium.org> wrote: > > Hi, > > > > On 23 May 2018 at 00:32, Ley Foon Tan <lftan.linux@gmail.com> wrote: > >> On Sat, May 19, 2018 at 10:37 PM, Simon Glass <sjg@chromium.org> wrote: > >>> Hi Ley, > >>> > >>> On 18 May 2018 at 04:03, Ley Foon Tan <ley.foon.tan@intel.com> wrote: > >>>> Follow implementation in mALLOc(). Check GD_FLG_FULL_MALLOC_INIT flag and use > >>>> malloc_simple if GD_FLG_FULL_MALLOC_INIT is unset. Adjust the malloc bytes > >>>> to align with the requested alignment. > >>>> > >>>> The original memalign() function will access mchunkptr struct to adjust the > >>>> alignment if there is misalignment happen, but mchunkptr struct is not being > >>>> initialized before full malloc is initialized. This cause the system crash. > >>>> > >>>> Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com> > >>>> --- > >>>> common/dlmalloc.c | 7 +++++++ > >>>> 1 files changed, 7 insertions(+), 0 deletions(-) > >>>> > >>>> diff --git a/common/dlmalloc.c b/common/dlmalloc.c > >>>> index b395eef..edaad29 100644 > >>>> --- a/common/dlmalloc.c > >>>> +++ b/common/dlmalloc.c > >>>> @@ -1891,6 +1891,13 @@ Void_t* mEMALIGn(alignment, bytes) size_t alignment; size_t bytes; > >>>> > >>>> if ((long)bytes < 0) return NULL; > >>>> > >>>> +#if CONFIG_VAL(SYS_MALLOC_F_LEN) > >>> > >>> How about: > >>> > >>> if (IS_ENABLED(CONFIG_SYS_MALLOC_F)) > >> > >> I think this is the reason it uses #if CONFIG_VAL(SYS_MALLOC_F_LEN), > >> same for malloc(). > >> > >> "spl: make SPL and normal u-boot stage use independent SYS_MALLOC_F_LEN" > >> > >> http://git.denx.de/?p=u-boot.git;a=commit;h=f1896c45cb2f7d8dbed27e784a6459a129fc0762 > > > > So how about > > > > if (CONFIG_IS_ENABLED(SYS_MALLOC_F_LEN) > > > > Or you could use #if if you need to > > Tested both #if (CONFIG_IS_ENABLED(SYS_MALLOC_F_LEN)) and if > (CONFIG_IS_ENABLED(SYS_MALLOC_F_LEN)), both are not working. Sorry I mean CONFIG_IS_ENABLED(SYS_MALLOC_F) That tells you whether the feature is enabled in U-Boot or SPL. > > It will not go into this code. > > > > > > To me it seems better to use the setting itself (i.e. whether the > > pre-reloc malloc() is enabled) rather than one of its parameters (the > > size of the region). [..] Regards, Simon
On Fri, May 25, 2018 at 11:33 AM, Simon Glass <sjg@chromium.org> wrote: > Hi, > > On 24 May 2018 at 21:24, Ley Foon Tan <lftan.linux@gmail.com> wrote: >> >> On Thu, May 24, 2018 at 12:33 AM, Simon Glass <sjg@chromium.org> wrote: >> > Hi, >> > >> > On 23 May 2018 at 00:32, Ley Foon Tan <lftan.linux@gmail.com> wrote: >> >> On Sat, May 19, 2018 at 10:37 PM, Simon Glass <sjg@chromium.org> wrote: >> >>> Hi Ley, >> >>> >> >>> On 18 May 2018 at 04:03, Ley Foon Tan <ley.foon.tan@intel.com> wrote: >> >>>> Follow implementation in mALLOc(). Check GD_FLG_FULL_MALLOC_INIT flag and use >> >>>> malloc_simple if GD_FLG_FULL_MALLOC_INIT is unset. Adjust the malloc bytes >> >>>> to align with the requested alignment. >> >>>> >> >>>> The original memalign() function will access mchunkptr struct to adjust the >> >>>> alignment if there is misalignment happen, but mchunkptr struct is not being >> >>>> initialized before full malloc is initialized. This cause the system crash. >> >>>> >> >>>> Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com> >> >>>> --- >> >>>> common/dlmalloc.c | 7 +++++++ >> >>>> 1 files changed, 7 insertions(+), 0 deletions(-) >> >>>> >> >>>> diff --git a/common/dlmalloc.c b/common/dlmalloc.c >> >>>> index b395eef..edaad29 100644 >> >>>> --- a/common/dlmalloc.c >> >>>> +++ b/common/dlmalloc.c >> >>>> @@ -1891,6 +1891,13 @@ Void_t* mEMALIGn(alignment, bytes) size_t alignment; size_t bytes; >> >>>> >> >>>> if ((long)bytes < 0) return NULL; >> >>>> >> >>>> +#if CONFIG_VAL(SYS_MALLOC_F_LEN) >> >>> >> >>> How about: >> >>> >> >>> if (IS_ENABLED(CONFIG_SYS_MALLOC_F)) >> >> >> >> I think this is the reason it uses #if CONFIG_VAL(SYS_MALLOC_F_LEN), >> >> same for malloc(). >> >> >> >> "spl: make SPL and normal u-boot stage use independent SYS_MALLOC_F_LEN" >> >> >> >> http://git.denx.de/?p=u-boot.git;a=commit;h=f1896c45cb2f7d8dbed27e784a6459a129fc0762 >> > >> > So how about >> > >> > if (CONFIG_IS_ENABLED(SYS_MALLOC_F_LEN) >> > >> > Or you could use #if if you need to >> >> Tested both #if (CONFIG_IS_ENABLED(SYS_MALLOC_F_LEN)) and if >> (CONFIG_IS_ENABLED(SYS_MALLOC_F_LEN)), both are not working. > > Sorry I mean > > CONFIG_IS_ENABLED(SYS_MALLOC_F) > > That tells you whether the feature is enabled in U-Boot or SPL. #if CONFIG_IS_ENABLED(SYS_MALLOC_F) if not working in SPL build. CONFIG_IS_ENABLED() expects config with "y" or "m", but SPL config is with "1". Need to use #ifdef CONFIG_SYS_MALLOC_F. Do you want to change to this? Regards Ley Foon
+Masahiro On 25 May 2018 at 02:50, Ley Foon Tan <lftan.linux@gmail.com> wrote: > > On Fri, May 25, 2018 at 11:33 AM, Simon Glass <sjg@chromium.org> wrote: > > Hi, > > > > On 24 May 2018 at 21:24, Ley Foon Tan <lftan.linux@gmail.com> wrote: > >> > >> On Thu, May 24, 2018 at 12:33 AM, Simon Glass <sjg@chromium.org> wrote: > >> > Hi, > >> > > >> > On 23 May 2018 at 00:32, Ley Foon Tan <lftan.linux@gmail.com> wrote: > >> >> On Sat, May 19, 2018 at 10:37 PM, Simon Glass <sjg@chromium.org> wrote: > >> >>> Hi Ley, > >> >>> > >> >>> On 18 May 2018 at 04:03, Ley Foon Tan <ley.foon.tan@intel.com> wrote: > >> >>>> Follow implementation in mALLOc(). Check GD_FLG_FULL_MALLOC_INIT flag and use > >> >>>> malloc_simple if GD_FLG_FULL_MALLOC_INIT is unset. Adjust the malloc bytes > >> >>>> to align with the requested alignment. > >> >>>> > >> >>>> The original memalign() function will access mchunkptr struct to adjust the > >> >>>> alignment if there is misalignment happen, but mchunkptr struct is not being > >> >>>> initialized before full malloc is initialized. This cause the system crash. > >> >>>> > >> >>>> Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com> > >> >>>> --- > >> >>>> common/dlmalloc.c | 7 +++++++ > >> >>>> 1 files changed, 7 insertions(+), 0 deletions(-) > >> >>>> > >> >>>> diff --git a/common/dlmalloc.c b/common/dlmalloc.c > >> >>>> index b395eef..edaad29 100644 > >> >>>> --- a/common/dlmalloc.c > >> >>>> +++ b/common/dlmalloc.c > >> >>>> @@ -1891,6 +1891,13 @@ Void_t* mEMALIGn(alignment, bytes) size_t alignment; size_t bytes; > >> >>>> > >> >>>> if ((long)bytes < 0) return NULL; > >> >>>> > >> >>>> +#if CONFIG_VAL(SYS_MALLOC_F_LEN) > >> >>> > >> >>> How about: > >> >>> > >> >>> if (IS_ENABLED(CONFIG_SYS_MALLOC_F)) > >> >> > >> >> I think this is the reason it uses #if CONFIG_VAL(SYS_MALLOC_F_LEN), > >> >> same for malloc(). > >> >> > >> >> "spl: make SPL and normal u-boot stage use independent SYS_MALLOC_F_LEN" > >> >> > >> >> http://git.denx.de/?p=u-boot.git;a=commit;h=f1896c45cb2f7d8dbed27e784a6459a129fc0762 > >> > > >> > So how about > >> > > >> > if (CONFIG_IS_ENABLED(SYS_MALLOC_F_LEN) > >> > > >> > Or you could use #if if you need to > >> > >> Tested both #if (CONFIG_IS_ENABLED(SYS_MALLOC_F_LEN)) and if > >> (CONFIG_IS_ENABLED(SYS_MALLOC_F_LEN)), both are not working. > > > > Sorry I mean > > > > CONFIG_IS_ENABLED(SYS_MALLOC_F) > > > > That tells you whether the feature is enabled in U-Boot or SPL. > > #if CONFIG_IS_ENABLED(SYS_MALLOC_F) if not working in SPL build. > CONFIG_IS_ENABLED() expects config with "y" or "m", but SPL config is > with "1". That seems very strange, since it defeats the purpose of the macro. Masahiro, do you know what going on here? > Need to use #ifdef CONFIG_SYS_MALLOC_F. Do you want to change to this? Regards, Simon
On Fri, May 18, 2018 at 06:03:12PM +0800, Ley Foon Tan wrote: > Follow implementation in mALLOc(). Check GD_FLG_FULL_MALLOC_INIT flag and use > malloc_simple if GD_FLG_FULL_MALLOC_INIT is unset. Adjust the malloc bytes > to align with the requested alignment. > > The original memalign() function will access mchunkptr struct to adjust the > alignment if there is misalignment happen, but mchunkptr struct is not being > initialized before full malloc is initialized. This cause the system crash. > > Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com> > Reviewed-by: Marek Vasut <marek.vasut@gmail.com> Applied to u-boot/master, thanks!
2018-05-26 11:06 GMT+09:00 Simon Glass <sjg@chromium.org>: > +Masahiro > > On 25 May 2018 at 02:50, Ley Foon Tan <lftan.linux@gmail.com> wrote: >> >> On Fri, May 25, 2018 at 11:33 AM, Simon Glass <sjg@chromium.org> wrote: >> > Hi, >> > >> > On 24 May 2018 at 21:24, Ley Foon Tan <lftan.linux@gmail.com> wrote: >> >> >> >> On Thu, May 24, 2018 at 12:33 AM, Simon Glass <sjg@chromium.org> wrote: >> >> > Hi, >> >> > >> >> > On 23 May 2018 at 00:32, Ley Foon Tan <lftan.linux@gmail.com> wrote: >> >> >> On Sat, May 19, 2018 at 10:37 PM, Simon Glass <sjg@chromium.org> wrote: >> >> >>> Hi Ley, >> >> >>> >> >> >>> On 18 May 2018 at 04:03, Ley Foon Tan <ley.foon.tan@intel.com> wrote: >> >> >>>> Follow implementation in mALLOc(). Check GD_FLG_FULL_MALLOC_INIT flag and use >> >> >>>> malloc_simple if GD_FLG_FULL_MALLOC_INIT is unset. Adjust the malloc bytes >> >> >>>> to align with the requested alignment. >> >> >>>> >> >> >>>> The original memalign() function will access mchunkptr struct to adjust the >> >> >>>> alignment if there is misalignment happen, but mchunkptr struct is not being >> >> >>>> initialized before full malloc is initialized. This cause the system crash. >> >> >>>> >> >> >>>> Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com> >> >> >>>> --- >> >> >>>> common/dlmalloc.c | 7 +++++++ >> >> >>>> 1 files changed, 7 insertions(+), 0 deletions(-) >> >> >>>> >> >> >>>> diff --git a/common/dlmalloc.c b/common/dlmalloc.c >> >> >>>> index b395eef..edaad29 100644 >> >> >>>> --- a/common/dlmalloc.c >> >> >>>> +++ b/common/dlmalloc.c >> >> >>>> @@ -1891,6 +1891,13 @@ Void_t* mEMALIGn(alignment, bytes) size_t alignment; size_t bytes; >> >> >>>> >> >> >>>> if ((long)bytes < 0) return NULL; >> >> >>>> >> >> >>>> +#if CONFIG_VAL(SYS_MALLOC_F_LEN) >> >> >>> >> >> >>> How about: >> >> >>> >> >> >>> if (IS_ENABLED(CONFIG_SYS_MALLOC_F)) >> >> >> >> >> >> I think this is the reason it uses #if CONFIG_VAL(SYS_MALLOC_F_LEN), >> >> >> same for malloc(). >> >> >> >> >> >> "spl: make SPL and normal u-boot stage use independent SYS_MALLOC_F_LEN" >> >> >> >> >> >> http://git.denx.de/?p=u-boot.git;a=commit;h=f1896c45cb2f7d8dbed27e784a6459a129fc0762 >> >> > >> >> > So how about >> >> > >> >> > if (CONFIG_IS_ENABLED(SYS_MALLOC_F_LEN) >> >> > >> >> > Or you could use #if if you need to >> >> >> >> Tested both #if (CONFIG_IS_ENABLED(SYS_MALLOC_F_LEN)) and if >> >> (CONFIG_IS_ENABLED(SYS_MALLOC_F_LEN)), both are not working. >> > >> > Sorry I mean >> > >> > CONFIG_IS_ENABLED(SYS_MALLOC_F) >> > >> > That tells you whether the feature is enabled in U-Boot or SPL. >> >> #if CONFIG_IS_ENABLED(SYS_MALLOC_F) if not working in SPL build. >> CONFIG_IS_ENABLED() expects config with "y" or "m", but SPL config is >> with "1". > > That seems very strange, since it defeats the purpose of the macro. > > Masahiro, do you know what going on here? CONFIG_IS_ENABLED(SYS_MALLOC_F) evaluates SYS_MALLOC_F, or SPL_SYS_MALLOC_F depending on which image is being built. I see SPL_SYS_MALLOC_F_LEN in Kconfig, but do not see SPL_SYS_MALLOC_F at all.
Hi Masahiro, On 28 May 2018 at 23:16, Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > 2018-05-26 11:06 GMT+09:00 Simon Glass <sjg@chromium.org>: >> +Masahiro >> >> On 25 May 2018 at 02:50, Ley Foon Tan <lftan.linux@gmail.com> wrote: >>> >>> On Fri, May 25, 2018 at 11:33 AM, Simon Glass <sjg@chromium.org> wrote: >>> > Hi, >>> > >>> > On 24 May 2018 at 21:24, Ley Foon Tan <lftan.linux@gmail.com> wrote: >>> >> >>> >> On Thu, May 24, 2018 at 12:33 AM, Simon Glass <sjg@chromium.org> wrote: >>> >> > Hi, >>> >> > >>> >> > On 23 May 2018 at 00:32, Ley Foon Tan <lftan.linux@gmail.com> wrote: >>> >> >> On Sat, May 19, 2018 at 10:37 PM, Simon Glass <sjg@chromium.org> wrote: >>> >> >>> Hi Ley, >>> >> >>> >>> >> >>> On 18 May 2018 at 04:03, Ley Foon Tan <ley.foon.tan@intel.com> wrote: >>> >> >>>> Follow implementation in mALLOc(). Check GD_FLG_FULL_MALLOC_INIT flag and use >>> >> >>>> malloc_simple if GD_FLG_FULL_MALLOC_INIT is unset. Adjust the malloc bytes >>> >> >>>> to align with the requested alignment. >>> >> >>>> >>> >> >>>> The original memalign() function will access mchunkptr struct to adjust the >>> >> >>>> alignment if there is misalignment happen, but mchunkptr struct is not being >>> >> >>>> initialized before full malloc is initialized. This cause the system crash. >>> >> >>>> >>> >> >>>> Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com> >>> >> >>>> --- >>> >> >>>> common/dlmalloc.c | 7 +++++++ >>> >> >>>> 1 files changed, 7 insertions(+), 0 deletions(-) >>> >> >>>> >>> >> >>>> diff --git a/common/dlmalloc.c b/common/dlmalloc.c >>> >> >>>> index b395eef..edaad29 100644 >>> >> >>>> --- a/common/dlmalloc.c >>> >> >>>> +++ b/common/dlmalloc.c >>> >> >>>> @@ -1891,6 +1891,13 @@ Void_t* mEMALIGn(alignment, bytes) size_t alignment; size_t bytes; >>> >> >>>> >>> >> >>>> if ((long)bytes < 0) return NULL; >>> >> >>>> >>> >> >>>> +#if CONFIG_VAL(SYS_MALLOC_F_LEN) >>> >> >>> >>> >> >>> How about: >>> >> >>> >>> >> >>> if (IS_ENABLED(CONFIG_SYS_MALLOC_F)) >>> >> >> >>> >> >> I think this is the reason it uses #if CONFIG_VAL(SYS_MALLOC_F_LEN), >>> >> >> same for malloc(). >>> >> >> >>> >> >> "spl: make SPL and normal u-boot stage use independent SYS_MALLOC_F_LEN" >>> >> >> >>> >> >> http://git.denx.de/?p=u-boot.git;a=commit;h=f1896c45cb2f7d8dbed27e784a6459a129fc0762 >>> >> > >>> >> > So how about >>> >> > >>> >> > if (CONFIG_IS_ENABLED(SYS_MALLOC_F_LEN) >>> >> > >>> >> > Or you could use #if if you need to >>> >> >>> >> Tested both #if (CONFIG_IS_ENABLED(SYS_MALLOC_F_LEN)) and if >>> >> (CONFIG_IS_ENABLED(SYS_MALLOC_F_LEN)), both are not working. >>> > >>> > Sorry I mean >>> > >>> > CONFIG_IS_ENABLED(SYS_MALLOC_F) >>> > >>> > That tells you whether the feature is enabled in U-Boot or SPL. >>> >>> #if CONFIG_IS_ENABLED(SYS_MALLOC_F) if not working in SPL build. >>> CONFIG_IS_ENABLED() expects config with "y" or "m", but SPL config is >>> with "1". >> >> That seems very strange, since it defeats the purpose of the macro. >> >> Masahiro, do you know what going on here? > > > CONFIG_IS_ENABLED(SYS_MALLOC_F) evaluates > SYS_MALLOC_F, or SPL_SYS_MALLOC_F > depending on which image is being built. > > > I see SPL_SYS_MALLOC_F_LEN in Kconfig, > but do not see SPL_SYS_MALLOC_F at all. OK, well that explains the problem, thanks. Regards, Simon
diff --git a/common/dlmalloc.c b/common/dlmalloc.c index b395eef..edaad29 100644 --- a/common/dlmalloc.c +++ b/common/dlmalloc.c @@ -1891,6 +1891,13 @@ Void_t* mEMALIGn(alignment, bytes) size_t alignment; size_t bytes; if ((long)bytes < 0) return NULL; +#if CONFIG_VAL(SYS_MALLOC_F_LEN) + if (!(gd->flags & GD_FLG_FULL_MALLOC_INIT)) { + nb = roundup(bytes, alignment); + return malloc_simple(nb); + } +#endif + /* If need less alignment than we give anyway, just relay to malloc */ if (alignment <= MALLOC_ALIGNMENT) return mALLOc(bytes);
Follow implementation in mALLOc(). Check GD_FLG_FULL_MALLOC_INIT flag and use malloc_simple if GD_FLG_FULL_MALLOC_INIT is unset. Adjust the malloc bytes to align with the requested alignment. The original memalign() function will access mchunkptr struct to adjust the alignment if there is misalignment happen, but mchunkptr struct is not being initialized before full malloc is initialized. This cause the system crash. Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com> --- common/dlmalloc.c | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-)