- Status Unconfirmed
- Percent Complete
- Task Type Patches
- Category Operating System/Drivers
- Assigned To No-one
- Operating System SW-codec
- Severity Low
- Priority Very Low
- Reported Version Daily build (which?)
- Due in Version Undecided
-
Due Date
Undecided
- Votes
- Private
FS#9621 - FAT read-ahead when filling buffer
I noticed that when filling the buffer my Video iPod (32MB RAM) does noticeable seeks at semi-regular intervals even if reading from a single non-fragmented file. It seems this is due to having to read the next FAT sector. In firmware/drivers/fat.c there is a FAT cache but no read-ahead of the FAT.
I created fat_preload( struct fat_file *file, long sectorcount ) in firmware/drivers/fat.c to preload the FAT cache. In firmware/common/file.c I created file_preloadfat(int fd, size_t count) because calling a fat_ function directly would be too much of a kludge. I use file_preloadfat in apps/buffering.c just before filling the buffer.
This got rid of that regular seeking and it seems to it can remove about two seconds from the time needed to fill the buffer when reading a single non-fragmented file. Currently the code is quite simple and unintelligent. Fragmentation or other activity can overwrite the pre-loaded FAT sectors in the cache, and in such cases the pre-load leads to needless extra reads. More work is probably needed before this can be included in Rockbox.
Currently this patch is for SW-codec only. I think file_preloadfat could be called from HW-codec buffering code also, though few seeks would be prevented with 2MB of RAM. This patch is also probably useless with flash storage.
fatpreload-v0.1.patch
(4 KiB)
I think I like the idea. With a large enough FAT cache, I don't see much of a disadvantage. Please tidy up the patch and remove the #if 0's and the commented code (/* char buf[80]; */). You should also make sure that you use 4 spaces for indentation instead of TAB characters.
I've tidied up the patch as per Linus' instructions above.
Thanks Bryan.
I was thinking that I should probably do more to ensure that the FAT preloading actually helps,for example making sure I'm not overwriting already pre-loaded sectors. I was also thinking that another approach might be better, like each open file having a buffer with a list of extents ({ sector, length } pairs), and maybe only having a smaller FAT cache for writes. However, I don't have a fully formed idea on this, and right now I'm focused on figuring out how to do ATA DMA on the PP5020.
The patch breaks the simulator. Probably it's due to some missing ifdefs.
The error messages are:
… LD lua.rock
/home/asettico-9.04/Sviluppo/rockbox/rockbox/build_sim/apps/buffering.o: In function `buffer_handle':
/home/asettico-9.04/Sviluppo/rockbox/rockbox/apps/buffering.c:609: undefined reference to `file_preloadfat'
collect2: ld returned 1 exit status
make: *** [/home/asettico-9.04/Sviluppo/rockbox/rockbox/build_sim/rockboxui] Errore 1
Updated against r25260 and made compilable for pcsim.
"Data abort at 0006D8F0 (0)" during playback using patched rev. 25342. It happened after about 2 hours of playback.