[rtems commit] libio: Remove special-case reference count

Sebastian Huber sebh at rtems.org
Thu Sep 14 05:03:12 UTC 2017


Module:    rtems
Branch:    master
Commit:    694e946dbd64c94343aeb289edd80a60759f7b26
Changeset: http://git.rtems.org/rtems/commit/?id=694e946dbd64c94343aeb289edd80a60759f7b26

Author:    Sebastian Huber <sebastian.huber at embedded-brains.de>
Date:      Wed Sep  6 07:31:48 2017 +0200

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 8226d18..7022de6 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 695a4c4..c2fb975 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 e89634f..22be641 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 ff59d91..e1cc672 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 b5af180..9d9c063 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 fb9bb87..5348be7 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



More information about the vc mailing list