This is an automated email from the ASF dual-hosted git repository. xiaoxiang pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/nuttx.git
commit 09a9611ae9b82f7892f59bdcb5abb36221f64c36 Author: dongjiuzhu1 <[email protected]> AuthorDate: Sun Sep 29 22:00:04 2024 +0800 fs/inode: using rwsem lock as inode_lock to avoid deadlock Example: When executing "df -h" on Core A to view mount information, this process will traverse inode nodes, thereby holding the inode_lock. Since the inode type of the mount point may be rpmsgfs, it will fetch statfs information from another Core B. Meanwhile, rcS on Core B needs to obtain file information from Core A, which will be achieved by fetching stat information through rpmsgfs. When this message arrives at Core A, a deadlock can occur between Core A's rptun ap and nsh task. However, both of these places involve read operations only, thus a reader-writer lock can be utilized to prevent such a deadlock. Signed-off-by: dongjiuzhu1 <[email protected]> --- fs/driver/fs_registerblockdriver.c | 7 +----- fs/driver/fs_registerdriver.c | 7 +----- fs/driver/fs_registermtddriver.c | 7 +----- fs/driver/fs_registerpipedriver.c | 7 +----- fs/driver/fs_unregisterblockdriver.c | 14 ++++------- fs/driver/fs_unregisterdriver.c | 14 ++++------- fs/driver/fs_unregistermtddriver.c | 14 ++++------- fs/driver/fs_unregisterpipedriver.c | 14 ++++------- fs/inode/fs_foreachinode.c | 18 +++++--------- fs/inode/fs_inode.c | 46 ++++++++++++++++++++++++++---------- fs/inode/fs_inodeaddref.c | 4 +--- fs/inode/fs_inodefind.c | 7 +----- fs/inode/inode.h | 26 +++++++++++++++++--- fs/mount/fs_automount.c | 8 ++----- fs/mount/fs_mount.c | 9 ++----- fs/mount/fs_umount2.c | 7 +----- fs/mqueue/mq_open.c | 7 +----- fs/mqueue/mq_unlink.c | 7 +----- fs/semaphore/sem_open.c | 7 +----- fs/semaphore/sem_unlink.c | 7 +----- fs/shm/shm_open.c | 8 +------ fs/shm/shm_unlink.c | 8 +------ fs/shm/shmfs.c | 36 +++++++++++++--------------- fs/vfs/fs_mkdir.c | 8 +------ fs/vfs/fs_pseudofile.c | 8 +------ fs/vfs/fs_rename.c | 7 +----- fs/vfs/fs_rmdir.c | 8 +------ fs/vfs/fs_symlink.c | 10 +------- fs/vfs/fs_unlink.c | 7 +----- 29 files changed, 117 insertions(+), 220 deletions(-) diff --git a/fs/driver/fs_registerblockdriver.c b/fs/driver/fs_registerblockdriver.c index 63bedb73f6..6c9308e407 100644 --- a/fs/driver/fs_registerblockdriver.c +++ b/fs/driver/fs_registerblockdriver.c @@ -74,12 +74,7 @@ int register_blockdriver(FAR const char *path, * valid data. */ - ret = inode_lock(); - if (ret < 0) - { - return ret; - } - + inode_lock(); ret = inode_reserve(path, mode, &node); if (ret >= 0) { diff --git a/fs/driver/fs_registerdriver.c b/fs/driver/fs_registerdriver.c index b3db8b9bde..d7bdb43f23 100644 --- a/fs/driver/fs_registerdriver.c +++ b/fs/driver/fs_registerdriver.c @@ -73,12 +73,7 @@ int register_driver(FAR const char *path, * will have a momentarily bad structure. */ - ret = inode_lock(); - if (ret < 0) - { - return ret; - } - + inode_lock(); ret = inode_reserve(path, mode, &node); if (ret >= 0) { diff --git a/fs/driver/fs_registermtddriver.c b/fs/driver/fs_registermtddriver.c index 8eb76b13d9..5d8cff872c 100644 --- a/fs/driver/fs_registermtddriver.c +++ b/fs/driver/fs_registermtddriver.c @@ -74,12 +74,7 @@ int register_mtddriver(FAR const char *path, FAR struct mtd_dev_s *mtd, * valid data. */ - ret = inode_lock(); - if (ret < 0) - { - return ret; - } - + inode_lock(); ret = inode_reserve(path, mode, &node); if (ret >= 0) { diff --git a/fs/driver/fs_registerpipedriver.c b/fs/driver/fs_registerpipedriver.c index ed5f48d8a1..fccb484f2a 100644 --- a/fs/driver/fs_registerpipedriver.c +++ b/fs/driver/fs_registerpipedriver.c @@ -72,12 +72,7 @@ int register_pipedriver(FAR const char *path, * will have a momentarily bad structure. */ - ret = inode_lock(); - if (ret < 0) - { - return ret; - } - + inode_lock(); ret = inode_reserve(path, mode, &node); if (ret >= 0) { diff --git a/fs/driver/fs_unregisterblockdriver.c b/fs/driver/fs_unregisterblockdriver.c index 73cd6982b8..22df0fed4f 100644 --- a/fs/driver/fs_unregisterblockdriver.c +++ b/fs/driver/fs_unregisterblockdriver.c @@ -45,17 +45,11 @@ int unregister_blockdriver(FAR const char *path) { int ret; - ret = inode_lock(); - if (ret >= 0) - { - ret = inode_remove(path); - inode_unlock(); + inode_lock(); + ret = inode_remove(path); + inode_unlock(); #ifdef CONFIG_FS_NOTIFY - notify_unlink(path); + notify_unlink(path); #endif - return OK; - } - - inode_unlock(); return ret; } diff --git a/fs/driver/fs_unregisterdriver.c b/fs/driver/fs_unregisterdriver.c index 9b40c1f63f..0dca9584d2 100644 --- a/fs/driver/fs_unregisterdriver.c +++ b/fs/driver/fs_unregisterdriver.c @@ -55,17 +55,11 @@ int unregister_driver(FAR const char *path) /* If unlink failed, only remove inode. */ - ret = inode_lock(); - if (ret >= 0) - { - ret = inode_remove(path); - inode_unlock(); + inode_lock(); + ret = inode_remove(path); + inode_unlock(); #ifdef CONFIG_FS_NOTIFY - notify_unlink(path); + notify_unlink(path); #endif - return OK; - } - - inode_unlock(); return ret; } diff --git a/fs/driver/fs_unregistermtddriver.c b/fs/driver/fs_unregistermtddriver.c index b99a46bc91..724d569acb 100644 --- a/fs/driver/fs_unregistermtddriver.c +++ b/fs/driver/fs_unregistermtddriver.c @@ -46,17 +46,11 @@ int unregister_mtddriver(FAR const char *path) { int ret; - ret = inode_lock(); - if (ret >= 0) - { - ret = inode_remove(path); - inode_unlock(); + inode_lock(); + ret = inode_remove(path); + inode_unlock(); #ifdef CONFIG_FS_NOTIFY - notify_unlink(path); + notify_unlink(path); #endif - return OK; - } - - inode_unlock(); return ret; } diff --git a/fs/driver/fs_unregisterpipedriver.c b/fs/driver/fs_unregisterpipedriver.c index f29b90ae58..c89ce1b663 100644 --- a/fs/driver/fs_unregisterpipedriver.c +++ b/fs/driver/fs_unregisterpipedriver.c @@ -47,18 +47,12 @@ int unregister_pipedriver(FAR const char *path) { int ret; - ret = inode_lock(); - if (ret >= 0) - { - ret = inode_remove(path); - inode_unlock(); + inode_lock(); + ret = inode_remove(path); + inode_unlock(); #ifdef CONFIG_FS_NOTIFY - notify_unlink(path); + notify_unlink(path); #endif - return OK; - } - - inode_unlock(); return ret; } diff --git a/fs/inode/fs_foreachinode.c b/fs/inode/fs_foreachinode.c index bfe294a475..3700a7d96c 100644 --- a/fs/inode/fs_foreachinode.c +++ b/fs/inode/fs_foreachinode.c @@ -185,12 +185,9 @@ int foreach_inode(foreach_inode_t handler, FAR void *arg) /* Start the recursion at the root inode */ - ret = inode_lock(); - if (ret >= 0) - { - ret = foreach_inodelevel(g_root_inode->i_child, info); - inode_unlock(); - } + inode_rlock(); + ret = foreach_inodelevel(g_root_inode->i_child, info); + inode_runlock(); /* Free the info structure and return the result */ @@ -209,12 +206,9 @@ int foreach_inode(foreach_inode_t handler, FAR void *arg) /* Start the recursion at the root inode */ - ret = inode_lock(); - if (ret >= 0) - { - ret = foreach_inodelevel(g_root_inode->i_child, &info); - inode_unlock(); - } + inode_rlock(); + ret = foreach_inodelevel(g_root_inode->i_child, &info); + inode_runlock(); return ret; diff --git a/fs/inode/fs_inode.c b/fs/inode/fs_inode.c index 9fb5ace69e..d2c392af7c 100644 --- a/fs/inode/fs_inode.c +++ b/fs/inode/fs_inode.c @@ -22,14 +22,8 @@ * Included Files ****************************************************************************/ -#include <nuttx/config.h> - -#include <unistd.h> -#include <assert.h> -#include <errno.h> - #include <nuttx/fs/fs.h> -#include <nuttx/mutex.h> +#include <nuttx/rwsem.h> #include "inode/inode.h" @@ -45,7 +39,7 @@ * Private Data ****************************************************************************/ -static rmutex_t g_inode_lock = NXRMUTEX_INITIALIZER; +static rw_semaphore_t g_inode_lock = RWSEM_INITIALIZER; /**************************************************************************** * Public Functions @@ -71,24 +65,50 @@ void inode_initialize(void) * Name: inode_lock * * Description: - * Get exclusive access to the in-memory inode tree (g_inode_sem). + * Get writeable exclusive access to the in-memory inode tree. + * + ****************************************************************************/ + +void inode_lock(void) +{ + down_write(&g_inode_lock); +} + +/**************************************************************************** + * Name: inode_rlock + * + * Description: + * Get readable exclusive access to the in-memory inode tree. * ****************************************************************************/ -int inode_lock(void) +void inode_rlock(void) { - return nxrmutex_lock(&g_inode_lock); + down_read(&g_inode_lock); } /**************************************************************************** * Name: inode_unlock * * Description: - * Relinquish exclusive access to the in-memory inode tree (g_inode_sem). + * Relinquish writeable exclusive access to the in-memory inode tree. * ****************************************************************************/ void inode_unlock(void) { - DEBUGVERIFY(nxrmutex_unlock(&g_inode_lock)); + up_write(&g_inode_lock); +} + +/**************************************************************************** + * Name: inode_runlock + * + * Description: + * Relinquish read exclusive access to the in-memory inode tree. + * + ****************************************************************************/ + +void inode_runlock(void) +{ + up_read(&g_inode_lock); } diff --git a/fs/inode/fs_inodeaddref.c b/fs/inode/fs_inodeaddref.c index 9187a8fae0..af3dacd59b 100644 --- a/fs/inode/fs_inodeaddref.c +++ b/fs/inode/fs_inodeaddref.c @@ -43,12 +43,10 @@ int inode_addref(FAR struct inode *inode) { - int ret = OK; - if (inode) { atomic_fetch_add(&inode->i_crefs, 1); } - return ret; + return OK; } diff --git a/fs/inode/fs_inodefind.c b/fs/inode/fs_inodefind.c index 47fe84df95..71044c67a8 100644 --- a/fs/inode/fs_inodefind.c +++ b/fs/inode/fs_inodefind.c @@ -55,12 +55,7 @@ int inode_find(FAR struct inode_search_s *desc) * references on the node. */ - ret = inode_lock(); - if (ret < 0) - { - return ret; - } - + inode_lock(); ret = inode_search(desc); if (ret >= 0) { diff --git a/fs/inode/inode.h b/fs/inode/inode.h index aa92bb27c2..e6aa04ac18 100644 --- a/fs/inode/inode.h +++ b/fs/inode/inode.h @@ -170,22 +170,42 @@ void inode_initialize(void); * Name: inode_lock * * Description: - * Get exclusive access to the in-memory inode tree (tree_sem). + * Get writeable exclusive access to the in-memory inode tree. * ****************************************************************************/ -int inode_lock(void); +void inode_lock(void); + +/**************************************************************************** + * Name: inode_rlock + * + * Description: + * Get readable exclusive access to the in-memory inode tree. + * + ****************************************************************************/ + +void inode_rlock(void); /**************************************************************************** * Name: inode_unlock * * Description: - * Relinquish exclusive access to the in-memory inode tree (tree_sem). + * Relinquish writeable exclusive access to the in-memory inode tree. * ****************************************************************************/ void inode_unlock(void); +/**************************************************************************** + * Name: inode_runlock + * + * Description: + * Relinquish read exclusive access to the in-memory inode tree. + * + ****************************************************************************/ + +void inode_runlock(void); + /**************************************************************************** * Name: inode_search * diff --git a/fs/mount/fs_automount.c b/fs/mount/fs_automount.c index bba3aa949f..a18c2279bc 100644 --- a/fs/mount/fs_automount.c +++ b/fs/mount/fs_automount.c @@ -402,11 +402,7 @@ static int automount_findinode(FAR const char *path) /* Get exclusive access to the in-memory inode tree. */ - ret = inode_lock(); - if (ret < 0) - { - return ret; - } + inode_rlock(); /* Find the inode */ @@ -440,7 +436,7 @@ static int automount_findinode(FAR const char *path) /* Relinquish our exclusive access to the inode try and return the result */ - inode_unlock(); + inode_runlock(); RELEASE_SEARCH(&desc); return ret; } diff --git a/fs/mount/fs_mount.c b/fs/mount/fs_mount.c index f266083132..ffa29ca937 100644 --- a/fs/mount/fs_mount.c +++ b/fs/mount/fs_mount.c @@ -367,12 +367,7 @@ int nx_mount(FAR const char *source, FAR const char *target, goto errout; } - ret = inode_lock(); - if (ret < 0) - { - goto errout_with_inode; - } - + inode_lock(); #ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS /* Check if the inode already exists */ @@ -436,7 +431,7 @@ int nx_mount(FAR const char *source, FAR const char *target, #else ret = mops->bind(NULL, data, &fshandle); #endif - DEBUGVERIFY(inode_lock() >= 0); + inode_lock(); if (ret < 0) { /* The inode is unhappy with the driver for some reason. Back out diff --git a/fs/mount/fs_umount2.c b/fs/mount/fs_umount2.c index 76ee449ece..632f85d4ae 100644 --- a/fs/mount/fs_umount2.c +++ b/fs/mount/fs_umount2.c @@ -110,12 +110,7 @@ int nx_umount2(FAR const char *target, unsigned int flags) /* Hold the semaphore through the unbind logic */ - ret = inode_lock(); - if (ret < 0) - { - goto errout_with_mountpt; - } - + inode_lock(); ret = mountpt_inode->u.i_mops->unbind(mountpt_inode->i_private, &blkdrvr_inode, flags); if (ret < 0) diff --git a/fs/mqueue/mq_open.c b/fs/mqueue/mq_open.c index 84d1454558..73eb895c19 100644 --- a/fs/mqueue/mq_open.c +++ b/fs/mqueue/mq_open.c @@ -285,12 +285,7 @@ static int file_mq_vopen(FAR struct file *mq, FAR const char *mq_name, /* Create an inode in the pseudo-filesystem at this path */ - ret = inode_lock(); - if (ret < 0) - { - goto errout_with_lock; - } - + inode_lock(); ret = inode_reserve(fullpath, mode, &inode); inode_unlock(); diff --git a/fs/mqueue/mq_unlink.c b/fs/mqueue/mq_unlink.c index d6a1c14e58..271fb213c9 100644 --- a/fs/mqueue/mq_unlink.c +++ b/fs/mqueue/mq_unlink.c @@ -137,12 +137,7 @@ int file_mq_unlink(FAR const char *mq_name) * functioning as a directory and the directory is not empty. */ - ret = inode_lock(); - if (ret < 0) - { - goto errout_with_inode; - } - + inode_lock(); if (inode->i_child != NULL) { ret = -ENOTEMPTY; diff --git a/fs/semaphore/sem_open.c b/fs/semaphore/sem_open.c index 406244b31d..e063000895 100644 --- a/fs/semaphore/sem_open.c +++ b/fs/semaphore/sem_open.c @@ -178,12 +178,7 @@ int nxsem_open(FAR sem_t **sem, FAR const char *name, int oflags, ...) * inode will be created with a reference count of zero. */ - ret = inode_lock(); - if (ret < 0) - { - goto errout_with_lock; - } - + inode_lock(); ret = inode_reserve(fullpath, mode, &inode); inode_unlock(); diff --git a/fs/semaphore/sem_unlink.c b/fs/semaphore/sem_unlink.c index 74ff982ea9..ff92a1a9c9 100644 --- a/fs/semaphore/sem_unlink.c +++ b/fs/semaphore/sem_unlink.c @@ -102,12 +102,7 @@ int nxsem_unlink(FAR const char *name) * functioning as a directory and the directory is not empty. */ - ret = inode_lock(); - if (ret < 0) - { - goto errout_with_inode; - } - + inode_lock(); if (inode->i_child != NULL) { ret = -ENOTEMPTY; diff --git a/fs/shm/shm_open.c b/fs/shm/shm_open.c index cf980497e7..431ba2e0b7 100644 --- a/fs/shm/shm_open.c +++ b/fs/shm/shm_open.c @@ -81,12 +81,7 @@ static int file_shm_open(FAR struct file *shm, FAR const char *name, SETUP_SEARCH(&desc, fullpath, false); - ret = inode_lock(); - if (ret < 0) - { - goto errout_with_search; - } - + inode_lock(); ret = inode_find(&desc); if (ret >= 0) { @@ -148,7 +143,6 @@ static int file_shm_open(FAR struct file *shm, FAR const char *name, errout_with_sem: inode_unlock(); -errout_with_search: RELEASE_SEARCH(&desc); #ifdef CONFIG_FS_NOTIFY if (ret >= 0) diff --git a/fs/shm/shm_unlink.c b/fs/shm/shm_unlink.c index c35235cacd..84ac6a9bc7 100644 --- a/fs/shm/shm_unlink.c +++ b/fs/shm/shm_unlink.c @@ -77,12 +77,7 @@ static int file_shm_unlink(FAR const char *name) SETUP_SEARCH(&desc, fullpath, false); - ret = inode_lock(); - if (ret < 0) - { - goto errout_with_search; - } - + inode_lock(); ret = inode_find(&desc); if (ret < 0) { @@ -139,7 +134,6 @@ errout_with_inode: inode_release(inode); errout_with_sem: inode_unlock(); -errout_with_search: RELEASE_SEARCH(&desc); #ifdef CONFIG_FS_NOTIFY if (ret >= 0) diff --git a/fs/shm/shmfs.c b/fs/shm/shmfs.c index cf826302c5..fb0b94116b 100644 --- a/fs/shm/shmfs.c +++ b/fs/shm/shmfs.c @@ -215,40 +215,36 @@ static int shmfs_close(FAR struct file *filep) static int shmfs_truncate(FAR struct file *filep, off_t length) { FAR struct shmfs_object_s *object; - int ret; + int ret = 0; if (length == 0) { return -EINVAL; } - ret = inode_lock(); - if (ret >= 0) + inode_lock(); + object = filep->f_inode->i_private; + if (!object) { - object = filep->f_inode->i_private; - if (!object) + filep->f_inode->i_private = shmfs_alloc_object(length); + if (!filep->f_inode->i_private) { - filep->f_inode->i_private = shmfs_alloc_object(length); - if (!filep->f_inode->i_private) - { - filep->f_inode->i_size = 0; - ret = -EFAULT; - } - else - { - filep->f_inode->i_size = length; - } + filep->f_inode->i_size = 0; + ret = -EFAULT; } - else if (object->length != length) + else { - /* This doesn't support resize */ - - ret = -EINVAL; + filep->f_inode->i_size = length; } + } + else if (object->length != length) + { + /* This doesn't support resize */ - inode_unlock(); + ret = -EINVAL; } + inode_unlock(); return ret; } diff --git a/fs/vfs/fs_mkdir.c b/fs/vfs/fs_mkdir.c index a5464bdb9b..db8e1a4279 100644 --- a/fs/vfs/fs_mkdir.c +++ b/fs/vfs/fs_mkdir.c @@ -139,13 +139,7 @@ int mkdir(const char *pathname, mode_t mode) * count of zero. */ - ret = inode_lock(); - if (ret < 0) - { - errcode = -ret; - goto errout_with_search; - } - + inode_lock(); ret = inode_reserve(pathname, mode, &inode); inode_unlock(); diff --git a/fs/vfs/fs_pseudofile.c b/fs/vfs/fs_pseudofile.c index 13f349fe54..57b2601beb 100644 --- a/fs/vfs/fs_pseudofile.c +++ b/fs/vfs/fs_pseudofile.c @@ -484,12 +484,7 @@ int pseudofile_create(FAR struct inode **node, FAR const char *path, nxmutex_init(&pf->lock); - ret = inode_lock(); - if (ret < 0) - { - goto lock_err; - } - + inode_lock(); ret = inode_reserve(path, mode, node); if (ret < 0) { @@ -508,7 +503,6 @@ int pseudofile_create(FAR struct inode **node, FAR const char *path, reserve_err: inode_unlock(); -lock_err: nxmutex_destroy(&pf->lock); fs_heap_free(pf); return ret; diff --git a/fs/vfs/fs_rename.c b/fs/vfs/fs_rename.c index d322e8b98d..a41e8542ad 100644 --- a/fs/vfs/fs_rename.c +++ b/fs/vfs/fs_rename.c @@ -178,12 +178,7 @@ next_subdir: * of zero. */ - ret = inode_lock(); - if (ret < 0) - { - goto errout; - } - + inode_lock(); ret = inode_reserve(newpath, 0777, &newinode); if (ret < 0) { diff --git a/fs/vfs/fs_rmdir.c b/fs/vfs/fs_rmdir.c index d144d6c624..4a8d1eb577 100644 --- a/fs/vfs/fs_rmdir.c +++ b/fs/vfs/fs_rmdir.c @@ -134,13 +134,7 @@ int rmdir(FAR const char *pathname) * -EBUSY to indicate that the inode was not deleted now. */ - ret = inode_lock(); - if (ret < 0) - { - errcode = -ret; - goto errout_with_inode; - } - + inode_lock(); ret = inode_remove(pathname); inode_unlock(); diff --git a/fs/vfs/fs_symlink.c b/fs/vfs/fs_symlink.c index 9c6de4f994..e0b21b3819 100644 --- a/fs/vfs/fs_symlink.c +++ b/fs/vfs/fs_symlink.c @@ -141,17 +141,9 @@ int symlink(FAR const char *path1, FAR const char *path2) * count of zero. */ - ret = inode_lock(); - if (ret < 0) - { - lib_free(newpath2); - errcode = -ret; - goto errout_with_search; - } - + inode_lock(); ret = inode_reserve(path2, 0777, &inode); inode_unlock(); - if (ret < 0) { lib_free(newpath2); diff --git a/fs/vfs/fs_unlink.c b/fs/vfs/fs_unlink.c index b68b4da60d..998ab70831 100644 --- a/fs/vfs/fs_unlink.c +++ b/fs/vfs/fs_unlink.c @@ -171,12 +171,7 @@ int nx_unlink(FAR const char *pathname) * return -EBUSY to indicate that the inode was not deleted now. */ - ret = inode_lock(); - if (ret < 0) - { - goto errout_with_inode; - } - + inode_lock(); ret = inode_remove(pathname); inode_unlock();
