You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by xi...@apache.org on 2023/11/13 01:01:18 UTC

(nuttx) branch master updated (66ccaed5ce -> 74d698f439)

This is an automated email from the ASF dual-hosted git repository.

xiaoxiang pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/nuttx.git


    from 66ccaed5ce stm32f4/stm32f401rc-rs485: add support to  userleds
     new 0a567bbae4 fs/inode: improve the performance of get file pointer
     new 3b2c585ab7 fs/inode: add common function to get file count from list
     new 74d698f439 spinlock: add __cplusplus check to avoid build break with C++

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 fs/inode/fs_files.c       | 294 +++++++++++++++++++++-------------------------
 fs/procfs/fs_procfsproc.c |  70 +++++------
 include/nuttx/fs/fs.h     |  16 ++-
 include/nuttx/spinlock.h  |  29 +++--
 4 files changed, 208 insertions(+), 201 deletions(-)


(nuttx) 02/03: fs/inode: add common function to get file count from list

Posted by xi...@apache.org.
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 3b2c585ab7b5bf2cca2a0eec787750340b334cc7
Author: chao an <an...@xiaomi.com>
AuthorDate: Sun Nov 12 21:04:07 2023 +0800

    fs/inode: add common function to get file count from list
    
    common function to get file count from file list
    
    Signed-off-by: chao an <an...@xiaomi.com>
---
 fs/inode/fs_files.c       | 35 ++++++++++++++++++++----
 fs/procfs/fs_procfsproc.c | 70 +++++++++++++++++++++++------------------------
 include/nuttx/fs/fs.h     | 13 +++++++++
 3 files changed, 78 insertions(+), 40 deletions(-)

diff --git a/fs/inode/fs_files.c b/fs/inode/fs_files.c
index 51b8437e23..48482526fa 100644
--- a/fs/inode/fs_files.c
+++ b/fs/inode/fs_files.c
@@ -98,7 +98,7 @@ static int files_extend(FAR struct filelist *list, size_t row)
       return 0;
     }
 
-  if (row * CONFIG_NFILE_DESCRIPTORS_PER_BLOCK > OPEN_MAX)
+  if (files_countlist(list) > OPEN_MAX)
     {
       return -EMFILE;
     }
