FS#9621 : FAT read-ahead when filling buffer

FS#9621 - FAT read-ahead when filling buffer

Attached to Project: Rockbox
Opened by Boris Gjenero (dreamlayers) - Wednesday, 10 December 2008, 01:36 GMT

Task Type	Patches
Category	Operating System/Drivers
Status	Unconfirmed
Assigned To	No-one
Operating System	SW-codec
Severity	Low
Priority	Normal
Reported Version	Daily build (which?)
Due in Version	Undecided
Due Date	Undecided
Percent Complete
Votes	0
Private	No

Details

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)

This task depends upon

Comments (6)
Related Tasks (0/0)

Comment by Linus Nielsen Feltzing (linusnielsen) - Wednesday, 10 December 2008, 07:52 GMT

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.

Comment by Bryan Childs (GodEater) - Wednesday, 17 December 2008, 08:42 GMT

I've tidied up the patch as per Linus' instructions above.

fatpreload-v0.2.patch (3.5 KiB)

Comment by Boris Gjenero (dreamlayers) - Friday, 19 December 2008, 21:41 GMT

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.

Comment by Rosso Maltese (asettico) - Monday, 03 August 2009, 16:44 GMT

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

Comment by Andree Buschmann (Buschel) - Saturday, 20 March 2010, 18:53 GMT

Updated against r25260 and made compilable for pcsim.

fatpreload-v0.3.patch (3.5 KiB)

Comment by Przemysław Hołubowski (p.h.) - Friday, 26 March 2010, 15:24 GMT

"Data abort at 0006D8F0 (0)" during playback using patched rev. 25342. It happened after about 2 hours of playback.

	Tasks related to this task (0)

Duplicate tasks of this task (0)

Rockbox

FS#9621 - FAT read-ahead when filling buffer

Details

Loading...