From 3ea7a6fb77c4a348ae166bee31f7d606e56c7fd0 Mon Sep 17 00:00:00 2001 From: Jukka Laitinen Date: Tue, 10 Jan 2023 17:03:04 +0400 Subject: [PATCH] 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 --- 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; - if (mm_map_remove(get_group_mm(group), entry)) + /* Remove the entry from the process' mappings */ + + ret = mm_map_remove(get_group_mm(group), entry); + if (ret < 0) { - shmerr("ERROR: mm_map_remove() failed\n"); + return ret; } - return shmdt_priv(group, shmaddr, shmid); + /* 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(®ion->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); + + /* 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) + { + 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(®ion->sr_lock); + + 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(®ion->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(®ion->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;