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/01/10 10:34:34 UTC

[nuttx] 03/06: mm/shm: Remove gs_vaddr from task group and use dynamic vm_map instead

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 51509794888584d89432370da281753cfad8af09
Author: Jukka Laitinen <ju...@ssrc.tii.ae>
AuthorDate: Tue Apr 12 16:34:50 2022 +0400

    mm/shm: Remove gs_vaddr from task group and use dynamic vm_map instead
    
    Replace static gs_vaddr with a new dynamic mapping list. Collecting all
    this kind of virtual memory mappings into a single structure makes
    things more consistent.
    
    This still leaves the task group specific granule alloocator, gs_handle,
    in the task group
    
    Signed-off-by: Jukka Laitinen <ju...@ssrc.tii.ae>
---
 include/nuttx/mm/shm.h |  25 ++++++++----
 mm/shm/shmat.c         |  40 ++++++++++++++++--
 mm/shm/shmdt.c         | 109 +++++++++++++++++++++++++++++++++----------------
 3 files changed, 127 insertions(+), 47 deletions(-)

diff --git a/include/nuttx/mm/shm.h b/include/nuttx/mm/shm.h
index eeb9d14e55..b89db5bbd3 100644
--- a/include/nuttx/mm/shm.h
+++ b/include/nuttx/mm/shm.h
@@ -84,13 +84,6 @@ struct group_shm_s
    */
 
   GRAN_HANDLE gs_handle;
-
-  /* This array is used to do a reverse lookup:  Give the virtual address
-   * of a shared memory region, find the region index that performs that
-   * mapping.
-   */
-
-  uintptr_t gs_vaddr[CONFIG_ARCH_SHM_MAXREGIONS];
 };
 
 /****************************************************************************
@@ -171,5 +164,23 @@ 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 590f68cdd3..9642729cc5 100644
--- a/mm/shm/shmat.c
+++ b/mm/shm/shmat.c
@@ -32,11 +32,32 @@
 #include <nuttx/sched.h>
 #include <nuttx/arch.h>
 #include <nuttx/pgalloc.h>
+#include <nuttx/mm/map.h>
 
 #include "shm/shm.h"
 
 #ifdef CONFIG_MM_SHM
 
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+static int munmap_shm(FAR struct task_group_s *group,
+                      FAR struct mm_map_entry_s *entry,
+                      FAR void *start,
+                      size_t length)
+{
+  FAR const void *shmaddr = entry->vaddr;
+  int shmid = entry->priv.i;
+
+  if (mm_map_remove(get_group_mm(group), entry))
+    {
+      shmerr("ERROR: mm_map_remove() failed\n");
+    }
+
+  return shmdt_priv(group, shmaddr, shmid);
+}
+
 /****************************************************************************
  * Public Functions
  ****************************************************************************/
@@ -103,6 +124,7 @@ FAR void *shmat(int shmid, FAR const void *shmaddr, int shmflg)
   FAR void *vaddr;
   unsigned int npages;
   int ret;
+  struct mm_map_entry_s entry;
 
   /* Get the region associated with the shmid */
 
@@ -114,9 +136,8 @@ FAR void *shmat(int shmid, FAR const void *shmaddr, int shmflg)
 
   tcb = nxsched_self();
   DEBUGASSERT(tcb && tcb->group);
+
   group = tcb->group;
-  DEBUGASSERT(group->tg_shm.gs_handle != NULL &&
-              group->tg_shm.gs_vaddr[shmid] == 0);
 
   /* Get exclusive access to the region data structure */
 
@@ -150,12 +171,23 @@ FAR void *shmat(int shmid, FAR const void *shmaddr, int shmflg)
       goto errout_with_vaddr;
     }
 
-  /* Save the virtual address of the region.  We will need that in shmat()
+  /* Save the virtual address of the region.  We will need that in shmdt()
    * to do the reverse lookup:  Give the virtual address of the region to
    * detach, we need to get the region table index.
    */
 
-  group->tg_shm.gs_vaddr[shmid] = (uintptr_t)vaddr;
+  entry.vaddr = vaddr;
+  entry.length = region->sr_ds.shm_segsz;
+  entry.offset = 0;
+  entry.munmap = munmap_shm;
+  entry.priv.i = shmid;
+
+  ret = mm_map_add(&entry);
+  if (ret < 0)
+    {
+      shmerr("ERROR: mm_map_add() failed\n");
+      goto errout_with_vaddr;
+    }
 
   /* Increment the count of processes attached to this region */
 
