From 694e946dbd64c94343aeb289edd80a60759f7b26 Mon Sep 17 00:00:00 2001 From: Sebastian Huber Date: Wed, 6 Sep 2017 07:31:48 +0200 Subject: libio: Remove special-case reference count 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 #include /* FIXME: use score chains for proper layering? */ +#include #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 #include -#include +#include #include -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 -- cgit v1.2.3