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 2022/11/28 10:04:44 UTC

[nuttx] branch master updated (578f7783c6 -> f243e4f268)

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 578f7783c6 Corrects PIO errors and omissions  for SAMA5D2
     new 11fa70d53a romfs: change lock to recursion lock
     new f243e4f268 localtime: fix deadlock when print syslog in romfs

The 2 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/romfs/fs_romfs.c            | 55 +++++++++++++++++++++---------------------
 fs/romfs/fs_romfs.h            |  2 +-
 libs/libc/time/lib_localtime.c | 31 ++++++++++++------------
 3 files changed, 45 insertions(+), 43 deletions(-)


[nuttx] 02/02: localtime: fix deadlock when print syslog in romfs

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 f243e4f268d6370b4d4aad5415310c1b0399b9a5
Author: ligd <li...@xiaomi.com>
AuthorDate: Tue Nov 15 18:35:32 2022 +0800

    localtime: fix deadlock when print syslog in romfs
    
    thread1:
    romfs_seek -> take romfslock -> syslog -> tzset
    thread2:
    sylsog -> tzset -> tz_load -> romfs_open -> deadlock
    
    Signed-off-by: ligd <li...@xiaomi.com>
---
 libs/libc/time/lib_localtime.c | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/libs/libc/time/lib_localtime.c b/libs/libc/time/lib_localtime.c
index 528dc86688..93985a4002 100644
--- a/libs/libc/time/lib_localtime.c
+++ b/libs/libc/time/lib_localtime.c
@@ -1901,6 +1901,11 @@ static FAR struct tm *localsub(FAR const time_t *timep,
       return gmtsub(timep, offset, tmp);
     }
 
+  if (nxrmutex_is_hold(&g_lcl_lock))
+    {
+      return NULL;
+    }
+
   if ((sp->goback && t < sp->ats[0]) ||
       (sp->goahead && t > sp->ats[sp->timecnt - 1]))
     {
@@ -2746,7 +2751,6 @@ static int zoneinit(FAR const char *name)
 void tzset(void)
 {
   FAR const char *name;
-  int lcl = -1;
 
 #ifndef __KERNEL__
   if (up_interrupt_context() || (sched_idletask() && OSINIT_IDLELOOP()))
@@ -2755,23 +2759,24 @@ void tzset(void)
     }
 #endif
 
-  nxrmutex_lock(&g_lcl_lock);
-
   name = getenv("TZ");
-  if (name != NULL)
+  if (name == NULL)
     {
-      lcl = 1;
+      return;
     }
 
-  if (lcl < 0 && g_lcl_isset < 0)
+  if (g_lcl_isset > 0 && strcmp(g_lcl_tzname, name) == 0)
     {
-      goto out;
+      return;
     }
-  else if (lcl > 0 && g_lcl_isset > 0 && strcmp(g_lcl_tzname, name) == 0)
+
+  if (nxrmutex_is_hold(&g_lcl_lock))
     {
-      goto out;
+      return;
     }
 
+  nxrmutex_lock(&g_lcl_lock);
+
   if (g_lcl_ptr == NULL)
     {
       g_lcl_ptr = lib_malloc(sizeof(*g_lcl_ptr));
@@ -2786,15 +2791,11 @@ void tzset(void)
       zoneinit("");
     }
 
-  if (lcl > 0)
-    {
-      strcpy(g_lcl_tzname, name);
-    }
+  strcpy(g_lcl_tzname, name);
 
 tzname:
   settzname();
-  g_lcl_isset = lcl;
-out:
+  g_lcl_isset = 1;
   nxrmutex_unlock(&g_lcl_lock);
 }
 


[nuttx] 01/02: romfs: change lock to recursion lock

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 11fa70d53a1c8bf8dfb588b004b06802a579a1b3
Author: ligd <li...@xiaomi.com>
AuthorDate: Tue Nov 15 18:32:51 2022 +0800

    romfs: change lock to recursion lock
    
    Signed-off-by: ligd <li...@xiaomi.com>
---
 fs/romfs/fs_romfs.c | 55 +++++++++++++++++++++++++++--------------------------
 fs/romfs/fs_romfs.h |  2 +-
 2 files changed, 29 insertions(+), 28 deletions(-)