diff --git a/mm/shm/shmdt.c b/mm/shm/shmdt.c
index 1f24f2e55f..7878b70eec 100644
--- a/mm/shm/shmdt.c
+++ b/mm/shm/shmdt.c
@@ -33,6 +33,7 @@
 #include <nuttx/sched.h>
 #include <nuttx/mm/shm.h>
 #include <nuttx/pgalloc.h>
+#include <nuttx/mm/map.h>
 
 #include "shm/shm.h"
 
@@ -43,22 +44,23 @@
  ****************************************************************************/
 
 /****************************************************************************
- * Name: shmdt
+ * Name: shmdt_priv
  *
  * Description:
- *   The shmdt() function detaches the shared memory segment located at the
- *   address specified by shmaddr from the address space of the calling
+ *   The shmdt_priv() 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() will decrement the value of
+ *   Upon successful completion, shmdt_priv() 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()
+ *   Otherwise, the shared memory segment will not be detached, shmdt_priv()
  *   will return -1, and errno will be set to indicate the error.
  *
  *   - EINVAL
@@ -67,38 +69,14 @@
  *
  ****************************************************************************/
 
-int shmdt(FAR const void *shmaddr)
+int shmdt_priv(FAR struct task_group_s *group, FAR const void *shmaddr,
+               int shmid)
 {
   FAR struct shm_region_s *region;
-  FAR struct task_group_s *group;
-  FAR struct tcb_s *tcb;
+  pid_t pid;
   unsigned int npages;
-  int shmid;
   int ret;
 
-  /* Get the TCB and group containing our virtual memory allocator */
-
-  tcb = nxsched_self();
-  DEBUGASSERT(tcb && tcb->group);
-  group = tcb->group;
-
-  /* Perform the reverse lookup to get the shmid corresponding to this
-   * shmaddr.
-   */
-
-  for (shmid = 0;
-       shmid < CONFIG_ARCH_SHM_MAXREGIONS &&
-       group->tg_shm.gs_vaddr[shmid] != (uintptr_t)shmaddr;
-       shmid++);
-
-  if (shmid >= CONFIG_ARCH_SHM_MAXREGIONS)
-    {
-      shmerr("ERROR: No region matching this virtual address: %p\n",
-             shmaddr);
-      ret = -EINVAL;
-      goto errout_with_errno;
-    }
-
   /* Get the region associated with the shmid */
 
   region =  &g_shminfo.si_region[shmid];
@@ -113,6 +91,16 @@ int shmdt(FAR const void *shmaddr)
       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);
@@ -131,9 +119,7 @@ int shmdt(FAR const void *shmaddr)
       shmerr("ERROR: up_shmdt() failed\n");
     }
 
-  /* Indicate that there is no longer any mapping for this region. */
-
-  group->tg_shm.gs_vaddr[shmid] = 0;
+on_process_exit:
 
   /* Decrement the count of processes attached to this region.
    * If the count decrements to zero and there is a pending unlink,
@@ -162,7 +148,7 @@ int shmdt(FAR const void *shmaddr)
 
   /* Save the process ID of the last operation */
 
-  region->sr_ds.shm_lpid = tcb->pid;
+  region->sr_ds.shm_lpid = pid;
 
   /* Save the time of the last shmdt() */
 
@@ -178,4 +164,55 @@ errout_with_errno:
   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 */
+
+  tcb = nxsched_self();
+  DEBUGASSERT(tcb && tcb->group);
+  group = tcb->group;
+
+  /* Get exclusive access to process' mm_map */
+
+  ret = mm_map_lock();
+  if (ret == OK)
+    {
+      /* Perform the reverse lookup to get the shmid corresponding to this
+       * shmaddr. The mapping is matched with just shmaddr == map->vaddr.
+       */
+
+      entry = mm_map_find(shmaddr, 1);
+      if (!entry || entry->vaddr != shmaddr)
+        {
+          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");
+        }
+
+      mm_map_unlock();
+
+      ret = shmdt_priv(tcb->group, shmaddr, shmid);
+    }
+
+  return ret;
+}
+
 #endif /* CONFIG_MM_SHM */