From 9f7f258728c119e2bcfb134cde97b26ec280c8e3 Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Sun, 15 Mar 2015 07:45:19 -0600 Subject: [PATCH] Add support for umount2(target, MNT_FORCE) in the FAT file system. --- TODO | 68 +------ fs/binfs/fs_binfs.c | 2 +- fs/fat/fs_fat32.c | 452 ++++++++++++++++++++++++++------------------ fs/fat/fs_fat32.h | 8 +- 4 files changed, 274 insertions(+), 256 deletions(-) diff --git a/TODO b/TODO index 57a84b081b..66e1be8272 100644 --- a/TODO +++ b/TODO @@ -18,7 +18,7 @@ nuttx/ (12) Network (net/, drivers/net) (4) USB (drivers/usbdev, drivers/usbhost) (11) Libraries (libc/, libm/) - (13) File system/Generic drivers (fs/, drivers/) + (11) File system/Generic drivers (fs/, drivers/) (9) Graphics subystem (graphics/) (1) Pascal add-on (pcode/) (1) Documentation (Documentation/) @@ -1303,72 +1303,6 @@ o File system / Generic drivers (fs/, drivers/) Status: Open Priority: Medium - Title: UNMOUNT WITH OPEN FILES - Description: What is the policy for umounting files that have open file - references? Currently, umount() does the unmount - unconditionally and this has been noted to cause crashes in - some subsequent FAT file system operations. umount() is not - a standard OS interface and so have no specification. Here - are some possible behaviors or unmount() with open files: - - 1. Refuse to unmount() the file system if there are open - references to files on the file system (i.e., the file - system unbind() method returns a failure). - 2. The file system knows that it has open references and so - does not unbind() immediately, but defers until the last - file is closed. - 3. The file system performs the unbind() immediately, but - returns an error for an any subsequent operations on the - file system using one of the stale file handles. - 4. In your application, do not un-mount until you have closed - all references to files or directories in the file system. - This would probably be a good practice anyway. - - I lieu of any real specifications, I often just do what Linux - does. Here is now Linux does things: - http://man7.org/linux/man-pages/man2/umount.2.html . - Linux has have a umount2() that takes flags that control these - behaviors. They have a couple of other options as well. - - It is not explicitly stated in the manpage, but it looks like - the Linux umount() does the first option: It should return - EBUSY meaning that the "target could not be unmounted because - it is busy." That one is pretty easy to implement within the - filesystem unbind() method. - - I now think for an embedded system with mostly removable - storage devices, the option 3 is probably the most usable. For - example, NuttX has a little automounter at nuttx/fs/mount/fs_automount.c - That will automatically mount and unmount SD cards as they are - inserted or removed. I think that is a desire-able feature on - an embedded device. And I think it demands option 3 where the - umount occurs immediately, but then subsequent operations on - the volume fail. - - The true lazy umount could also cause issues if the file is - never closed. Of course if the media is gone, I would hope - that the file access also fail in that case. But what if the - media is inserted and removed many times. Seems like the lazy - unmount could cause some problems and won't really buy you - anything anyway (because the file I/O will fail anyway). - Status: Open - Priority: Medium-High - - Title: AUTMOUNTER BROKEN - Description: A bug was recently fixed in the FAT file system umount() logic. - FAT (and all other file systems) implement logic to fail the umount() - with EBUSY if there are open references to any file. For FAT, there - was a bug that prevented this from working so the busy check always - failed and FAT always permitted the umount(), whether there are open - file or not. - - Unfortunately, the automounter depended on this bug to force the - un-mounting. I have changed the umount() call in the automounter to - umount2(MNT_FORCE), but the automounter will be broken until that - forced unmount feature is implemented. - Status: Open - Priority: Medium - o Graphics subsystem (graphics/) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/fs/binfs/fs_binfs.c b/fs/binfs/fs_binfs.c index a3ffa3a34b..52a801560f 100644 --- a/fs/binfs/fs_binfs.c +++ b/fs/binfs/fs_binfs.c @@ -100,7 +100,7 @@ static int binfs_stat(FAR struct inode *mountpt, FAR const char *relpath, * Public Variables ****************************************************************************/ -/* See fs_mount.c -- this structure is explicitly externed there. +/* See fs_mount.c -- this structure is explicitly extern'ed there. * We use the old-fashioned kind of initializers so that this will compile * with any compiler. */ diff --git a/fs/fat/fs_fat32.c b/fs/fat/fs_fat32.c index d755493006..ebc3fb7bcb 100644 --- a/fs/fat/fs_fat32.c +++ b/fs/fat/fs_fat32.c @@ -48,6 +48,7 @@ #include #include #include +#include #include #include @@ -158,15 +159,15 @@ const struct mountpt_operations fat_operations = * Name: fat_open ****************************************************************************/ -static int fat_open(FAR struct file *filep, const char *relpath, +static int fat_open(FAR struct file *filep, FAR const char *relpath, int oflags, mode_t mode) { - struct fat_dirinfo_s dirinfo; - struct inode *inode; - struct fat_mountpt_s *fs; - struct fat_file_s *ff; - uint8_t *direntry; - int ret; + struct fat_dirinfo_s dirinfo; + FAR struct inode *inode; + FAR struct fat_mountpt_s *fs; + FAR struct fat_file_s *ff; + uint8_t *direntry; + int ret; /* Sanity checks */ @@ -389,12 +390,12 @@ errout_with_semaphore: static int fat_close(FAR struct file *filep) { - struct inode *inode; - struct fat_file_s *ff; - struct fat_file_s *currff; - struct fat_file_s *prevff; - struct fat_mountpt_s *fs; - int ret; + FAR struct inode *inode; + FAR struct fat_file_s *ff; + FAR struct fat_file_s *currff; + FAR struct fat_file_s *prevff; + FAR struct fat_mountpt_s *fs; + int ret = OK; /* Sanity checks */ @@ -402,35 +403,44 @@ static int fat_close(FAR struct file *filep) /* Recover our private data from the struct file instance */ - ff = filep->f_priv; - inode = filep->f_inode; - fs = inode->i_private; + ff = filep->f_priv; - /* Do not check if the mount is healthy. We must support closing of - * the file even when there is healthy mount. - */ + /* Check for the forced mount condition */ - /* Synchronize the file buffers and disk content; update times */ - - ret = fat_sync(filep); - - /* Remove the file structure from the list of open files in the mountpoint - * structure. - */ - - for (prevff = NULL, currff = fs->fs_head; - currff && currff != ff; - prevff = currff, currff = currff->ff_next); - - if (currff) + if ((ff->ff_bflags & UMOUNT_FORCED) == 0) { - if (prevff) + inode = filep->f_inode; + fs = inode->i_private; + + DEBUGASSERT(fs != NULL); + + /* Do not check if the mount is healthy. We must support closing of + * the file even when there is healthy mount. + */ + + /* Synchronize the file buffers and disk content; update times */ + + ret = fat_sync(filep); + + /* Remove the file structure from the list of open files in the + * mountpoint + * structure. + */ + + for (prevff = NULL, currff = fs->fs_head; + currff && currff != ff; + prevff = currff, currff = currff->ff_next); + + if (currff) { - prevff->ff_next = ff->ff_next; - } - else - { - fs->fs_head = ff->ff_next; + if (prevff) + { + prevff->ff_next = ff->ff_next; + } + else + { + fs->fs_head = ff->ff_next; + } } } @@ -456,20 +466,20 @@ static int fat_close(FAR struct file *filep) * Name: fat_read ****************************************************************************/ -static ssize_t fat_read(FAR struct file *filep, char *buffer, size_t buflen) +static ssize_t fat_read(FAR struct file *filep, FAR char *buffer, size_t buflen) { - struct inode *inode; - struct fat_mountpt_s *fs; - struct fat_file_s *ff; - unsigned int bytesread; - unsigned int readsize; - unsigned int nsectors; - size_t bytesleft; - int32_t cluster; - uint8_t *userbuffer = (uint8_t*)buffer; - int sectorindex; - int ret; - bool force_indirect = false; + FAR struct inode *inode; + FAR struct fat_mountpt_s *fs; + FAR struct fat_file_s *ff; + unsigned int bytesread; + unsigned int readsize; + unsigned int nsectors; + size_t bytesleft; + int32_t cluster; + FAR uint8_t *userbuffer = (uint8_t*)buffer; + int sectorindex; + int ret; + bool force_indirect = false; /* Sanity checks */ @@ -477,7 +487,15 @@ static ssize_t fat_read(FAR struct file *filep, char *buffer, size_t buflen) /* Recover our private data from the struct file instance */ - ff = filep->f_priv; + ff = filep->f_priv; + + /* Check for the forced mount condition */ + + if ((ff->ff_bflags & UMOUNT_FORCED) != 0) + { + return -EPIPE; + } + inode = filep->f_inode; fs = inode->i_private; @@ -676,20 +694,20 @@ errout_with_semaphore: * Name: fat_write ****************************************************************************/ -static ssize_t fat_write(FAR struct file *filep, const char *buffer, +static ssize_t fat_write(FAR struct file *filep, FAR const char *buffer, size_t buflen) { - struct inode *inode; - struct fat_mountpt_s *fs; - struct fat_file_s *ff; - int32_t cluster; - unsigned int byteswritten; - unsigned int writesize; - unsigned int nsectors; - uint8_t *userbuffer = (uint8_t*)buffer; - int sectorindex; - int ret; - bool force_indirect = false; + FAR struct inode *inode; + FAR struct fat_mountpt_s *fs; + FAR struct fat_file_s *ff; + int32_t cluster; + unsigned int byteswritten; + unsigned int writesize; + unsigned int nsectors; + FAR uint8_t *userbuffer = (uint8_t*)buffer; + int sectorindex; + int ret; + bool force_indirect = false; /* Sanity checks. I have seen the following assertion misfire if * CONFIG_DEBUG_MM is enabled while re-directing output to a @@ -709,7 +727,15 @@ static ssize_t fat_write(FAR struct file *filep, const char *buffer, /* Recover our private data from the struct file instance */ - ff = filep->f_priv; + ff = filep->f_priv; + + /* Check for the forced mount condition */ + + if ((ff->ff_bflags & UMOUNT_FORCED) != 0) + { + return -EPIPE; + } + inode = filep->f_inode; fs = inode->i_private; @@ -971,13 +997,13 @@ errout_with_semaphore: static off_t fat_seek(FAR struct file *filep, off_t offset, int whence) { - struct inode *inode; - struct fat_mountpt_s *fs; - struct fat_file_s *ff; - int32_t cluster; - off_t position; - unsigned int clustersize; - int ret; + FAR struct inode *inode; + FAR struct fat_mountpt_s *fs; + FAR struct fat_file_s *ff; + int32_t cluster; + off_t position; + unsigned int clustersize; + int ret; /* Sanity checks */ @@ -985,7 +1011,15 @@ static off_t fat_seek(FAR struct file *filep, off_t offset, int whence) /* Recover our private data from the struct file instance */ - ff = filep->f_priv; + ff = filep->f_priv; + + /* Check for the forced mount condition */ + + if ((ff->ff_bflags & UMOUNT_FORCED) != 0) + { + return -EPIPE; + } + inode = filep->f_inode; fs = inode->i_private; @@ -1112,7 +1146,7 @@ static off_t fat_seek(FAR struct file *filep, off_t offset, int whence) } else { - /* Otherwise we can only follong the existing chain */ + /* Otherwise we can only follow the existing chain */ cluster = fat_getcluster(fs, cluster); } @@ -1131,7 +1165,7 @@ static off_t fat_seek(FAR struct file *filep, off_t offset, int whence) if (cluster == 0) { - /* At the position to the current locaiton and + /* At the position to the current location and * break out. */ @@ -1199,14 +1233,23 @@ errout_with_semaphore: static int fat_ioctl(FAR struct file *filep, int cmd, unsigned long arg) { - struct inode *inode; - struct fat_mountpt_s *fs; - int ret; + FAR struct inode *inode; + FAR struct fat_file_s *ff; + FAR struct fat_mountpt_s *fs; + int ret; /* Sanity checks */ DEBUGASSERT(filep->f_priv != NULL && filep->f_inode != NULL); + /* Check for the forced mount condition */ + + ff = filep->f_priv; + if ((ff->ff_bflags & UMOUNT_FORCED) != 0) + { + return -EPIPE; + } + /* Recover our private data from the struct file instance */ inode = filep->f_inode; @@ -1240,20 +1283,27 @@ static int fat_ioctl(FAR struct file *filep, int cmd, unsigned long arg) static int fat_sync(FAR struct file *filep) { - struct inode *inode; - struct fat_mountpt_s *fs; - struct fat_file_s *ff; - uint32_t wrttime; - uint8_t *direntry; - int ret; + FAR struct inode *inode; + FAR struct fat_mountpt_s *fs; + FAR struct fat_file_s *ff; + uint32_t wrttime; + uint8_t *direntry; + int ret; /* Sanity checks */ DEBUGASSERT(filep->f_priv != NULL && filep->f_inode != NULL); + /* Check for the forced mount condition */ + + ff = filep->f_priv; + if ((ff->ff_bflags & UMOUNT_FORCED) != 0) + { + return -EPIPE; + } + /* Recover our private data from the struct file instance */ - ff = filep->f_priv; inode = filep->f_inode; fs = inode->i_private; @@ -1350,9 +1400,21 @@ static int fat_dup(FAR const struct file *oldp, FAR struct file *newp) newp->f_priv == NULL && newp->f_inode != NULL); + /* Recover the old private data from the old struct file instance */ + + oldff = oldp->f_priv; + + /* Check for the forced mount condition */ + + if ((oldff->ff_bflags & UMOUNT_FORCED) != 0) + { + return -EPIPE; + } + /* Recover our private data from the struct file instance */ fs = (struct fat_mountpt_s *)oldp->f_inode->i_private; + DEBUGASSERT(fs != NULL); /* Check if the mount is still healthy */ @@ -1364,10 +1426,6 @@ static int fat_dup(FAR const struct file *oldp, FAR struct file *newp) goto errout_with_semaphore; } - /* Recover the old private data from the old struct file instance */ - - oldff = oldp->f_priv; - /* Create a new instance of the file private date to describe the * dup'ed file. */ @@ -1453,12 +1511,13 @@ errout_with_semaphore: * ****************************************************************************/ -static int fat_opendir(struct inode *mountpt, const char *relpath, struct fs_dirent_s *dir) +static int fat_opendir(FAR struct inode *mountpt, FAR const char *relpath, + FAR struct fs_dirent_s *dir) { - struct fat_mountpt_s *fs; - struct fat_dirinfo_s dirinfo; - uint8_t *direntry; - int ret; + FAR struct fat_mountpt_s *fs; + FAR struct fat_dirinfo_s dirinfo; + uint8_t *direntry; + int ret; /* Sanity checks */ @@ -1537,15 +1596,15 @@ errout_with_semaphore: * ****************************************************************************/ -static int fat_readdir(struct inode *mountpt, struct fs_dirent_s *dir) +static int fat_readdir(FAR struct inode *mountpt, FAR struct fs_dirent_s *dir) { - struct fat_mountpt_s *fs; - unsigned int dirindex; - uint8_t *direntry; - uint8_t ch; - uint8_t attribute; - bool found; - int ret; + FAR struct fat_mountpt_s *fs; + unsigned int dirindex; + FAR uint8_t *direntry; + uint8_t ch; + uint8_t attribute; + bool found; + int ret; /* Sanity checks */ @@ -1682,9 +1741,10 @@ errout_with_semaphore: * ****************************************************************************/ -static int fat_rewinddir(struct inode *mountpt, struct fs_dirent_s *dir) +static int fat_rewinddir(FAR struct inode *mountpt, + FAR struct fs_dirent_s *dir) { - struct fat_mountpt_s *fs; + FAR struct fat_mountpt_s *fs; int ret; /* Sanity checks */ @@ -1757,10 +1817,10 @@ errout_with_semaphore: * ****************************************************************************/ -static int fat_bind(FAR struct inode *blkdriver, const void *data, - void **handle) +static int fat_bind(FAR struct inode *blkdriver, FAR const void *data, + FAR void **handle) { - struct fat_mountpt_s *fs; + FAR struct fat_mountpt_s *fs; int ret; /* Open the block driver */ @@ -1820,7 +1880,7 @@ static int fat_bind(FAR struct inode *blkdriver, const void *data, static int fat_unbind(FAR void *handle, FAR struct inode **blkdriver, unsigned int flags) { - struct fat_mountpt_s *fs = (struct fat_mountpt_s*)handle; + FAR struct fat_mountpt_s *fs = (FAR struct fat_mountpt_s*)handle; int ret; if (!fs) @@ -1830,59 +1890,78 @@ static int fat_unbind(FAR void *handle, FAR struct inode **blkdriver, /* Check if there are sill any files opened on the filesystem. */ - ret = OK; /* Assume success */ fat_semtake(fs); if (fs->fs_head) { - /* We cannot unmount now.. there are open files - * - * This implementation currently only supports unmounting if there are - * no open file references. + /* There are open files. We umount now unless we are forced with the + * MNT_FORCE flag. Forcing the unmount will cause data loss because + * the filesystem buffers are not flushed to the media. MNT_DETACH, + * the 'lazy' unmount, could be implemented to fix this. */ - ret = (flags != 0) ? -ENOSYS : -EBUSY; - } - else - { - /* Unmount ... close the block driver */ - - if (fs->fs_blkdriver) + if ((flags & MNT_FORCE) != 0) { - struct inode *inode = fs->fs_blkdriver; - if (inode) + FAR struct fat_file_s *ff; + + /* Set a flag in each open file structure. This flag will signal + * the file system to fail any subsequent attempts to used the + * file handle. + */ + + for (ff = fs->fs_head; ff; ff = ff->ff_next) { - if (inode->u.i_bops && inode->u.i_bops->close) - { - (void)inode->u.i_bops->close(inode); - } - - /* We hold a reference to the block driver but should - * not but mucking with inodes in this context. So, we will just return - * our contained reference to the block driver inode and let the umount - * logic dispose of it. - */ - - if (blkdriver) - { - *blkdriver = inode; - } + ff->ff_bflags |= UMOUNT_FORCED; } } - - /* Release the mountpoint private data */ - - if (fs->fs_buffer) + else { - fat_io_free(fs->fs_buffer, fs->fs_hwsectorsize); - } + /* We cannot unmount now.. there are open files. This + * implementation does not support any other umount2() + * options. + */ - sem_destroy(&fs->fs_sem); - kmm_free(fs); - return ret; + fat_semgive(fs); + ret = (flags != 0) ? -ENOSYS : -EBUSY; + return ret; + } } - fat_semgive(fs); - return ret; + /* Unmount ... close the block driver */ + + if (fs->fs_blkdriver) + { + FAR struct inode *inode = fs->fs_blkdriver; + if (inode) + { + if (inode->u.i_bops && inode->u.i_bops->close) + { + (void)inode->u.i_bops->close(inode); + } + + /* We hold a reference to the block driver but should not but + * mucking with inodes in this context. So, we will just return + * our contained reference to the block driver inode and let the + * umount + * logic dispose of it. + */ + + if (blkdriver) + { + *blkdriver = inode; + } + } + } + + /* Release the mountpoint private data */ + + if (fs->fs_buffer) + { + fat_io_free(fs->fs_buffer, fs->fs_hwsectorsize); + } + + sem_destroy(&fs->fs_sem); + kmm_free(fs); + return OK; } /**************************************************************************** @@ -1892,10 +1971,10 @@ static int fat_unbind(FAR void *handle, FAR struct inode **blkdriver, * ****************************************************************************/ -static int fat_statfs(struct inode *mountpt, struct statfs *buf) +static int fat_statfs(FAR struct inode *mountpt, FAR struct statfs *buf) { - struct fat_mountpt_s *fs; - int ret; + FAR struct fat_mountpt_s *fs; + int ret; /* Sanity checks */ @@ -1949,10 +2028,10 @@ errout_with_semaphore: * ****************************************************************************/ -static int fat_unlink(struct inode *mountpt, const char *relpath) +static int fat_unlink(FAR struct inode *mountpt, FAR const char *relpath) { - struct fat_mountpt_s *fs; - int ret; + FAR struct fat_mountpt_s *fs; + int ret; /* Sanity checks */ @@ -1990,19 +2069,20 @@ static int fat_unlink(struct inode *mountpt, const char *relpath) * ****************************************************************************/ -static int fat_mkdir(struct inode *mountpt, const char *relpath, mode_t mode) +static int fat_mkdir(FAR struct inode *mountpt, FAR const char *relpath, + mode_t mode) { - struct fat_mountpt_s *fs; - struct fat_dirinfo_s dirinfo; - uint8_t *direntry; - uint8_t *direntry2; - off_t parentsector; - off_t dirsector; - int32_t dircluster; - uint32_t parentcluster; - uint32_t crtime; + FAR struct fat_mountpt_s *fs; + FAR struct fat_dirinfo_s dirinfo; + FAR uint8_t *direntry; + FAR uint8_t *direntry2; + off_t parentsector; + off_t dirsector; + int32_t dircluster; + uint32_t parentcluster; + uint32_t crtime; unsigned int i; - int ret; + int ret; /* Sanity checks */ @@ -2207,10 +2287,10 @@ errout_with_semaphore: * ****************************************************************************/ -int fat_rmdir(struct inode *mountpt, const char *relpath) +int fat_rmdir(FAR struct inode *mountpt, FAR const char *relpath) { - struct fat_mountpt_s *fs; - int ret; + FAR struct fat_mountpt_s *fs; + int ret; /* Sanity checks */ @@ -2248,15 +2328,15 @@ int fat_rmdir(struct inode *mountpt, const char *relpath) * ****************************************************************************/ -int fat_rename(struct inode *mountpt, const char *oldrelpath, - const char *newrelpath) +int fat_rename(FAR struct inode *mountpt, FAR const char *oldrelpath, + FAR const char *newrelpath) { - struct fat_mountpt_s *fs; - struct fat_dirinfo_s dirinfo; - struct fat_dirseq_s dirseq; - uint8_t *direntry; - uint8_t dirstate[DIR_SIZE-DIR_ATTRIBUTES]; - int ret; + FAR struct fat_mountpt_s *fs; + FAR struct fat_dirinfo_s dirinfo; + FAR struct fat_dirseq_s dirseq; + uint8_t *direntry; + uint8_t dirstate[DIR_SIZE-DIR_ATTRIBUTES]; + int ret; /* Sanity checks */ @@ -2394,16 +2474,17 @@ errout_with_semaphore: * ****************************************************************************/ -static int fat_stat(struct inode *mountpt, const char *relpath, struct stat *buf) +static int fat_stat(FAR struct inode *mountpt, FAR const char *relpath, + FAR struct stat *buf) { - struct fat_mountpt_s *fs; - struct fat_dirinfo_s dirinfo; - uint16_t fatdate; - uint16_t date2; - uint16_t fattime; - uint8_t *direntry; - uint8_t attribute; - int ret; + FAR struct fat_mountpt_s *fs; + struct fat_dirinfo_s dirinfo; + uint16_t fatdate; + uint16_t date2; + uint16_t fattime; + FAR uint8_t *direntry; + uint8_t attribute; + int ret; /* Sanity checks */ @@ -2510,4 +2591,3 @@ errout_with_semaphore: /**************************************************************************** * Public Functions ****************************************************************************/ - diff --git a/fs/fat/fs_fat32.h b/fs/fat/fs_fat32.h index 3b3852fe47..cfb97f37d8 100644 --- a/fs/fat/fs_fat32.h +++ b/fs/fat/fs_fat32.h @@ -271,12 +271,16 @@ #define FSTYPE_FAT16 1 #define FSTYPE_FAT32 2 -/* File buffer flags */ +/* File buffer flags (ff_bflags) */ #define FFBUFF_VALID 1 #define FFBUFF_DIRTY 2 #define FFBUFF_MODIFIED 4 +/* Mount status flags (ff_bflags) */ + +#define UMOUNT_FORCED 8 + /**************************************************************************** * These offset describe the FSINFO sector */ @@ -753,7 +757,7 @@ struct fat_mountpt_s struct fat_file_s { struct fat_file_s *ff_next; /* Retained in a singly linked list */ - uint8_t ff_bflags; /* The file buffer flags */ + uint8_t ff_bflags; /* The file buffer/mount flags */ uint8_t ff_oflags; /* Flags provided when file was opened */ uint8_t ff_sectorsincluster; /* Sectors remaining in cluster */ uint16_t ff_dirindex; /* Index into ff_dirsector to directory entry */