[U-Boot] [PATCH] malloc: Use malloc simple before malloc is fully initialized in memalign()

From: Simon Glass <sjg@chromium.org>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] malloc: Use malloc simple before malloc is fully initialized in memalign()
Date: Wed, 30 May 2018 13:18:27 -0600	[thread overview]
Message-ID: <CAPnjgZ0eYMU8sdDGA94gdpYWgh=dSaMkckzQp9pNz8TP0UVODg@mail.gmail.com> (raw)
In-Reply-To: <CAK7LNAQ3=YeRtmOqWEnWDCNYZCTxSDJ6_ga_+UBJQgOSvz24CQ@mail.gmail.com>

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