Rockbox

This is the bug/patch tracker for Rockbox. Click here for more information.

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

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, 23:23 GMT+2
Task Type Patches
Category Operating System/Drivers
Status Unconfirmed
Assigned To No-one
Player Type All players
Severity Low
Priority Normal
Reported Version Daily build (which?)
Due in Version Undecided
Due Date Undecided
Percent Complete 0%
Private No

Details

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.)
   0001-When-unmounting-FAT-volumes-close-all-open-f... (3.5 KiB) 
 b/firmware/common/dir_uncached.c |    7 +++++--
 b/firmware/common/disk.c         |   22 ++++++++++++++--------
 b/firmware/common/file.c         |   31 +++++++++++++++++++++++++++++--
 b/firmware/include/file.h        |    1 +
 4 files changed, 49 insertions(+), 12 deletions(-)

   0002-Unmount-disks-explicitly-in-clean_shutdown-a... (1.1 KiB) 
 b/apps/misc.c     |    2 ++
 b/firmware/rolo.c |    2 ++
 2 files changed, 4 insertions(+)

   0003-write-Validate-file-handle-in-write-system-c... (0.7 KiB) 
 b/firmware/common/file.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

This task depends upon

Comment by sideral (sideral) - Monday, 24 January 2011, 22:41 GMT+2
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, 23:24 GMT+2
maybe /.rockbox/config.cfg or /.rockbox/nvram.bin (just guessing)?
Comment by Jonathan Gordon (jdgordon) - Tuesday, 25 January 2011, 00:09 GMT+2
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, 20:06 GMT+2
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.
   0002-Unmount-disks-explicitly-in-clean_shutdown-a... (1.3 KiB) 
 b/apps/misc.c     |    2 ++
 b/firmware/rolo.c |    3 +++
 2 files changed, 5 insertions(+)

Comment by sideral (sideral) - Friday, 08 April 2011, 16:01 GMT+2
Using a diagnostic patch originally developed by pamaury, I found one occurrence where a third file was open prior an USB connection:
database_4.tcd
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.
   openfiles-debug.patch (4.5 KiB) 
 b/firmware/common/disk.c |   17 ++++++++++++++---
 b/firmware/common/file.c |   27 ++++++++++++++++++++++++++-
 b/firmware/drivers/fat.c |   22 ++++++++++++++++++++++
 b/firmware/export/fat.h  |    2 ++
 b/firmware/usb.c         |    2 +-
 5 files changed, 65 insertions(+), 5 deletions(-)

Loading...