@@ -214,6 +214,7 @@ static int nx_dup3_from_tcb(FAR struct tcb_s *tcb, int fd1, int fd2,
   FAR struct filelist *list;
   FAR struct file *filep;
   FAR struct file  file;
+  int count;
   int ret;
 
   if (fd1 == fd2)
@@ -228,15 +229,17 @@ static int nx_dup3_from_tcb(FAR struct tcb_s *tcb, int fd1, int fd2,
 
   list = nxsched_get_files_from_tcb(tcb);
 
+  count = files_countlist(list);
+
   /* Get the file descriptor list.  It should not be NULL in this context. */
 
-  if (fd1 < 0 || fd1 >= CONFIG_NFILE_DESCRIPTORS_PER_BLOCK * list->fl_rows ||
+  if (fd1 < 0 || fd1 >= count ||
       fd2 < 0)
     {
       return -EBADF;
     }
 
-  if (fd2 >= CONFIG_NFILE_DESCRIPTORS_PER_BLOCK * list->fl_rows)
+  if (fd2 >= count)
     {
       ret = files_extend(list, fd2 / CONFIG_NFILE_DESCRIPTORS_PER_BLOCK + 1);
       if (ret < 0)
@@ -315,6 +318,28 @@ void files_releaselist(FAR struct filelist *list)
   kmm_free(list->fl_files);
 }
 
+/****************************************************************************
+ * Name: files_countlist
+ *
+ * Description:
+ *   Given a file descriptor, return the corresponding instance of struct
+ *   file.
+ *
+ * Input Parameters:
+ *   fd    - The file descriptor
+ *   filep - The location to return the struct file instance
+ *
+ * Returned Value:
+ *   Zero (OK) is returned on success; a negated errno value is returned on
+ *   any failure.
+ *
+ ****************************************************************************/
+
+int files_countlist(FAR struct filelist *list)
+{
+  return list->fl_rows * CONFIG_NFILE_DESCRIPTORS_PER_BLOCK;
+}
+
 /****************************************************************************
  * Name: file_allocate_from_tcb
  *
@@ -562,7 +587,7 @@ int fs_getfilep(int fd, FAR struct file **filep)
       return -EAGAIN;
     }
 
-  if (fd < 0 || fd >= list->fl_rows * CONFIG_NFILE_DESCRIPTORS_PER_BLOCK)
+  if (fd < 0 || fd >= files_countlist(list))
     {
       return -EBADF;
     }
@@ -714,7 +739,7 @@ int nx_close_from_tcb(FAR struct tcb_s *tcb, int fd)
 
   /* Perform the protected close operation */
 
-  if (fd < 0 || fd >= list->fl_rows * CONFIG_NFILE_DESCRIPTORS_PER_BLOCK)
+  if (fd < 0 || fd >= files_countlist(list))
     {
       return -EBADF;
     }
diff --git a/fs/procfs/fs_procfsproc.c b/fs/procfs/fs_procfsproc.c
index 7e18d3b7e6..04c83d53f7 100644
--- a/fs/procfs/fs_procfsproc.c
+++ b/fs/procfs/fs_procfsproc.c
@@ -1188,17 +1188,24 @@ static ssize_t proc_groupfd(FAR struct proc_file_s *procfile,
                             size_t buflen, off_t offset)
 {
   FAR struct task_group_s *group = tcb->group;
-  FAR struct file *file;
+  FAR struct file *filep;
   char path[PATH_MAX];
   size_t remaining;
   size_t linesize;
   size_t copysize;
   size_t totalsize;
+  int count;
+  int ret;
   int i;
-  int j;
 
   DEBUGASSERT(group != NULL);
 
+  count = files_countlist(&group->tg_filelist);
+  if (count == 0)
+    {
+      return 0;
+    }
+
   remaining = buflen;
   totalsize = 0;
 
@@ -1219,41 +1226,34 @@ static ssize_t proc_groupfd(FAR struct proc_file_s *procfile,
 
   /* Examine each open file descriptor */
 
-  for (i = 0; i < group->tg_filelist.fl_rows; i++)
+  for (i = 0; i < count; i++)
     {
-      for (j = 0, file = group->tg_filelist.fl_files[i];
-           j < CONFIG_NFILE_DESCRIPTORS_PER_BLOCK;
-           j++, file++)
+      ret = fs_getfilep(i, &filep);
+      if (ret != OK || filep == NULL)
+        {
+          continue;
+        }
+
+      if (file_ioctl(filep, FIOC_FILEPATH, path) < 0)
+        {
+          path[0] = '\0';
+        }
+
+      linesize   = procfs_snprintf(procfile->line, STATUS_LINELEN,
+                                   "%-3d %-7d %-4x %-9ld %s\n",
+                                   i, filep->f_oflags,
+                                   INODE_GET_TYPE(filep->f_inode),
+                                   (long)filep->f_pos, path);
+      copysize   = procfs_memcpy(procfile->line, linesize,
+                                 buffer, remaining, &offset);
+
+      totalsize += copysize;
+      buffer    += copysize;
+      remaining -= copysize;
+
+      if (totalsize >= buflen)
         {
-          /* Is there an inode associated with the file descriptor? */
-
-          if (file->f_inode == NULL)
-            {
-              continue;
-            }
-
-          if (file_ioctl(file, FIOC_FILEPATH, path) < 0)
-            {
-              path[0] = '\0';
-            }
-
-          linesize   = procfs_snprintf(procfile->line, STATUS_LINELEN,
-                                       "%-3d %-7d %-4x %-9ld %s\n",
-                                       i * CONFIG_NFILE_DESCRIPTORS_PER_BLOCK
-                                       + j, file->f_oflags,
-                                       INODE_GET_TYPE(file->f_inode),
-                                       (long)file->f_pos, path);
-          copysize   = procfs_memcpy(procfile->line, linesize,
-                                     buffer, remaining, &offset);
-
-          totalsize += copysize;
-          buffer    += copysize;
-          remaining -= copysize;
-
-          if (totalsize >= buflen)
-            {
-              return totalsize;
-            }
+          return totalsize;
         }
     }
 
diff --git a/include/nuttx/fs/fs.h b/include/nuttx/fs/fs.h
index f944c22d99..7e3813695c 100644
--- a/include/nuttx/fs/fs.h
+++ b/include/nuttx/fs/fs.h
@@ -852,6 +852,19 @@ void files_initlist(FAR struct filelist *list);
 
 void files_releaselist(FAR struct filelist *list);
 
+/****************************************************************************
+ * Name: files_countlist
+ *
+ * Description:
+ *   Get file count from file list
+ *
+ * Returned Value:
+ *   file count of file list
+ *
+ ****************************************************************************/
+
+int files_countlist(FAR struct filelist *list);
+
 /****************************************************************************
  * Name: files_duplist
  *


(nuttx) 01/03: fs/inode: improve the performance of get file pointer

Posted by xi...@apache.org.
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 0a567bbae407d382ace7439977a4151b3c11d80d
Author: chao an <an...@xiaomi.com>
AuthorDate: Wed Nov 8 17:23:44 2023 +0800

    fs/inode: improve the performance of get file pointer
    
    Remove file locks and enter critical sections to protect file lists
    
    Signed-off-by: chao an <an...@xiaomi.com>
---
 fs/inode/fs_files.c   | 265 +++++++++++++++++++++-----------------------------
 include/nuttx/fs/fs.h |   3 +-
 2 files changed, 111 insertions(+), 157 deletions(-)

diff --git a/fs/inode/fs_files.c b/fs/inode/fs_files.c
index bcb7638288..51b8437e23 100644
--- a/fs/inode/fs_files.c
+++ b/fs/inode/fs_files.c
@@ -51,14 +51,47 @@
  * Private Functions
  ****************************************************************************/
 
+/****************************************************************************
+ * Name: files_fget_by_index
+ ****************************************************************************/
+
+static FAR struct file *files_fget_by_index(FAR struct filelist *list,
+                                            int l1, int l2)
+{
+  FAR struct file *filep;
+  irqstate_t flags;
+
+  flags = spin_lock_irqsave(&list->fl_lock);
+
+  filep = &list->fl_files[l1][l2];
+
+  spin_unlock_irqrestore(&list->fl_lock, flags);
+
+  return filep;
+}
+
+/****************************************************************************
+ * Name: files_fget
+ ****************************************************************************/
+
+static FAR struct file *files_fget(FAR struct filelist *list, int fd)
+{
+  return files_fget_by_index(list, fd / CONFIG_NFILE_DESCRIPTORS_PER_BLOCK,
+                             fd % CONFIG_NFILE_DESCRIPTORS_PER_BLOCK);
+}
+
 /****************************************************************************
  * Name: files_extend
  ****************************************************************************/
 
 static int files_extend(FAR struct filelist *list, size_t row)
 {
-  FAR struct file **tmp;
+  FAR struct file **files;
+  uint8_t orig_rows;
+  FAR void *tmp;
+  int flags;
   int i;
+  int j;
 
   if (row <= list->fl_rows)
     {
@@ -70,9 +103,11 @@ static int files_extend(FAR struct filelist *list, size_t row)
       return -EMFILE;
     }
 
-  tmp = kmm_realloc(list->fl_files, sizeof(FAR struct file *) * row);
-  DEBUGASSERT(tmp);
-  if (tmp == NULL)
+  orig_rows = list->fl_rows;
+
+  files = kmm_malloc(sizeof(FAR struct file *) * row);
+  DEBUGASSERT(files);
+  if (files == NULL)
     {
       return -ENFILE;
     }
@@ -80,30 +115,54 @@ static int files_extend(FAR struct filelist *list, size_t row)
   i = list->fl_rows;
   do
     {
-      tmp[i] = kmm_zalloc(sizeof(struct file) *
-                          CONFIG_NFILE_DESCRIPTORS_PER_BLOCK);
-      if (tmp[i] == NULL)
+      files[i] = kmm_zalloc(sizeof(struct file) *
+                            CONFIG_NFILE_DESCRIPTORS_PER_BLOCK);
+      if (files[i] == NULL)
         {
           while (--i >= list->fl_rows)
             {
-              kmm_free(tmp[i]);
+              kmm_free(files[i]);
             }
 
-          kmm_free(tmp);
+          kmm_free(files);
           return -ENFILE;
         }
     }
   while (++i < row);
 
-  list->fl_files = tmp;
-  list->fl_rows = row;
+  flags = spin_lock_irqsave(&list->fl_lock);
 
-  /* Note: If assertion occurs, the fl_rows has a overflow.
-   * And there may be file descriptors leak in system.
+  /* To avoid race condition, if the file list is updated by other threads
+   * and list rows is greater or equal than temp list,
+   * release the obsolete buffers
    */
 
-  DEBUGASSERT(list->fl_rows == row);
-  return 0;
+  if (orig_rows != list->fl_rows && list->fl_rows >= row)
+    {
+      spin_unlock_irqrestore(&list->fl_lock, flags);
+
+      for (j = orig_rows; j < i; j++)
+        {
+          kmm_free(files[i]);
+        }
+
+      kmm_free(files);
+
+      return OK;
+    }
+
+  memcpy(files, list->fl_files,
+         list->fl_rows * sizeof(FAR struct file *));
+
+  tmp = list->fl_files;
+  list->fl_files = files;
+  list->fl_rows = row;
+
+  spin_unlock_irqrestore(&list->fl_lock, flags);
+
+  kmm_free(tmp);
+
+  return OK;
 }
 
 static void task_fssync(FAR struct tcb_s *tcb, FAR void *arg)
@@ -113,10 +172,6 @@ static void task_fssync(FAR struct tcb_s *tcb, FAR void *arg)
   int j;
 
   list = &tcb->group->tg_filelist;
-  if (nxmutex_lock(&list->fl_lock) < 0)
-    {
-      return;
-    }
 
   for (i = 0; i < list->fl_rows; i++)
     {
@@ -124,15 +179,13 @@ static void task_fssync(FAR struct tcb_s *tcb, FAR void *arg)
         {
           FAR struct file *filep;
 
-          filep = &list->fl_files[i][j];
-          if (filep != NULL && filep->f_inode != NULL)
+          filep = files_fget_by_index(list, i, j);
+          if (filep->f_inode != NULL)
             {
               file_fsync(filep);
             }
         }
     }
-
-  nxmutex_unlock(&list->fl_lock);
 }
 
 /****************************************************************************
@@ -183,41 +236,27 @@ static int nx_dup3_from_tcb(FAR struct tcb_s *tcb, int fd1, int fd2,
       return -EBADF;
     }
 
-  ret = nxmutex_lock(&list->fl_lock);
-  if (ret < 0)
-    {
-      /* Probably canceled */
-
-      return ret;
-    }
-
   if (fd2 >= CONFIG_NFILE_DESCRIPTORS_PER_BLOCK * list->fl_rows)
     {
       ret = files_extend(list, fd2 / CONFIG_NFILE_DESCRIPTORS_PER_BLOCK + 1);
       if (ret < 0)
         {
-          nxmutex_unlock(&list->fl_lock);
           return ret;
         }
     }
 
-  filep = &list->fl_files[fd2 / CONFIG_NFILE_DESCRIPTORS_PER_BLOCK]
-                         [fd2 % CONFIG_NFILE_DESCRIPTORS_PER_BLOCK];
+  filep = files_fget(list, fd2);
   memcpy(&file, filep, sizeof(struct file));
   memset(filep, 0,     sizeof(struct file));
 
   /* Perform the dup3 operation */
 
-  ret = file_dup3(&list->fl_files[fd1 / CONFIG_NFILE_DESCRIPTORS_PER_BLOCK]
-                                 [fd1 % CONFIG_NFILE_DESCRIPTORS_PER_BLOCK],
-                  filep, flags);
+  ret = file_dup3(files_fget(list, fd1), filep, flags);
 
 #ifdef CONFIG_FDSAN
   filep->f_tag = file.f_tag;
 #endif
 
-  nxmutex_unlock(&list->fl_lock);
-
   file_close(&file);
 
 #ifdef CONFIG_FDCHECK
@@ -241,10 +280,6 @@ static int nx_dup3_from_tcb(FAR struct tcb_s *tcb, int fd1, int fd2,
 void files_initlist(FAR struct filelist *list)
 {
   DEBUGASSERT(list);
-
-  /* Initialize the list access mutex */
-
-  nxmutex_init(&list->fl_lock);
 }
 
 /****************************************************************************
@@ -278,10 +313,6 @@ void files_releaselist(FAR struct filelist *list)
     }
 
   kmm_free(list->fl_files);
-
-  /* Destroy the mutex */
-
-  nxmutex_destroy(&list->fl_lock);
 }
 
 /****************************************************************************
@@ -302,6 +333,7 @@ int file_allocate_from_tcb(FAR struct tcb_s *tcb, FAR struct inode *inode,
                            bool addref)
 {
   FAR struct filelist *list;
+  FAR struct file *filep;
   int ret;
   int i;
   int j;
@@ -309,15 +341,6 @@ int file_allocate_from_tcb(FAR struct tcb_s *tcb, FAR struct inode *inode,
   /* Get the file descriptor list.  It should not be NULL in this context. */
 
   list = nxsched_get_files_from_tcb(tcb);
-  DEBUGASSERT(list != NULL);
-
-  ret = nxmutex_lock(&list->fl_lock);
-  if (ret < 0)
-    {
-      /* Probably canceled */
-
-      return ret;
-    }
 
   /* Calculate minfd whether is in list->fl_files.
    * if not, allocate a new filechunk.
@@ -329,7 +352,6 @@ int file_allocate_from_tcb(FAR struct tcb_s *tcb, FAR struct inode *inode,
       ret = files_extend(list, i + 1);
       if (ret < 0)
         {
-          nxmutex_unlock(&list->fl_lock);
           return ret;
         }
     }
@@ -341,25 +363,10 @@ int file_allocate_from_tcb(FAR struct tcb_s *tcb, FAR struct inode *inode,
     {
       do
         {
-          if (!list->fl_files[i][j].f_inode)
+          filep = files_fget_by_index(list, i, j);
+          if (filep->f_inode == NULL)
             {
-              list->fl_files[i][j].f_oflags = oflags;
-              list->fl_files[i][j].f_pos    = pos;
-              list->fl_files[i][j].f_inode  = inode;
-              list->fl_files[i][j].f_priv   = priv;
-              nxmutex_unlock(&list->fl_lock);
-
-              if (addref)
-                {
-                  inode_addref(inode);
-                }
-
-#ifdef CONFIG_FDCHECK
-              return
-                fdcheck_protect(i * CONFIG_NFILE_DESCRIPTORS_PER_BLOCK + j);
-#else
-              return i * CONFIG_NFILE_DESCRIPTORS_PER_BLOCK + j;
-#endif
+              goto found;
             }
         }
       while (++j < CONFIG_NFILE_DESCRIPTORS_PER_BLOCK);
@@ -373,15 +380,16 @@ int file_allocate_from_tcb(FAR struct tcb_s *tcb, FAR struct inode *inode,
   ret = files_extend(list, i + 1);
   if (ret < 0)
     {
-      nxmutex_unlock(&list->fl_lock);
       return ret;
     }
 
-  list->fl_files[i][0].f_oflags = oflags;
-  list->fl_files[i][0].f_pos    = pos;
-  list->fl_files[i][0].f_inode  = inode;
-  list->fl_files[i][0].f_priv   = priv;
-  nxmutex_unlock(&list->fl_lock);
+  filep = files_fget_by_index(list, i, 0);
+found:
+
+  filep->f_oflags = oflags;
+  filep->f_pos    = pos;
+  filep->f_inode  = inode;
+  filep->f_priv   = priv;
 
   if (addref)
     {
@@ -389,9 +397,9 @@ int file_allocate_from_tcb(FAR struct tcb_s *tcb, FAR struct inode *inode,
     }
 
 #ifdef CONFIG_FDCHECK
-  return fdcheck_protect(i * CONFIG_NFILE_DESCRIPTORS_PER_BLOCK);
+  return fdcheck_protect(i * CONFIG_NFILE_DESCRIPTORS_PER_BLOCK + j);
 #else
-  return i * CONFIG_NFILE_DESCRIPTORS_PER_BLOCK;
+  return i * CONFIG_NFILE_DESCRIPTORS_PER_BLOCK + j;
 #endif
 }
 
@@ -432,14 +440,6 @@ int files_duplist(FAR struct filelist *plist, FAR struct filelist *clist)
   DEBUGASSERT(clist);
   DEBUGASSERT(plist);
 
-  ret = nxmutex_lock(&plist->fl_lock);
-  if (ret < 0)
-    {
-      /* Probably canceled */
-
-      return ret;
-    }
-
   for (i = 0; i < plist->fl_rows; i++)
     {
       for (j = 0; j < CONFIG_NFILE_DESCRIPTORS_PER_BLOCK; j++)
@@ -456,13 +456,12 @@ int files_duplist(FAR struct filelist *plist, FAR struct filelist *clist)
 
           if (i * CONFIG_NFILE_DESCRIPTORS_PER_BLOCK + j >= 3)
             {
-              goto out;
+              return OK;
             }
 #endif
 
-          filep = &plist->fl_files[i][j];
-
-          if (filep && filep->f_inode == NULL)
+          filep = files_fget_by_index(plist, i, j);
+          if (filep->f_inode == NULL)
             {
               continue;
             }
@@ -470,23 +469,21 @@ int files_duplist(FAR struct filelist *plist, FAR struct filelist *clist)
           ret = files_extend(clist, i + 1);
           if (ret < 0)
             {
-              goto out;
+              return ret;
             }
 
           /* Yes... duplicate it for the child, include O_CLOEXEC flag. */
 
-          ret = file_dup3(filep, &clist->fl_files[i][j],
+          ret = file_dup3(filep, files_fget_by_index(clist, i, j),
                           filep->f_oflags & O_CLOEXEC ? O_CLOEXEC : 0);
           if (ret < 0)
             {
-              goto out;
+              return ret;
             }
         }
     }
 
-out:
-  nxmutex_unlock(&plist->fl_lock);
-  return ret;
+  return OK;
 }
 
 /****************************************************************************
@@ -508,23 +505,20 @@ void files_close_onexec(FAR struct tcb_s *tcb)
   list = nxsched_get_files_from_tcb(tcb);
   DEBUGASSERT(list != NULL);
 
-  nxmutex_lock(&list->fl_lock);
   for (i = 0; i < list->fl_rows; i++)
     {
       for (j = 0; j < CONFIG_NFILE_DESCRIPTORS_PER_BLOCK; j++)
         {
           FAR struct file *filep;
 
-          filep = &list->fl_files[i][j];
-          if (filep && filep->f_inode != NULL &&
+          filep = files_fget_by_index(list, i, j);
+          if (filep->f_inode != NULL &&
               (filep->f_oflags & O_CLOEXEC) != 0)
             {
               file_close(filep);
             }
         }
     }
-
-  nxmutex_unlock(&list->fl_lock);
 }
 
 /****************************************************************************
@@ -547,8 +541,6 @@ void files_close_onexec(FAR struct tcb_s *tcb)
 int fs_getfilep(int fd, FAR struct file **filep)
 {
   FAR struct filelist *list;
-  irqstate_t flags;
-  int ret;
 
 #ifdef CONFIG_FDCHECK
   fd = fdcheck_restore(fd);
@@ -575,54 +567,21 @@ int fs_getfilep(int fd, FAR struct file **filep)
       return -EBADF;
     }
 
-  /* Protect this part with a critical section to make sure that we won't
-   * interrupt the mutex lock-unclock sequence below which may lead to the
-   * priority inversion. The case is as follows:
-   *
-   *   We have two threads: low-priority thread A and high-priority thread B,
-   *   both threads share the same task group data.
-   *
-   *   Thread A performs IO on files periodically. Thread B is woken up by a
-   *   high-frequency interrupts, and performs IO on files periodically.
-   *
-   *   There is a chance that thread B wakes up exactly when thread A holds
-   *   the mutex below, and consequently the file access in thread B will be
-   *   delayed due to thread A holding the list->fl_lock mutex and execution
-   *   will be returned to a thread with lower priority.
-   *
-   * The correct solution to this problem is to use the read-write lock,
-   * which is currently not supported by NuttX.
-   */
-
-  flags = enter_critical_section();
-
   /* The descriptor is in a valid range to file descriptor... Get the
    * thread-specific file list.
    */
 
-  /* And return the file pointer from the list */
-
-  ret = nxmutex_lock(&list->fl_lock);
-  if (ret < 0)
-    {
-      leave_critical_section(flags);
-      return ret;
-    }
-
-  *filep = &list->fl_files[fd / CONFIG_NFILE_DESCRIPTORS_PER_BLOCK]
-                          [fd % CONFIG_NFILE_DESCRIPTORS_PER_BLOCK];
+  *filep = files_fget(list, fd);
 
   /* if f_inode is NULL, fd was closed */
 
-  if (!(*filep)->f_inode)
+  if ((*filep)->f_inode == NULL)
     {
       *filep = NULL;
-      ret = -EBADF;
+      return -EBADF;
     }
 
-  nxmutex_unlock(&list->fl_lock);
-  leave_critical_section(flags);
-  return ret;
+  return OK;
 }
 
 /****************************************************************************
@@ -746,7 +705,6 @@ int nx_close_from_tcb(FAR struct tcb_s *tcb, int fd)
   FAR struct file     *filep;
   FAR struct file      file;
   FAR struct filelist *list;
-  int                  ret;
 
 #ifdef CONFIG_FDCHECK
   fd = fdcheck_restore(fd);
@@ -756,28 +714,23 @@ int nx_close_from_tcb(FAR struct tcb_s *tcb, int fd)
 
   /* Perform the protected close operation */
 
