FS#11883 - Close all files and safely unmount disks before entering USB mode, shutting down, or rebooting

Attached to Project: Rockbox
Opened by sideral (sideral) - Sunday, 16 January 2011, 22:23 GMT
Task Type Patches
Category Operating System/Drivers
Status Unconfirmed
Assigned To No-one
Operating System All players
Severity Low
Priority Normal
Reported Version Daily build (which?)
Due in Version Undecided
Due Date Undecided
Percent Complete 0%
Votes 0
Private No


The attached patch implements a clean disk unmount by way of the following changes:

Patch 1: Allow disk_unmount_all to close all files and flush dirty buffers when unmounting a filesystem. (Before, we've been calling disk_unmount, which does not flush buffers because it's written to handle user-ejected storage media that aren't accessible any more.)

Patch 2: Call disk_unmount_all in the power-down and reboot paths.

Patch 3: Fix write to detect a bad file handle.

(Split out from FS#11877, in which I first theorized that an unclean filesystem may be responsible for some filesystem corruption I'm seeing, but which turned out to be a deeper problem. Related to r28693 /  FS#11774 , in which we introduced disk unmounting before entering USB mode.)
This task depends upon

Comment by sideral (sideral) - Monday, 24 January 2011, 21:41 GMT
In an IRC conversation, gevaerts noted that code that doesn't close its files when it receives SYS_USB_CONNECTED is considered broken. In my experiments, I found that typically at least the following files are still open when entering usb_slave_mode:

* /.rockbox/fonts/08-Rockfont.fnt
* the currently played MP3 file

There's sometimes a third file still open that I haven't identified yet.

Also, there's always at least one file (sometimes more) still open when the player shuts down, but I don't know yet which files these are. (Presumably, at least one of them is the font file, again.)
Comment by Dominik Riebeling (bluebrother) - Monday, 24 January 2011, 22:24 GMT
maybe /.rockbox/config.cfg or /.rockbox/nvram.bin (just guessing)?
Comment by Jonathan Gordon (jdgordon) - Monday, 24 January 2011, 23:09 GMT
unlikely, those are only opened when being saved (and unless a stupid bug is there are closed straight away). very likely /.rockbox/.playlist_control, the current playlists .m3u file (if it doesnt fit in ram) any and all .fnt files.
Comment by sideral (sideral) - Monday, 31 January 2011, 19:06 GMT
Patch 0002 above is incomplete: It lacks a disk_unmount_all() in one of the variants of the rolo_load function bodies. A corrected patch is attached.
Comment by sideral (sideral) - Friday, 08 April 2011, 14:01 GMT
Using a diagnostic patch originally developed by pamaury, I found one occurrence where a third file was open prior an USB connection:
The USB connection was done shortly after boot while a database refresh was in progress. The file was open in read-only mode.

I've attached my version of pamaury's patch. It's relative to the other three patches found in this task.