[nuttx] branch master updated: mm/shm: Clean up the System-V shm driver

[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


The following commit(s) were added to refs/heads/master by this push:
     new 3ea7a6fb77 mm/shm: Clean up the System-V shm driver
3ea7a6fb77 is described below

commit 3ea7a6fb77c4a348ae166bee31f7d606e56c7fd0
Author: Jukka Laitinen <ju...@ssrc.tii.ae>
AuthorDate: Tue Jan 10 17:03:04 2023 +0400

    mm/shm: Clean up the System-V shm driver
    
    Move shmdt functionality into shm_unmap, and use shm_unmap as the common
    detach function.
    
    Signed-off-by: Jukka Laitinen <ju...@ssrc.tii.ae>
---
 include/nuttx/mm/shm.h |  18 -------
 mm/shm/shmat.c         |  93 ++++++++++++++++++++++++++++++++--
 mm/shm/shmdt.c         | 135 ++++++-------------------------------------------
 3 files changed, 106 insertions(+), 140 deletions(-)

diff --git a/include/nuttx/mm/shm.h b/include/nuttx/mm/shm.h
index b89db5bbd3..eaa181a9da 100644
--- a/include/nuttx/mm/shm.h
+++ b/include/nuttx/mm/shm.h
@@ -164,23 +164,5 @@ FAR void *shm_alloc(FAR struct task_group_s *group, FAR void *vaddr,
 
 void shm_free(FAR struct task_group_s *group, FAR void *vaddr, size_t size);
 
-/****************************************************************************
- * Name: shmdt_priv
- *
- * Description:
- *   This is the shmdt internal implementation of the shm driver. It takes
- *   the task group struct as a parameter and can handle both normal detach
- *   and cleanup during process exit.
- *
- * Input Parameters:
- *   group   - A reference to the group structure from which to detach
- *   shmaddr - Virtual start address where the allocation starts.
- *   shmid   - Id of the allocation
- *
- ****************************************************************************/
-
-int shmdt_priv(FAR struct task_group_s *group, FAR const void *shmaddr,
-               int shmid);
-
 #endif /* CONFIG_MM_SHM */
 #endif /* __INCLUDE_NUTTX_MM_SHM_H */
diff --git a/mm/shm/shmat.c b/mm/shm/shmat.c
index 9642729cc5..b4215f584e 100644
--- a/mm/shm/shmat.c
+++ b/mm/shm/shmat.c
@@ -49,13 +49,100 @@ static int munmap_shm(FAR struct task_group_s *group,
 {
   FAR const void *shmaddr = entry->vaddr;
   int shmid = entry->priv.i;
+  FAR struct shm_region_s *region;
+  pid_t pid;
+  unsigned int npages;
+  int ret;
+
+  /* Remove the entry from the process' mappings */
+
+  ret = mm_map_remove(get_group_mm(group), entry);
+  if (ret < 0)
+    {
+      return ret;
+    }
+
+  /* Get the region associated with the shmid */
+
+  region =  &g_shminfo.si_region[shmid];
+  DEBUGASSERT((region->sr_flags & SRFLAG_INUSE) != 0);
+
+  /* Get exclusive access to the region data structure */
+
+  ret = nxmutex_lock(&region->sr_lock);
+  if (ret < 0)
+    {
+      shmerr("ERROR: nxsem_wait failed: %d\n", ret);
+      return ret;
+    }
+
+  if (group)
+    {
+      /* Free the virtual address space */
+
+      shm_free(group, (FAR void *)shmaddr, region->sr_ds.shm_segsz);
 
-  if (mm_map_remove(get_group_mm(group), entry))
+      /* Convert the region size to pages */
+
+      npages = MM_NPAGES(region->sr_ds.shm_segsz);
+
+      /* Detach, i.e, unmap, on shared memory region from a user virtual
+       * address.
+       */
+
+      ret = up_shmdt((uintptr_t)shmaddr, npages);
+
+      /* Get pid of this process */
+
+      pid = group->tg_pid;
+    }
+  else
+    {
+      /* We are in the middle of process destruction and don't know the
+       * context
+       */
+
+      pid = 0;
+    }
+
+  /* Decrement the count of processes attached to this region.
+   * If the count decrements to zero and there is a pending unlink,
+   * then destroy the shared memory region now and stop any further
+   * operations on it.
+   */
+
+  DEBUGASSERT(region->sr_ds.shm_nattch > 0);
+  if (region->sr_ds.shm_nattch <= 1)
     {
-      shmerr("ERROR: mm_map_remove() failed\n");
+      region->sr_ds.shm_nattch = 0;
+      if ((region->sr_flags & SRFLAG_UNLINKED) != 0)
+        {
+          shm_destroy(shmid);
+          return OK;
+        }
     }
+  else
+    {
+      /* Just decrement the number of processes attached to the shared
+       * memory region.
+       */
+
+      region->sr_ds.shm_nattch--;
+    }
+
+  /* Save the process ID of the last operation */
+
+  region->sr_ds.shm_lpid = pid;
+
+  /* Save the time of the last shmdt() */
+
+  region->sr_ds.shm_dtime = time(NULL);
+
+  /* Release our lock on the entry */
+
+  nxmutex_unlock(&region->sr_lock);
 
-  return shmdt_priv(group, shmaddr, shmid);
+  return ret;
 }
 
 /****************************************************************************
diff --git a/mm/shm/shmdt.c b/mm/shm/shmdt.c
index 7878b70eec..d78b1c0887 100644
--- a/mm/shm/shmdt.c
+++ b/mm/shm/shmdt.c
@@ -44,132 +44,31 @@
  ****************************************************************************/
 
 /****************************************************************************
- * Name: shmdt_priv
+ * Name: shmdt
  *
  * Description:
- *   The shmdt_priv() function detaches the shared memory segment located at
+ *   The shmdt() function detaches the shared memory segment located at
  *   the address specified by shmaddr from the address space of the calling
  *   process.
  *
  * Input Parameters:
  *   shmid - Shared memory identifier
- *   group - Current task_group, NULL during process exit
  *
  * Returned Value:
- *   Upon successful completion, shmdt_priv() will decrement the value of
+ *   Upon successful completion, shmdt() will decrement the value of
  *   shm_nattch in the data structure associated with the shared memory ID
  *   of the attached shared memory segment and return 0.
  *
- *   Otherwise, the shared memory segment will not be detached, shmdt_priv()
+ *   Otherwise, the shared memory segment will not be detached, shmdt()
  *   will return -1, and errno will be set to indicate the error.
  *
- *   - EINVAL
- *     The value of shmaddr is not the data segment start address of a
- *     shared memory segment.
- *
  ****************************************************************************/
 
-int shmdt_priv(FAR struct task_group_s *group, FAR const void *shmaddr,
-               int shmid)
-{
-  FAR struct shm_region_s *region;
-  pid_t pid;
-  unsigned int npages;
-  int ret;
-
-  /* Get the region associated with the shmid */
-
-  region =  &g_shminfo.si_region[shmid];
-  DEBUGASSERT((region->sr_flags & SRFLAG_INUSE) != 0);
-
-  /* Get exclusive access to the region data structure */
-
-  ret = nxmutex_lock(&region->sr_lock);
-  if (ret < 0)
-    {
-      shmerr("ERROR: nxsem_wait failed: %d\n", ret);
-      goto errout_with_errno;
-    }
-
-  if (!group)
-    {
-      pid = 0;
-      goto on_process_exit;
-    }
-  else
-    {
-      pid = group->tg_pid;
-    }
-
-  /* Free the virtual address space */
-
-  shm_free(group, (FAR void *)shmaddr, region->sr_ds.shm_segsz);
-
-  /* Convert the region size to pages */
-
-  npages = MM_NPAGES(region->sr_ds.shm_segsz);
-
-  /* Detach, i.e, unmap, on shared memory region from a user virtual
-   * address.
-   */
-
-  ret = up_shmdt((uintptr_t)shmaddr, npages);
-  if (ret < 0)
-    {
-      shmerr("ERROR: up_shmdt() failed\n");
-    }
-
-on_process_exit:
-
-  /* Decrement the count of processes attached to this region.
-   * If the count decrements to zero and there is a pending unlink,
-   * then destroy the shared memory region now and stop any further
-   * operations on it.
-   */
-
-  DEBUGASSERT(region->sr_ds.shm_nattch > 0);
-  if (region->sr_ds.shm_nattch <= 1)
-    {
-      region->sr_ds.shm_nattch = 0;
-      if ((region->sr_flags & SRFLAG_UNLINKED) != 0)
-        {
-          shm_destroy(shmid);
-          return OK;
-        }
-    }
-  else
-    {
-      /* Just decrement the number of processes attached to the shared
-       * memory region.
-       */
-
-      region->sr_ds.shm_nattch--;
-    }
-
-  /* Save the process ID of the last operation */
-
-  region->sr_ds.shm_lpid = pid;
-
-  /* Save the time of the last shmdt() */
-
-  region->sr_ds.shm_dtime = time(NULL);
-
-  /* Release our lock on the entry */
-
-  nxmutex_unlock(&region->sr_lock);
-  return OK;
-
-errout_with_errno:
-  set_errno(-ret);
-  return ERROR;
-}
-
 int shmdt(FAR const void *shmaddr)
 {
   FAR struct tcb_s *tcb;
   FAR struct mm_map_entry_s *entry;
   FAR struct task_group_s *group;
-  int shmid;
   int ret;
 
   /* Get the TCB and group containing our virtual memory allocator */
@@ -188,28 +87,26 @@ int shmdt(FAR const void *shmaddr)
        */
 
       entry = mm_map_find(shmaddr, 1);
-      if (!entry || entry->vaddr != shmaddr)
+      if (entry && entry->vaddr == shmaddr)
+        {
+          DEBUGASSERT(entry->munmap);
+          ret = entry->munmap(group, entry, entry->vaddr, entry->length);
+        }
+      else
         {
-          ret = -EINVAL;
           shmerr("ERROR: No region matching this virtual address: %p\n",
                  shmaddr);
 
-          mm_map_unlock();
-          return -EINVAL;
-        }
-
-      shmid = entry->priv.i;
-
-      /* Indicate that there is no longer any mapping for this region. */
-
-      if (mm_map_remove(get_group_mm(group), entry) < 0)
-        {
-          shmerr("ERROR: mm_map_remove() failed\n");
+          ret = -EINVAL;
         }
 
       mm_map_unlock();
+    }
 
-      ret = shmdt_priv(tcb->group, shmaddr, shmid);
+  if (ret < 0)
+    {
+      set_errno(-ret);
+      ret = -1;
     }
 
   return ret;