diff --git a/nuttx/ChangeLog b/nuttx/ChangeLog index 4725a971f..56ea043b7 100644 --- a/nuttx/ChangeLog +++ b/nuttx/ChangeLog @@ -152,5 +152,6 @@ * Added stat() to fs layer * Added stat() supported to FAT + * Fixed reference counting errors associated with mounted filesystems * Started m68322 diff --git a/nuttx/Documentation/NuttX.html b/nuttx/Documentation/NuttX.html index 7f2d686b4..96b9e8221 100644 --- a/nuttx/Documentation/NuttX.html +++ b/nuttx/Documentation/NuttX.html @@ -586,6 +586,7 @@ Other memory: * Added stat() to fs layer * Added stat() supported to FAT + * Fixed reference counting errors associated with mounted filesystems * Started m68322 diff --git a/nuttx/examples/mount/mount_main.c b/nuttx/examples/mount/mount_main.c index 184c1fcc0..716b4d9a9 100644 --- a/nuttx/examples/mount/mount_main.c +++ b/nuttx/examples/mount/mount_main.c @@ -53,6 +53,9 @@ * Definitions ****************************************************************************/ +#define TEST_USE_STAT 1 +#define TEST_SHOW_DIRECTORIES 1 + /**************************************************************************** * Private Types ****************************************************************************/ @@ -83,6 +86,7 @@ static char g_namebuffer[256]; * Private Functions ****************************************************************************/ +#ifdef TEST_USE_STAT static void show_stat(const char *path, struct stat *ps) { printf("%s stat:\n", path); @@ -111,15 +115,17 @@ static void show_stat(const char *path, struct stat *ps) printf("\tsize : %d (bytes)\n", ps->st_size); printf("\tblock size : %d (bytes)\n", ps->st_blksize); printf("\tsize : %d (blocks)\n", ps->st_blocks); - printf("\taccess time : %d (blocks)\n", ps->st_atime); - printf("\tmodify time : %d (blocks)\n", ps->st_mtime); - printf("\tchange time : %d (blocks)\n", ps->st_ctime); + printf("\taccess time : %d\n", ps->st_atime); + printf("\tmodify time : %d\n", ps->st_mtime); + printf("\tchange time : %d\n", ps->st_ctime); } +#endif /**************************************************************************** * Name: show_directories ****************************************************************************/ +#ifdef TEST_SHOW_DIRECTORIES static void show_directories(const char *path, int indent) { DIR *dirp; @@ -157,6 +163,9 @@ static void show_directories(const char *path, int indent) closedir(dirp); } +#else +# define show_directories(p,i) +#endif /**************************************************************************** * Name: fail_read_open @@ -446,6 +455,7 @@ static void succeed_rename(const char *oldpath, const char *newpath) * Name: fail_stat ****************************************************************************/ +#ifdef TEST_USE_STAT static void fail_stat(const char *path, int expectederror) { struct stat buf; @@ -469,11 +479,15 @@ static void fail_stat(const char *path, int expectederror) g_nerrors++; } } +#else +# define fail_stat(p,e); +#endif /**************************************************************************** * Name: succeed_stat ****************************************************************************/ +#ifdef TEST_USE_STAT static void succeed_stat(const char *path) { struct stat buf; @@ -494,6 +508,9 @@ static void succeed_stat(const char *path) show_stat(path, &buf); } } +#else +#define succeed_stat(p) +#endif /**************************************************************************** * Public Functions diff --git a/nuttx/fs/fs_fat32.c b/nuttx/fs/fs_fat32.c index e8677e1be..098353cd5 100644 --- a/nuttx/fs/fs_fat32.c +++ b/nuttx/fs/fs_fat32.c @@ -91,7 +91,7 @@ static int fat_rewinddir(struct inode *mountpt, struct internal_dir_s *dir); static int fat_bind(FAR struct inode *blkdriver, const void *data, void **handle); -static int fat_unbind(void *handle); +static int fat_unbind(void *handle, FAR struct inode **blkdriver); static int fat_unlink(struct inode *mountpt, const char *relpath); static int fat_mkdir(struct inode *mountpt, const char *relpath, mode_t mode); @@ -1517,7 +1517,7 @@ static int fat_bind(FAR struct inode *blkdriver, const void *data, * ****************************************************************************/ -static int fat_unbind(void *handle) +static int fat_unbind(void *handle, FAR struct inode **blkdriver) { struct fat_mountpt_s *fs = (struct fat_mountpt_s*)handle; int ret; @@ -1551,9 +1551,16 @@ static int fat_unbind(void *handle) (void)inode->u.i_bops->close(inode); } - /* Release our reference to the block driver */ + /* 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. + */ - inode_release(inode); + if (blkdriver) + { + *blkdriver = inode; + } } } diff --git a/nuttx/fs/fs_inoderemove.c b/nuttx/fs/fs_inoderemove.c index 633222567..c227cfacc 100644 --- a/nuttx/fs/fs_inoderemove.c +++ b/nuttx/fs/fs_inoderemove.c @@ -136,6 +136,7 @@ STATUS inode_remove(const char *path) */ node->i_flags |= FSNODEFLAG_DELETED; + return -EBUSY; } else { @@ -149,5 +150,5 @@ STATUS inode_remove(const char *path) /* The node does not exist or it has references */ - return ERROR; + return -ENOENT; } diff --git a/nuttx/fs/fs_umount.c b/nuttx/fs/fs_umount.c index 347719982..d6a6351f4 100644 --- a/nuttx/fs/fs_umount.c +++ b/nuttx/fs/fs_umount.c @@ -92,7 +92,8 @@ int umount(const char *target) { FAR struct inode *mountpt_inode; - int errcode; + FAR struct inode *blkdrvr_inode = NULL; + int errcode = OK; int status; /* Verify required pointer arguments */ @@ -138,7 +139,7 @@ int umount(const char *target) */ inode_semtake(); /* Hold the semaphore through the unbind logic */ - status = mountpt_inode->u.i_mops->unbind( mountpt_inode->i_private ); + status = mountpt_inode->u.i_mops->unbind( mountpt_inode->i_private, &blkdrvr_inode); if (status < 0) { /* The inode is unhappy with the blkdrvr for some reason */ @@ -156,15 +157,39 @@ int umount(const char *target) mountpt_inode->i_private = NULL; - /* Remove the inode */ + /* Successfully unbound, remove the mountpoint inode from + * the inode tree. The inode will not be deleted yet because + * there is still at least reference on it (from the mount) + */ - inode_semgive(); /* Need to release for inode_release */ - inode_release(mountpt_inode); - - inode_semtake(); /* Need to hold for inode_remove */ status = inode_remove(target); inode_semgive(); - return status; + + /* The return value of -EBUSY is normal (in fact, it should + * not be OK) + */ + + if (status != OK && status != -EBUSY) + { + errcode = -status; + goto errout_with_mountpt; + } + + /* Release the mountpoint inode and any block driver inode + * returned by the file system unbind above. This should cause + * the inode to be deleted (unless there are other references) + */ + + inode_release(mountpt_inode); + + /* Did the unbind method return a contained block driver */ + + if (blkdrvr_inode) + { + inode_release(blkdrvr_inode); + } + + return OK; /* A lot of goto's! But they make the error handling much simpler */ @@ -172,6 +197,10 @@ int umount(const char *target) inode_semgive(); errout_with_mountpt: inode_release(mountpt_inode); + if (blkdrvr_inode) + { + inode_release(blkdrvr_inode); + } errout: *get_errno_ptr() = errcode; return ERROR; diff --git a/nuttx/include/nuttx/fs.h b/nuttx/include/nuttx/fs.h index efb3251f2..69d093716 100644 --- a/nuttx/include/nuttx/fs.h +++ b/nuttx/include/nuttx/fs.h @@ -157,7 +157,7 @@ struct mountpt_operations /* General volume-related mountpoint operations: */ int (*bind)(FAR struct inode *blkdriver, const void *data, void **handle); - int (*unbind)(void *handle); + int (*unbind)(void *handle, FAR struct inode **blkdriver); int (*unlink)(struct inode *mountpt, const char *relpath); int (*mkdir)(struct inode *mountpt, const char *relpath, mode_t mode);