diff --git a/fs/romfs/fs_romfs.c b/fs/romfs/fs_romfs.c
index f632464966..ad439a91e1 100644
--- a/fs/romfs/fs_romfs.c
+++ b/fs/romfs/fs_romfs.c
@@ -175,7 +175,7 @@ static int romfs_open(FAR struct file *filep, FAR const char *relpath,
 
   /* Check if the mount is still healthy */
 
-  ret = nxmutex_lock(&rm->rm_lock);
+  ret = nxrmutex_lock(&rm->rm_lock);
   if (ret < 0)
     {
       return ret;
@@ -287,7 +287,7 @@ static int romfs_open(FAR struct file *filep, FAR const char *relpath,
   rm->rm_refs++;
 
 errout_with_lock:
-  nxmutex_unlock(&rm->rm_lock);
+  nxrmutex_unlock(&rm->rm_lock);
   return ret;
 }
 
@@ -314,14 +314,14 @@ static int romfs_close(FAR struct file *filep)
 
   DEBUGASSERT(rm != NULL);
 
-  ret = nxmutex_lock(&rm->rm_lock);
+  ret = nxrmutex_lock(&rm->rm_lock);
   if (ret < 0)
     {
       return ret;
     }
 
   rm->rm_refs--;
-  nxmutex_unlock(&rm->rm_lock);
+  nxrmutex_unlock(&rm->rm_lock);
 
   /* Do not check if the mount is healthy.  We must support closing of
    * the file even when there is healthy mount.
@@ -380,7 +380,7 @@ static ssize_t romfs_read(FAR struct file *filep, FAR char *buffer,
 
   /* Make sure that the mount is still healthy */
 
-  ret = nxmutex_lock(&rm->rm_lock);
+  ret = nxrmutex_lock(&rm->rm_lock);
   if (ret < 0)
     {
       return (ssize_t)ret;
@@ -484,7 +484,7 @@ static ssize_t romfs_read(FAR struct file *filep, FAR char *buffer,
     }
 
 errout_with_lock:
-  nxmutex_unlock(&rm->rm_lock);
+  nxrmutex_unlock(&rm->rm_lock);
   return ret < 0 ? ret : readsize;
 }
 
@@ -539,7 +539,7 @@ static off_t romfs_seek(FAR struct file *filep, off_t offset, int whence)
 
   /* Make sure that the mount is still healthy */
 
-  ret = nxmutex_lock(&rm->rm_lock);
+  ret = nxrmutex_lock(&rm->rm_lock);
   if (ret < 0)
     {
       return (off_t)ret;
@@ -567,7 +567,7 @@ static off_t romfs_seek(FAR struct file *filep, off_t offset, int whence)
   finfo("New file position: %jd\n", (intmax_t)filep->f_pos);
 
 errout_with_lock:
-  nxmutex_unlock(&rm->rm_lock);
+  nxrmutex_unlock(&rm->rm_lock);
   return ret;
 }
 
@@ -645,7 +645,7 @@ static int romfs_dup(FAR const struct file *oldp, FAR struct file *newp)
 
   /* Check if the mount is still healthy */
 
-  ret = nxmutex_lock(&rm->rm_lock);
+  ret = nxrmutex_lock(&rm->rm_lock);
   if (ret < 0)
     {
       return ret;
@@ -696,7 +696,7 @@ static int romfs_dup(FAR const struct file *oldp, FAR struct file *newp)
   rm->rm_refs++;
 
 errout_with_lock:
-  nxmutex_unlock(&rm->rm_lock);
+  nxrmutex_unlock(&rm->rm_lock);
   return ret;
 }
 
@@ -731,7 +731,7 @@ static int romfs_fstat(FAR const struct file *filep, FAR struct stat *buf)
 
   /* Check if the mount is still healthy */
 
-  ret = nxmutex_lock(&rm->rm_lock);
+  ret = nxrmutex_lock(&rm->rm_lock);
   if (ret < 0)
     {
       return ret;
@@ -746,7 +746,7 @@ static int romfs_fstat(FAR const struct file *filep, FAR struct stat *buf)
                               rm->rm_hwsectorsize, buf);
     }
 
-  nxmutex_unlock(&rm->rm_lock);
+  nxrmutex_unlock(&rm->rm_lock);
   return ret;
 }
 
