Add support for umount2(target, MNT_FORCE) in the FAT file system.

This commit is contained in:
Gregory Nutt 2015-03-15 07:45:19 -06:00
parent 43936a6a69
commit 9f7f258728
4 changed files with 274 additions and 256 deletions

68
TODO
View File

@ -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/)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

View File

@ -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.
*/

View File

@ -48,6 +48,7 @@
#include <sys/types.h>
#include <sys/stat.h>
#include <sys/statfs.h>
#include <sys/mount.h>
#include <stdlib.h>
#include <unistd.h>
@ -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
****************************************************************************/

View File

@ -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 */