-  ret = nxmutex_lock(&list->fl_lock);
-  if (ret < 0)
+  if (fd < 0 || fd >= list->fl_rows * CONFIG_NFILE_DESCRIPTORS_PER_BLOCK)
     {
-      return ret;
+      return -EBADF;
     }
 
+  filep = files_fget(list, fd);
+
   /* If the file was properly opened, there should be an inode assigned */
 
-  if (fd < 0 || fd >= list->fl_rows * CONFIG_NFILE_DESCRIPTORS_PER_BLOCK ||
-      !list->fl_files[fd / CONFIG_NFILE_DESCRIPTORS_PER_BLOCK]
-                     [fd % CONFIG_NFILE_DESCRIPTORS_PER_BLOCK].f_inode)
+  if (filep->f_inode == NULL)
     {
-      nxmutex_unlock(&list->fl_lock);
       return -EBADF;
     }
 
-  filep = &list->fl_files[fd / CONFIG_NFILE_DESCRIPTORS_PER_BLOCK]
-                         [fd % CONFIG_NFILE_DESCRIPTORS_PER_BLOCK];
   memcpy(&file, filep, sizeof(struct file));
   memset(filep, 0,     sizeof(struct file));
 
-  nxmutex_unlock(&list->fl_lock);
   return file_close(&file);
 }
 
