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 <dongjiuzhu1@xiaomi.com>

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)
     {

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)
     {

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)
     {

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)
     {

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();
-#ifdef CONFIG_FS_NOTIFY
-      notify_unlink(path);
-#endif
-      return OK;
-    }
-
+  inode_lock();
+  ret = inode_remove(path);
   inode_unlock();
+#ifdef CONFIG_FS_NOTIFY
+  notify_unlink(path);
+#endif
   return ret;
 }

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();
-#ifdef CONFIG_FS_NOTIFY
-      notify_unlink(path);
-#endif
-      return OK;
-    }
-
+  inode_lock();
+  ret = inode_remove(path);
   inode_unlock();
+#ifdef CONFIG_FS_NOTIFY
+  notify_unlink(path);
+#endif
   return ret;
 }

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();
-#ifdef CONFIG_FS_NOTIFY
-      notify_unlink(path);
-#endif
-      return OK;
-    }
-
+  inode_lock();
+  ret = inode_remove(path);
   inode_unlock();
+#ifdef CONFIG_FS_NOTIFY
+  notify_unlink(path);
+#endif
   return ret;
 }

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();
-#ifdef CONFIG_FS_NOTIFY
-      notify_unlink(path);
-#endif
-      return OK;
-    }
-
+  inode_lock();
+  ret = inode_remove(path);
   inode_unlock();
+#ifdef CONFIG_FS_NOTIFY
+  notify_unlink(path);
+#endif
   return ret;
 }
 

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;
 

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.
  *
  ****************************************************************************/
 
-int inode_lock(void)
+void inode_lock(void)
 {
-  return nxrmutex_lock(&g_inode_lock);
+  down_write(&g_inode_lock);
+}
+
+/****************************************************************************
+ * Name: inode_rlock
+ *
+ * Description:
+ *   Get readable exclusive access to the in-memory inode tree.
+ *
+ ****************************************************************************/
+
+void inode_rlock(void)
+{
+  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);
 }

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;
 }

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)
     {

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
  *

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;
 }

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

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)

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();
 

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;

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();
 

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;

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)

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)

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;
 }
 

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();
 

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;

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)
     {

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();
 

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);

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();