[U-Boot] [PATCH 2/2] fs/fat: Optimizes memory size with single global variable instead of 3

[Stephen Warren]

On 08/13/2016 04:57 PM, Benoît Thébaudeau wrote:
> Hi,
>
> On Tue, Aug 2, 2016 at 9:35 PM, Benoît Thébaudeau
> <benoit.thebaudeau.dev at gmail.com> wrote:
>> On Tue, Aug 2, 2016 at 8:53 PM, Stephen Warren <swarren at wwwdotorg.org> wrote:
>>> On 07/28/2016 12:11 AM, Tien Fong Chee wrote:
>>>>
>>>> Single 64KB get_contents_vfatname_block global variable would be used for
>>>> all FAT implementation instead of allocating additional two global
>>>> variables
>>>> which are get_denfromdir_block and do_fat_read_at_block. This
>>>> implementation
>>>> can help in saving up 128KB memory space.
>>>
>>>
>>> The series,
>>>
>>> Tested-by: Stephen Warren <swarren at nvidia.com>
>>> (via DFU's FAT reading/writing on various Tegra; likely primarily FAT rather
>>> than VFAT though)
>>>
>>> Reviewed-by: Stephen Warren <swarren at nvidia.com>
>>
>> I suspect that reading a filename with VFAT entries crossing a cluster
>> boundary in a FAT32 root directory will be broken by this series. I do
>> not have time to test this and other corner cases right now though,
>> but it will be possible in the next few weeks.
>
> I have tested VFAT long filenames with the current implementation on
> Sandbox. It's completely broken:
>  - There is a length limit somewhere between 111 and 120 characters,
> instead of 256 characters. Beyond this limit, the files are invisible
> with ls.
>  - Some filenames are truncated or mixed up between files. I have
> tested 111-character random filenames for 1000 empty files in the root
> directory. Most filenames had the expected length, but a few were
> shorter or longer.
>  - If there are too many files in the root directory, ls hangs.
>
> I am pretty sure that this series introduces some regressions, but
> they seem to be in corner cases that cannot even be used or tested
> because of other bugs, so this series might not make this
> implementation much more broken than it currently is. It's risky,
> though.
>
> I've quickly looked into TianoCore EDK II. It is so deeply tied to the
> EFI driver model and APIs that it would be a pain to port to U-Boot.
> Its FAT module is not designed to be portable beyond EFI. Its build
> system would complicate things too.
>
> Stephen, according to what you say in test/fs/fat-noncontig-test.sh,
> your solution to accelerate FatFs seems to be working, even if the
> author is not interested in it, so maybe it would still be worth
> maintaining locally in order to have a reliable FAT support, also with
> a small memory footprint. barebox uses FatFs.

Tom, any thoughts here re: the FF FAT implementation again?

BTW, I had some user of FF FAT about the "contiguous read" patch 
claiming that it caused issues in some cases. I didn't investigate since 
we'd dropped the idea of using FF FAT. Hopefully I could find the email 
(or maybe it was a post of the FF forums) again.