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 <juk...@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 <juk...@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 */
