[PATCH] libio: Remove special-case reference count
Sebastian Huber
sebastian.huber at embedded-brains.de
Wed Sep 6 06:27:41 UTC 2017
The top-level IO library structures should contain no special-case data.
Update #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 | 12 ++++-----
cpukit/posix/src/mmap.c | 24 ++++++++---------
cpukit/posix/src/munmap.c | 29 ++++++--------------
6 files changed, 27 insertions(+), 112 deletions(-)
diff --git a/cpukit/libcsupport/include/rtems/libio.h b/cpukit/libcsupport/include/rtems/libio.h
index 8226d18ba2..7022de671c 100644
--- a/cpukit/libcsupport/include/rtems/libio.h
+++ b/cpukit/libcsupport/include/rtems/libio.h
@@ -1320,7 +1320,6 @@ 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 695a4c45a5..c2fb975bf7 100644
--- a/cpukit/libcsupport/include/rtems/libio_.h
+++ b/cpukit/libcsupport/include/rtems/libio_.h
@@ -304,47 +304,6 @@ 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 e89634f090..22be6411a2 100644
--- a/cpukit/libcsupport/src/libio.c
+++ b/cpukit/libcsupport/src/libio.c
@@ -138,36 +138,8 @@ void rtems_libio_free(
rtems_libio_lock();
iop->flags = 0;
- /* 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;
- }
+ iop->data1 = rtems_libio_iop_freelist;
+ rtems_libio_iop_freelist = 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 ff59d911ca..e1cc672331 100644
--- a/cpukit/posix/include/rtems/posix/mmanimpl.h
+++ b/cpukit/posix/include/rtems/posix/mmanimpl.h
@@ -18,6 +18,7 @@
#include <rtems/libio_.h>
#include <rtems/chain.h> /* FIXME: use score chains for proper layering? */
+#include <rtems/posix/shm.h>
#ifdef __cplusplus
extern "C" {
@@ -29,12 +30,11 @@ extern "C" {
* Every mmap'ed region has a mapping.
*/
typedef struct mmap_mappings_s {
- rtems_chain_node node; /**< The mapping chain's node */
- 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_shared_shm; /**< True if MAP_SHARED of shared memory */
+ rtems_chain_node node; /**< The mapping chain's node */
+ void* addr; /**< The address of the mapped memory */
+ size_t len; /**< The length of memory mapped */
+ int flags; /**< The mapping flags */
+ POSIX_Shm_Control *shm; /**< The shared memory object or NULL */
} mmap_mapping;
extern rtems_chain_control mmap_mappings;
diff --git a/cpukit/posix/src/mmap.c b/cpukit/posix/src/mmap.c
index b5af180d3d..9d9c0634ff 100644
--- a/cpukit/posix/src/mmap.c
+++ b/cpukit/posix/src/mmap.c
@@ -48,6 +48,7 @@ void *mmap(
bool map_anonymous;
bool map_shared;
bool map_private;
+ bool is_shared_shm;
int err;
map_fixed = (flags & MAP_FIXED) == MAP_FIXED;
@@ -194,7 +195,6 @@ void *mmap(
memset( mapping, 0, sizeof( mmap_mapping ));
mapping->len = len;
mapping->flags = flags;
- mapping->iop = iop;
if ( !map_anonymous ) {
/*
@@ -206,19 +206,19 @@ void *mmap(
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_shared_shm = false;
+ is_shared_shm = false;
} else {
- mapping->is_shared_shm = true;
+ is_shared_shm = true;
}
} else {
- mapping->is_shared_shm = false;
+ is_shared_shm = false;
}
if ( map_fixed ) {
mapping->addr = addr;
} else if ( map_private ) {
/* private mappings of shared memory do not need special treatment. */
- mapping->is_shared_shm = false;
+ is_shared_shm = false;
posix_memalign( &mapping->addr, PAGE_SIZE, len );
if ( !mapping->addr ) {
free( mapping );
@@ -228,7 +228,7 @@ void *mmap(
}
/* MAP_FIXED is not supported for shared memory objects with MAP_SHARED. */
- if ( map_fixed && mapping->is_shared_shm ) {
+ if ( map_fixed && is_shared_shm ) {
free( mapping );
errno = ENOTSUP;
return MAP_FAILED;
@@ -280,6 +280,11 @@ void *mmap(
memset( mapping->addr, 0, len );
}
} else if ( map_shared ) {
+ if ( is_shared_shm ) {
+ /* FIXME: This use of implementation details is a hack. */
+ mapping->shm = iop_to_shm( iop );
+ }
+
err = (*iop->pathinfo.handlers->mmap_h)(
iop, &mapping->addr, len, prot, off );
if ( err != 0 ) {
@@ -289,13 +294,6 @@ void *mmap(
}
}
- if ( iop != NULL ) {
- /* 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_unprotected( &mmap_mappings, &mapping->node );
mmap_mappings_lock_release( );
diff --git a/cpukit/posix/src/munmap.c b/cpukit/posix/src/munmap.c
index fb9bb872e0..5348be7a51 100644
--- a/cpukit/posix/src/munmap.c
+++ b/cpukit/posix/src/munmap.c
@@ -16,23 +16,14 @@
#include <stdlib.h>
#include <sys/mman.h>
-#include <rtems/posix/mmanimpl.h>
+#include <rtems/posix/mmanimpl.h>
#include <rtems/posix/shmimpl.h>
-static void shm_munmap( rtems_libio_t *iop )
-{
- POSIX_Shm_Control *shm = iop_to_shm( iop );
-
- /* decrement mmap's shm reference_count and maybe delete the object */
- POSIX_Shm_Attempt_delete(shm);
-}
-
int munmap(void *addr, size_t len)
{
- mmap_mapping *mapping;
- rtems_chain_node *node;
- uint32_t refcnt;
-
+ mmap_mapping *mapping;
+ rtems_chain_node *node;
+
/*
* Clear errno.
*/
@@ -60,17 +51,13 @@ int munmap(void *addr, size_t len)
if ( ( addr >= mapping->addr ) &&
( addr < ( mapping->addr + mapping->len )) ) {
rtems_chain_extract_unprotected( node );
+
/* FIXME: generally need a way to clean-up the backing object, but
* currently it only matters for MAP_SHARED shm objects. */
- if ( mapping->is_shared_shm == true ) {
- shm_munmap(mapping->iop);
- }
- if ( mapping->iop != NULL ) {
- refcnt = rtems_libio_decrement_mapping_refcnt(mapping->iop);
- if ( refcnt == 0 ) {
- rtems_libio_check_deferred_free(mapping->iop);
- }
+ if ( mapping->shm != NULL ) {
+ POSIX_Shm_Attempt_delete(mapping->shm);
}
+
/* only free the mapping address for non-fixed mapping */
if (( mapping->flags & MAP_FIXED ) != MAP_FIXED ) {
/* only free the mapping address for non-shared mapping, because we
--
2.12.3
More information about the devel
mailing list