diff --git a/include/nuttx/fs/fs.h b/include/nuttx/fs/fs.h
index be7c122be2..f944c22d99 100644
--- a/include/nuttx/fs/fs.h
+++ b/include/nuttx/fs/fs.h
@@ -37,6 +37,7 @@
 
 #include <nuttx/mutex.h>
 #include <nuttx/semaphore.h>
+#include <nuttx/spinlock.h>
 #include <nuttx/mm/map.h>
 
 /****************************************************************************
@@ -478,7 +479,7 @@ struct file
 
 struct filelist
 {
-  mutex_t           fl_lock;    /* Manage access to the file list */
+  spinlock_t        fl_lock;    /* Manage access to the file list */
   uint8_t           fl_rows;    /* The number of rows of fl_files array */
   FAR struct file **fl_files;   /* The pointer of two layer file descriptors array */
 };


(nuttx) 03/03: spinlock: add __cplusplus check to avoid build break with C++

Posted by xi...@apache.org.
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 74d698f439e91ea7b869a089bb019ee1c3df26b4
Author: chao an <an...@xiaomi.com>
AuthorDate: Sun Nov 12 21:50:20 2023 +0800

    spinlock: add __cplusplus check to avoid build break with C++
    
    stdatomic.h is incompatible with C++
    
    Signed-off-by: chao an <an...@xiaomi.com>
