[U-Boot] [U-Boot, v3, 1/2] fs: fat: dynamically allocate memory for temporary buffer

[Alexander Graf]

On 21.02.19 09:49, Marek Vasut wrote:
> On 2/21/19 9:44 AM, Alexander Graf wrote:
>>
>>
>> On 21.02.19 09:41, Marek Vasut wrote:
>>> On 2/21/19 9:40 AM, Chee, Tien Fong wrote:
>>>> On Thu, 2019-02-21 at 09:29 +0100, Alexander Graf wrote:
>>>>>
>>>>> On 21.02.19 09:23, Chee, Tien Fong wrote:
>>>>>>
>>>>>> On Thu, 2019-02-21 at 08:45 +0100, Michal Simek wrote:
>>>>>>>
>>>>>>> Hi Tom,
>>>>>>>
>>>>>>> On 20. 02. 19 2:58, Tom Rini wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On Mon, Feb 11, 2019 at 02:56:19PM +0800, tien.fong.chee at intel.
>>>>>>>> com
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> From: Tien Fong Chee <tien.fong.chee at intel.com>
>>>>>>>>>
>>>>>>>>> Drop the statically allocated get_contents_vfatname_block and
>>>>>>>>> dynamically allocate a buffer only if required. This saves
>>>>>>>>> 64KiB of memory.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Stefan Agner <stefan.ag... at toradex.com>
>>>>>>>>> Signed-off-by: Tien Fong Chee <tien.fong.chee at intel.com>
>>>>>>>> Applied to u-boot/master, thanks!
>>>>>>> please remove this patch (better both of them because they were
>>>>>>> in
>>>>>>> series)
>>>>>> I think patch 2/2 should be safe, because no memory size is
>>>>>> changed.
>>>>>> Basically, it just to release the allocated memory immediately when
>>>>>> it's not required, so other can re-use it.
>>>>>>
>>>>>>>
>>>>>>> because they are breaking at least ZynqMP SPL. It is also too
>>>>>>> late in cycle to create random fix.
>>>>>>>
>>>>>>> You can't simply move 64KB from code to malloc without reflecting
>>>>>>> this
>>>>>>> by changing MALLOC space size.
>>>>>>>
>>>>>>> Other boards with SPL fat could be also affected by this if they
>>>>>>> don't
>>>>>>> allocate big malloc space.
>>>>>> So, any suggestion to get the patch 1/2 accepted? inform all board
>>>>>> maintainers to test it out?
>>>>> You already received feedback that it does break ZynqMP, so the
>>>>> current
>>>>> approach won't work.
>>>>>
>>>>> How about you create a new kconfig option that allows you to say
>>>>> whether
>>>>> you want to use malloc or .bss for temporary data in the FAT driver.
>>>>> You
>>>>> can then have an _SPL_ version of that kconfig and check for it with
>>>>> IS_ENABLED() which should automatically tell you the right answer
>>>>> depending on whether you're in an SPL build or not.
>>>>>
>>>>> Then you can set the SPL version to default malloc and the non-SPL
>>>>> version to default .bss.
>>>> Marek and Tom rini,
>>>>
>>>> Are you guys okay with Alex's suggestion?
>>>
>>> I'm not a big fan of adding more and more ifdeffery.
>>> Is there some other option ?
>>
>> Is RAM up already at this point? Maybe we could improve the SPL malloc
>> mechanism to move allocations into DRAM once it's available.
> 
> Well, the FAT buffers waste some 64kiB of bss, so we can use that area
> for malloc instead, no ?

Yes, but that means you need to review every single board that uses FAT
in SPL today and adjust its malloc region size.

Alex