[PATCH 5/6] posix/mman: defer cleaning up mapped objects
Gedare Bloom
gedare at rtems.org
Fri Mar 31 20:47:03 UTC 2017
In case a file or shared memory object has been mmapped, then the
libc close() needs to defer recycling the file descriptor and related
rtems_libo_t structure until all mappings are unmapped. Similarly,
shm_close() needs to defer removing the backing shared memory object
until the mappings are unmapped. These deferrals are identified by
tracking the number of mapping references to open files and shm objects,
respectively, and checking if there are outstanding references before
cleaning up the related structures.
Updates #2859.
---
cpukit/libcsupport/include/rtems/libio.h | 1 +
cpukit/libcsupport/include/rtems/libio_.h | 41 +++++++++++++++++++++++++++
cpukit/libcsupport/src/libio.c | 32 +++++++++++++++++++--
cpukit/posix/include/rtems/posix/mmanimpl.h | 3 ++
cpukit/posix/include/rtems/posix/shmimpl.h | 30 ++++++++++++++++++++
cpukit/posix/src/mmap.c | 43 ++++++++++++++++++++++++++---
cpukit/posix/src/munmap.c | 29 +++++++++++--------
cpukit/posix/src/shmopen.c | 23 +--------------
cpukit/posix/src/shmunlink.c | 4 +--
9 files changed, 165 insertions(+), 41 deletions(-)
diff --git a/cpukit/libcsupport/include/rtems/libio.h b/cpukit/libcsupport/include/rtems/libio.h
index a87031c..d0824b4 100644
--- a/cpukit/libcsupport/include/rtems/libio.h
+++ b/cpukit/libcsupport/include/rtems/libio.h
@@ -1282,6 +1282,7 @@ struct rtems_libio_tt {
rtems_driver_name_t *driver;
off_t offset; /* current offset into file */
uint32_t flags;
+ uint32_t mapping_refcnt; /* current mappings */
rtems_filesystem_location_info_t pathinfo;
uint32_t data0; /* private to "driver" */
void *data1; /* ... */
diff --git a/cpukit/libcsupport/include/rtems/libio_.h b/cpukit/libcsupport/include/rtems/libio_.h
index c2fb975..695a4c4 100644
--- a/cpukit/libcsupport/include/rtems/libio_.h
+++ b/cpukit/libcsupport/include/rtems/libio_.h
@@ -304,6 +304,47 @@ void rtems_libio_free(
rtems_libio_t *iop
);
+/**
+ * Garbage collects the free libio in case it was previously freed but there
+ * were still references to it.
+ */
+void rtems_libio_check_deferred_free( rtems_libio_t *iop );
+
+/**
+ * Increment the reference count tracking number of mmap mappings of a file.
+ * Returns the updated reference count value.
+ */
+static inline uint32_t rtems_libio_increment_mapping_refcnt(rtems_libio_t *iop)
+{
+ uint32_t refcnt;
+ rtems_libio_lock();
+ refcnt = ++iop->mapping_refcnt;
+ rtems_libio_unlock();
+ return refcnt;
+}
+
+/**
+ * Decrement the reference count tracking number of mmap mappings of a file.
+ * Returns the updated reference count value.
+ */
+static inline uint32_t rtems_libio_decrement_mapping_refcnt(rtems_libio_t *iop)
+{
+ uint32_t refcnt;
+ rtems_libio_lock();
+ refcnt = --iop->mapping_refcnt;
+ rtems_libio_unlock();
+ return refcnt;
+}
+
+static inline bool rtems_libio_is_mapped(rtems_libio_t *iop)
+{
+ bool is_mapped;
+ rtems_libio_lock();
+ is_mapped = iop->mapping_refcnt != 0;
+ rtems_libio_unlock();
+ return is_mapped;
+}
+
/*
* File System Routine Prototypes
*/
diff --git a/cpukit/libcsupport/src/libio.c b/cpukit/libcsupport/src/libio.c
index 22be641..e89634f 100644
--- a/cpukit/libcsupport/src/libio.c
+++ b/cpukit/libcsupport/src/libio.c
@@ -138,8 +138,36 @@ void rtems_libio_free(
rtems_libio_lock();
iop->flags = 0;
- iop->data1 = rtems_libio_iop_freelist;
- rtems_libio_iop_freelist = iop;
+ /* If the mapping_refcnt is non-zero, the deferred free will be
+ * called by munmap. The iop is no longer good to use, but it cannot
+ * be recycled until the mapped file is unmapped. deferred free knows
+ * it can recycle the iop in case flags == 0 and iop->data1 == iop,
+ * since these two conditions are not otherwise satisifed at
+ * the same time. It may be possible that iop->data1 == iop when
+ * flags != 0 because data1 is private to the driver. However, flags == 0
+ * means a freed iop, and an iop on the freelist cannot store a pointer
+ * to itself in data1, or else the freelist is corrupted. We can't use
+ * NULL in data1 as an indicator because it is used by the tail of the
+ * freelist. */
+ if ( iop->mapping_refcnt == 0 ) {
+ iop->data1 = rtems_libio_iop_freelist;
+ rtems_libio_iop_freelist = iop;
+ } else {
+ iop->data1 = iop;
+ }
rtems_libio_unlock();
}
+
+void rtems_libio_check_deferred_free(
+ rtems_libio_t *iop
+)
+{
+ rtems_libio_lock();
+ if ( iop->mapping_refcnt == 0 && iop->flags == 0 && iop->data1 == iop) {
+ /* No mappings and rtems_libio_free already called, recycle the iop */
+ iop->data1 = rtems_libio_iop_freelist;
+ rtems_libio_iop_freelist = iop;
+ }
+ rtems_libio_unlock();
+}
diff --git a/cpukit/posix/include/rtems/posix/mmanimpl.h b/cpukit/posix/include/rtems/posix/mmanimpl.h
index 78bda53..eff20e2 100644
--- a/cpukit/posix/include/rtems/posix/mmanimpl.h
+++ b/cpukit/posix/include/rtems/posix/mmanimpl.h
@@ -16,6 +16,7 @@
#ifndef _RTEMS_POSIX_MMANIMPL_H
#define _RTEMS_POSIX_MMANIMPL_H
+#include <rtems/libio_.h>
#include <rtems/chain.h> /* FIXME: use score chains for proper layering? */
#ifdef __cplusplus
@@ -32,6 +33,8 @@ typedef struct mmap_mappings_s {
void* addr; /**< The address of the mapped memory */
size_t len; /**< The length of memory mapped */
int flags; /**< The mapping flags */
+ rtems_libio_t *iop; /**< The mapped object's file descriptor pointer */
+ bool is_shm; /**< True if mapping a shared memory object */
} mmap_mapping;
extern rtems_chain_control mmap_mappings;
diff --git a/cpukit/posix/include/rtems/posix/shmimpl.h b/cpukit/posix/include/rtems/posix/shmimpl.h
index 6d81e5e..f16af81 100644
--- a/cpukit/posix/include/rtems/posix/shmimpl.h
+++ b/cpukit/posix/include/rtems/posix/shmimpl.h
@@ -96,6 +96,36 @@ static inline POSIX_Shm_Control* loc_to_shm(
return (POSIX_Shm_Control*) loc->node_access;
}
+static inline int POSIX_Shm_Attempt_delete(
+ POSIX_Shm_Control* shm
+)
+{
+ Objects_Control *obj;
+ ISR_lock_Context lock_ctx;
+ int err;
+
+ err = 0;
+
+ _Objects_Allocator_lock();
+ --shm->reference_count;
+ if ( shm->reference_count == 0 ) {
+ if ( (*shm->shm_object.ops->object_delete)( &shm->shm_object ) != 0 ) {
+ err = EIO;
+ }
+ }
+ /* check if the object has been unlinked yet. */
+ obj = _Objects_Get( shm->Object.id, &lock_ctx, &_POSIX_Shm_Information );
+ if ( obj == NULL ) {
+ /* if it was unlinked, then it can be freed. */
+ _POSIX_Shm_Free( shm );
+ } else {
+ /* it will be freed when it is unlinked. */
+ _ISR_lock_ISR_enable( &lock_ctx );
+ }
+ _Objects_Allocator_unlock();
+ return err;
+}
+
/** @} */
#ifdef __cplusplus
diff --git a/cpukit/posix/src/mmap.c b/cpukit/posix/src/mmap.c
index 3c5f81e..b1d1653 100644
--- a/cpukit/posix/src/mmap.c
+++ b/cpukit/posix/src/mmap.c
@@ -25,6 +25,7 @@
#include "rtems/libio_.h"
#include <rtems/posix/mmanimpl.h>
+#include <rtems/posix/shmimpl.h>
#define RTEMS_MUTEX_ATTRIBS \
(RTEMS_PRIORITY | RTEMS_SIMPLE_BINARY_SEMAPHORE | \
@@ -106,13 +107,23 @@ bool mmap_mappings_lock_release(
return true;
}
+static void shm_mmap( rtems_libio_t *iop )
+{
+ POSIX_Shm_Control *shm = iop_to_shm( iop );
+
+ _Objects_Allocator_lock();
+ ++shm->reference_count;
+ _Objects_Allocator_unlock();
+}
+
void *mmap(
void *addr, size_t len, int prot, int flags, int fildes, off_t off
)
{
- struct stat sb;
- mmap_mapping* mapping;
- ssize_t r;
+ struct stat sb;
+ mmap_mapping *mapping;
+ ssize_t r;
+ rtems_libio_t *iop;
/*
* Clear errno.
@@ -130,6 +141,10 @@ void *mmap(
return MAP_FAILED;
}
+ /* Sucessful fstat ensures we have a good file descriptor. Get the
+ * associated iop for later. */
+ iop = rtems_libio_iop( fildes );
+
if ( len == 0 ) {
errno = EINVAL;
return MAP_FAILED;
@@ -217,6 +232,7 @@ void *mmap(
mapping->len = len;
mapping->flags = flags;
+ mapping->iop = iop;
if (( flags & MAP_FIXED ) != MAP_FIXED ) {
mapping->addr = malloc( len );
@@ -251,13 +267,32 @@ void *mmap(
return MAP_FAILED;
}
+ /* FIXME: should have a better generic way to handle differences between
+ * shm objects and other mmap'd files. the best way is probably to add an
+ * mmap/munmap handler to the file_operations table, or some other reasonable
+ * OOP-like approach to discriminate the 'type' of mapping. */
+ if ( S_ISREG( sb.st_mode ) || S_ISBLK( sb.st_mode ) ||
+ S_ISCHR( sb.st_mode ) || S_ISFIFO( sb.st_mode ) ||
+ S_ISSOCK( sb.st_mode ) ) {
+ mapping->is_shm = false;
+ } else {
+ mapping->is_shm = true;
+ shm_mmap(iop);
+ }
+
+ /* FIXME:
+ * MAP_SHARED is not supported for regular files, and probably should be?
+ * MAP_PRIVATE is not supported for shared memory objects, and should be?
+ */
+
/* FIXME: for a mapping with MAP_SHARED and PROT_WRITE the mtime/ctime
* should be updated when there is a write reference to the mapped region.
* How do we make this happen? */
- /* FIXME: add an extra reference to the file associated with fildes that
+ /* add an extra reference to the file associated with fildes that
* is not removed by a subsequent close(). This reference shall be removed
* when there are no more mappings to the file. */
+ rtems_libio_increment_mapping_refcnt(iop);
rtems_chain_append( &mmap_mappings, &mapping->node );
diff --git a/cpukit/posix/src/munmap.c b/cpukit/posix/src/munmap.c
index d3687df..b315034 100644
--- a/cpukit/posix/src/munmap.c
+++ b/cpukit/posix/src/munmap.c
@@ -16,13 +16,14 @@
#include <sys/mman.h>
#include <rtems/posix/mmanimpl.h>
+#include <rtems/posix/shmimpl.h>
-int munmap(
- void *addr, size_t len
-)
+int munmap(void *addr, size_t len)
{
- mmap_mapping* mapping;
- rtems_chain_node* node;
+ mmap_mapping *mapping;
+ rtems_chain_node *node;
+ uint32_t refcnt;
+ POSIX_Shm_Control *shm;
/*
* Clear errno.
@@ -45,19 +46,25 @@ int munmap(
node = rtems_chain_first (&mmap_mappings);
while ( !rtems_chain_is_tail( &mmap_mappings, node )) {
- /*
- * If the map is fixed see if this address is already mapped. At this
- * point in time if there is an overlap in the mappings we return an
- * error.
- */
mapping = (mmap_mapping*) node;
if ( ( addr >= mapping->addr ) &&
( addr < ( mapping->addr + mapping->len )) ) {
rtems_chain_extract( node );
+ /* FIXME: generally need a way to clean-up the backing object, but
+ * currently it only matters for shm objects. */
+ if ( mapping->is_shm == true ) {
+ /* decrement mmap's shm reference_count and maybe delete the object */
+ shm = iop_to_shm(mapping->iop);
+ POSIX_Shm_Attempt_delete(shm);
+ }
+ refcnt = rtems_libio_decrement_mapping_refcnt(mapping->iop);
+ if ( refcnt == 0 ) {
+ rtems_libio_check_deferred_free(mapping->iop);
+ }
if (( mapping->flags & MAP_FIXED ) != MAP_FIXED ) {
free( mapping->addr );
- free( mapping );
}
+ free( mapping );
break;
}
node = rtems_chain_next( node );
diff --git a/cpukit/posix/src/shmopen.c b/cpukit/posix/src/shmopen.c
index bedde15..fa1027e 100644
--- a/cpukit/posix/src/shmopen.c
+++ b/cpukit/posix/src/shmopen.c
@@ -93,34 +93,13 @@ static int shm_ftruncate( rtems_libio_t *iop, off_t length )
static int shm_close( rtems_libio_t *iop )
{
POSIX_Shm_Control *shm = iop_to_shm( iop );
- Objects_Control *obj;
- ISR_lock_Context lock_ctx;
int err;
err = 0;
- _Objects_Allocator_lock();
-
- --shm->reference_count;
- if ( shm->reference_count == 0 ) {
- /* TODO: need to make sure this counts mmaps too! */
- if ( (*shm->shm_object.ops->object_delete)( &shm->shm_object ) != 0 ) {
- err = EIO;
- }
- /* check if the object has been unlinked yet. */
- obj = _Objects_Get( shm->Object.id, &lock_ctx, &_POSIX_Shm_Information );
- if ( obj == NULL ) {
- /* if it was unlinked, then it can be freed. */
- _POSIX_Shm_Free( shm );
- } else {
- /* it will be freed when it is unlinked. */
- _ISR_lock_ISR_enable( &lock_ctx );
- }
- }
+ POSIX_Shm_Attempt_delete(shm);
iop->pathinfo.node_access = NULL;
- _Objects_Allocator_unlock();
-
if ( err != 0 ) {
rtems_set_errno_and_return_minus_one( err );
}
diff --git a/cpukit/posix/src/shmunlink.c b/cpukit/posix/src/shmunlink.c
index ab18450..f81e234 100644
--- a/cpukit/posix/src/shmunlink.c
+++ b/cpukit/posix/src/shmunlink.c
@@ -40,8 +40,8 @@ int shm_unlink( const char *name )
default:
_Objects_Close( &_POSIX_Shm_Information, &shm->Object );
if ( shm->reference_count == 0 ) {
- /* TODO: need to make sure this counts mmaps too! */
- /* only remove the shm object if no references exist to it. */
+ /* Only remove the shm object if no references exist to it. Otherwise,
+ * the shm object will be freed later in _POSIX_Shm_Attempt_delete */
_POSIX_Shm_Free( shm );
}
break;
--
2.7.4
More information about the devel
mailing list