---
 include/nuttx/spinlock.h | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/include/nuttx/spinlock.h b/include/nuttx/spinlock.h
index 63c82c2e8d..e0663afda0 100644
--- a/include/nuttx/spinlock.h
+++ b/include/nuttx/spinlock.h
@@ -32,12 +32,21 @@
 
 #include <nuttx/irq.h>
 
-#ifdef CONFIG_RW_SPINLOCK
-#include <stdatomic.h>
+#undef EXTERN
+#if defined(__cplusplus)
+#define EXTERN extern "C"
+extern "C"
+{
+#else
+#define EXTERN extern
+#endif
+
+#if defined(CONFIG_RW_SPINLOCK) && !defined(__cplusplus)
+#  include <stdatomic.h>
 typedef atomic_int rwlock_t;
-#define RW_SP_UNLOCKED      0
-#define RW_SP_READ_LOCKED   1
-#define RW_SP_WRITE_LOCKED -1
+#  define RW_SP_UNLOCKED      0
+#  define RW_SP_READ_LOCKED   1
+#  define RW_SP_WRITE_LOCKED -1
 #endif
 
 #ifndef CONFIG_SPINLOCK
@@ -500,7 +509,7 @@ void spin_unlock_irqrestore_wo_note(FAR spinlock_t *lock, irqstate_t flags);
 #  define spin_unlock_irqrestore_wo_note(l, f) up_irq_restore(f)
 #endif
 
-#ifdef CONFIG_RW_SPINLOCK
+#if defined(CONFIG_RW_SPINLOCK) && !defined(__cplusplus)
 
 /****************************************************************************
  * Name: rwlock_init
@@ -808,5 +817,11 @@ void write_unlock_irqrestore(FAR rwlock_t *lock, irqstate_t flags);
 #  define write_unlock_irqrestore(l, f) up_irq_restore(f)
 #endif
 
-#endif /* CONFIG_RW_SPINLOCK */
+#endif /* CONFIG_RW_SPINLOCK && !__cplusplus */
+
+#undef EXTERN
+#if defined(__cplusplus)
+}
+#endif
+
 #endif /* __INCLUDE_NUTTX_SPINLOCK_H */