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

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

[Marek Vasut]

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>

-- 
Best regards,
Marek Vasut

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

[Ley Foon Tan]

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)
+	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

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

[Simon Glass]

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

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

[Ley Foon Tan]

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

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

[Simon Glass]

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

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

[Ley Foon Tan]

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

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

[Simon Glass]

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

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

[Ley Foon Tan]

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

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

[Simon Glass]

+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

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

[Tom Rini]

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!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180528/46a061ea/attachment.sig>

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

[Masahiro Yamada]

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.



-- 
Best Regards
Masahiro Yamada

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

[Simon Glass]

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