@@ -784,7 +784,7 @@ static int romfs_opendir(FAR struct inode *mountpt, FAR const char *relpath,
 
   /* Make sure that the mount is still healthy */
 
-  ret = nxmutex_lock(&rm->rm_lock);
+  ret = nxrmutex_lock(&rm->rm_lock);
   if (ret < 0)
     {
       goto errout_with_rdir;
@@ -828,11 +828,11 @@ static int romfs_opendir(FAR struct inode *mountpt, FAR const char *relpath,
 #endif
 
   *dir = &rdir->base;
-  nxmutex_unlock(&rm->rm_lock);
+  nxrmutex_unlock(&rm->rm_lock);
   return OK;
 
 errout_with_lock:
-  nxmutex_unlock(&rm->rm_lock);
+  nxrmutex_unlock(&rm->rm_lock);
 
 errout_with_rdir:
   kmm_free(rdir);
@@ -888,7 +888,7 @@ static int romfs_readdir(FAR struct inode *mountpt,
 
   /* Make sure that the mount is still healthy */
 
-  ret = nxmutex_lock(&rm->rm_lock);
+  ret = nxrmutex_lock(&rm->rm_lock);
   if (ret < 0)
     {
       return ret;
@@ -973,7 +973,7 @@ static int romfs_readdir(FAR struct inode *mountpt,
     }
 
 errout_with_lock:
-  nxmutex_unlock(&rm->rm_lock);
+  nxrmutex_unlock(&rm->rm_lock);
   return ret;
 }
 
@@ -1004,7 +1004,7 @@ static int romfs_rewinddir(FAR struct inode *mountpt,
 
   /* Make sure that the mount is still healthy */
 
-  ret = nxmutex_lock(&rm->rm_lock);
+  ret = nxrmutex_lock(&rm->rm_lock);
   if (ret < 0)
     {
       return ret;
@@ -1020,7 +1020,7 @@ static int romfs_rewinddir(FAR struct inode *mountpt,
 #endif
     }
 
-  nxmutex_unlock(&rm->rm_lock);
+  nxrmutex_unlock(&rm->rm_lock);
   return ret;
 }
 
@@ -1073,7 +1073,7 @@ static int romfs_bind(FAR struct inode *blkdriver, FAR const void *data,
    * have to addref() here (but does have to release in ubind().
    */
 
-  nxmutex_init(&rm->rm_lock);   /* Initialize the mutex that controls access */
+  nxrmutex_init(&rm->rm_lock);  /* Initialize the mutex that controls access */
   rm->rm_blkdriver = blkdriver; /* Save the block driver reference */
 
   /* Get the hardware configuration and setup buffering appropriately */
@@ -1108,6 +1108,7 @@ errout_with_buffer:
     }
 
 errout:
+  nxrmutex_destroy(&rm->rm_lock);
   kmm_free(rm);
   return ret;
 }
@@ -1137,7 +1138,7 @@ static int romfs_unbind(FAR void *handle, FAR struct inode **blkdriver,
 
   /* Check if there are sill any files opened on the filesystem. */
 
-  ret = nxmutex_lock(&rm->rm_lock);
+  ret = nxrmutex_lock(&rm->rm_lock);
   if (ret < 0)
     {
       return ret;
@@ -1192,12 +1193,12 @@ static int romfs_unbind(FAR void *handle, FAR struct inode **blkdriver,
 #ifdef CONFIG_FS_ROMFS_CACHE_NODE
       romfs_freenode(rm->rm_root);
 #endif
-      nxmutex_destroy(&rm->rm_lock);
+      nxrmutex_destroy(&rm->rm_lock);
       kmm_free(rm);
       return OK;
     }
 
-  nxmutex_unlock(&rm->rm_lock);
+  nxrmutex_unlock(&rm->rm_lock);
   return ret;
 }
 
@@ -1225,7 +1226,7 @@ static int romfs_statfs(FAR struct inode *mountpt, FAR struct statfs *buf)
 
   /* Check if the mount is still healthy */
 
-  ret = nxmutex_lock(&rm->rm_lock);
+  ret = nxrmutex_lock(&rm->rm_lock);
   if (ret < 0)
     {
       return ret;
@@ -1255,7 +1256,7 @@ static int romfs_statfs(FAR struct inode *mountpt, FAR struct statfs *buf)
   buf->f_namelen = NAME_MAX;
 
 errout_with_lock:
-  nxmutex_unlock(&rm->rm_lock);
+  nxrmutex_unlock(&rm->rm_lock);
   return ret;
 }
 
@@ -1342,7 +1343,7 @@ static int romfs_stat(FAR struct inode *mountpt, FAR const char *relpath,
 
   /* Check if the mount is still healthy */
 
-  ret = nxmutex_lock(&rm->rm_lock);
+  ret = nxrmutex_lock(&rm->rm_lock);
   if (ret < 0)
     {
       return ret;
@@ -1373,7 +1374,7 @@ static int romfs_stat(FAR struct inode *mountpt, FAR const char *relpath,
   ret  = romfs_stat_common(type, nodeinfo.rn_size, rm->rm_hwsectorsize, buf);
 
 errout_with_lock:
-  nxmutex_unlock(&rm->rm_lock);
+  nxrmutex_unlock(&rm->rm_lock);
   return ret;
 }
 
diff --git a/fs/romfs/fs_romfs.h b/fs/romfs/fs_romfs.h
index 2db26b5884..20eabc3bc2 100644
--- a/fs/romfs/fs_romfs.h
+++ b/fs/romfs/fs_romfs.h
@@ -132,7 +132,7 @@ struct romfs_mountpt_s
 #endif
   bool     rm_mounted;            /* true: The file system is ready */
   uint16_t rm_hwsectorsize;       /* HW: Sector size reported by block driver */
-  mutex_t  rm_lock;               /* Used to assume thread-safe access */
+  rmutex_t rm_lock;               /* Used to assume thread-safe access */
   uint32_t rm_refs;               /* The references for all files opened on this mountpoint */
   uint32_t rm_hwnsectors;         /* HW: The number of sectors reported by the hardware */
   uint32_t rm_volsize;            /* Size of the ROMFS volume */