[PATCH v2 6/6] posix/mmap: add mmap hooks for MAP_SHARED shms

Gedare Bloom gedare at rtems.org
Thu Apr 13 20:22:34 UTC 2017


Updates #2859.
---
 cpukit/posix/include/rtems/posix/mmanimpl.h |   2 +-
 cpukit/posix/include/rtems/posix/shm.h      |  30 ++++
 cpukit/posix/src/mmap.c                     | 208 +++++++++++++++++-----------
 cpukit/posix/src/munmap.c                   |  17 ++-
 cpukit/posix/src/shmheap.c                  |  20 +++
 cpukit/posix/src/shmwkspace.c               |  17 +++
 6 files changed, 203 insertions(+), 91 deletions(-)

diff --git a/cpukit/posix/include/rtems/posix/mmanimpl.h b/cpukit/posix/include/rtems/posix/mmanimpl.h
index eff20e2..bb33ac9 100644
--- a/cpukit/posix/include/rtems/posix/mmanimpl.h
+++ b/cpukit/posix/include/rtems/posix/mmanimpl.h
@@ -34,7 +34,7 @@ typedef struct mmap_mappings_s {
   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 */
+  bool             is_shared_shm; /**< True if MAP_SHARED of shared memory */
 } mmap_mapping;
 
 extern rtems_chain_control mmap_mappings;
diff --git a/cpukit/posix/include/rtems/posix/shm.h b/cpukit/posix/include/rtems/posix/shm.h
index dfdd58e..9284b39 100644
--- a/cpukit/posix/include/rtems/posix/shm.h
+++ b/cpukit/posix/include/rtems/posix/shm.h
@@ -91,6 +91,16 @@ struct POSIX_Shm_Object_operations {
    * Returns the number of bytes read (copied) into @a buf.
    */
   int ( *object_read ) ( POSIX_Shm_Object *shm_obj, void *buf, size_t count );
+
+  /**
+   * @brief Maps a shared memory object.
+   *
+   * Establishes a memory mapping between the shared memory object and the
+   * caller.
+   *
+   * Returns the mapped address of the object.
+   */
+  void * ( *object_mmap ) ( POSIX_Shm_Object *shm_obj, size_t len, int prot, off_t off);
 };
 
 /**
@@ -144,6 +154,16 @@ extern int _POSIX_Shm_Object_read_from_workspace(
 );
 
 /**
+ * @brief object_mmap operation for shm objects stored in RTEMS Workspace.
+ */
+extern void * _POSIX_Shm_Object_mmap_from_workspace(
+  POSIX_Shm_Object *shm_obj,
+  size_t len,
+  int prot,
+  off_t off
+);
+
+/**
  * @brief object_create operation for shm objects stored in C program heap.
  */
 extern int _POSIX_Shm_Object_create_from_heap(
@@ -173,6 +193,16 @@ extern int _POSIX_Shm_Object_read_from_heap(
   size_t count
 );
 
+/**
+ * @brief object_mmap operation for shm objects stored in C program heap.
+ */
+extern void * _POSIX_Shm_Object_mmap_from_heap(
+  POSIX_Shm_Object *shm_obj,
+  size_t len,
+  int prot,
+  off_t off
+);
+
 /** @} */
 
 #ifdef __cplusplus
diff --git a/cpukit/posix/src/mmap.c b/cpukit/posix/src/mmap.c
index b1d1653..74c8a33 100644
--- a/cpukit/posix/src/mmap.c
+++ b/cpukit/posix/src/mmap.c
@@ -24,7 +24,7 @@
 
 #include "rtems/libio_.h"
 
-#include <rtems/posix/mmanimpl.h> 
+#include <rtems/posix/mmanimpl.h>
 #include <rtems/posix/shmimpl.h>
 
 #define RTEMS_MUTEX_ATTRIBS \
@@ -107,13 +107,28 @@ bool mmap_mappings_lock_release(
   return true;
 }
 
-static void shm_mmap( rtems_libio_t *iop )
+/* Helper function only gets called for mmap mappings of shared memory objects
+ * with the MAP_SHARED flag.
+ */
+static void *shm_mmap( rtems_libio_t *iop, size_t len, int prot, off_t off)
 {
   POSIX_Shm_Control *shm = iop_to_shm( iop );
+  void *m;
 
   _Objects_Allocator_lock();
-  ++shm->reference_count;
+
+  m = (*shm->shm_object.ops->object_mmap)( &shm->shm_object, len, prot, off);
+  if ( m != NULL ) {
+    /* Keep a reference in the shared memory to prevent its removal. */
+    ++shm->reference_count;
+
+    /* Update atime */
+    _POSIX_Shm_Update_atime(shm);
+  }
+
   _Objects_Allocator_unlock();
+
+  return m;
 }
 
 void *mmap(
@@ -124,12 +139,13 @@ void *mmap(
   mmap_mapping   *mapping;
   ssize_t         r;
   rtems_libio_t  *iop;
-  
-  /*
-   * Clear errno.
-   */
+  bool            map_fixed;
+
+  map_fixed = (flags & MAP_FIXED) == MAP_FIXED;
+
+  /* Clear errno. */
   errno = 0;
-  
+
   /*
    * Get a stat of the file to get the dev + inode number and to make sure the
    * fd is ok. The normal libio calls cannot be used because we need to return
@@ -141,8 +157,7 @@ void *mmap(
     return MAP_FAILED;
   }
 
-  /* Sucessful fstat ensures we have a good file descriptor. Get the
-   * associated iop for later. */
+  /* fstat ensures we have a good file descriptor. Hold on to iop. */
   iop = rtems_libio_iop( fildes );
 
   if ( len == 0 ) {
@@ -150,14 +165,12 @@ void *mmap(
     return MAP_FAILED;
   }
 
-  /*
-   * Check the type of file we have and make sure it is supported.
-   */
+  /* Check the type of file we have and make sure it is supported. */
   if ( S_ISDIR( sb.st_mode ) || S_ISLNK( sb.st_mode )) {
     errno = ENODEV;
     return MAP_FAILED;
   }
-  
+
   /*
    * We can provide read, write and execute because the memory in RTEMS does
    * not normally have protections but we cannot hide access to memory.
@@ -166,7 +179,18 @@ void *mmap(
     errno = ENOTSUP;
     return MAP_FAILED;
   }
-  
+
+  /* Either MAP_SHARED or MAP_PRIVATE must be defined, but not both */
+  if ( (flags & MAP_SHARED) == MAP_SHARED ) {
+    if ( (flags & MAP_PRIVATE) == MAP_PRIVATE ) {
+      errno = EINVAL;
+      return MAP_FAILED;
+    }
+  } else if ( (flags & MAP_PRIVATE != MAP_PRIVATE) ) {
+    errno = EINVAL;
+    return MAP_FAILED;
+  }
+
   /*
    * We can not normally provide restriction of write access. Reject any
    * attempt to map without write permission, since we are not able to
@@ -177,11 +201,9 @@ void *mmap(
     return MAP_FAILED;
   }
 
-  /*
-   * Check to see if the mapping is valid for a regular file.
-   */
+  /* Check to see if the mapping is valid for a regular file. */
   if ( S_ISREG( sb.st_mode )
-  /* FIXME: Should this be using strict inequality comparisons? It would
+  /* FIXME: Should this be using strict inequality (>) comparisons? It would
    * be valid to map a region exactly equal to the st_size, e.g. see below. */
        && (( off >= sb.st_size ) || (( off + len ) >= sb.st_size ))) {
     errno = EOVERFLOW;
@@ -190,29 +212,92 @@ void *mmap(
 
   /*
    * Check to see if the mapping is valid for other file/object types.
+   * Does this satisfy for devices?
    */
   if ( sb.st_size < off + len ) {
     errno = ENXIO;
     return MAP_FAILED;
   }
 
+  /* Do not seek on character devices, pipes, sockets, or memory objects. */
+  if ( S_ISREG( sb.st_mode ) || S_ISBLK( sb.st_mode ) ) {
+    if ( lseek( fildes, off, SEEK_SET ) < 0 ) {
+      return MAP_FAILED;
+    }
+  }
+
+  /* Create the mapping */
+  mapping = malloc( sizeof( mmap_mapping ));
+  if ( !mapping ) {
+    errno = ENOMEM;
+    return NULL;
+  }
+  memset( mapping, 0, sizeof( mmap_mapping ));
+  mapping->len = len;
+  mapping->flags = flags;
+  mapping->iop = iop;
+
+  /*
+   * HACK: We should have a better generic way to distinguish between
+   * shm objects and other mmap'd files. We need to know at munmap time
+   * if the mapping is to a shared memory object in order to refcnt shms.
+   * We could do this by providing mmap in the file operations if needed.
+   */
+  if ( (MAP_PRIVATE == (flags & MAP_PRIVATE)) ||
+       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;
+  } else {
+    mapping->is_shared_shm = true;
+  }
+
   /*
-   * Obtain the mmap lock. Sets errno on failure.
+   * MAP_SHARED currently is only supported for shared memory objects.
    */
+  if ( (MAP_SHARED == (flags & MAP_SHARED)) && (mapping->is_shared_shm == false) ) {
+    free( mapping );
+    errno = ENOTSUP;
+    return MAP_FAILED;
+  }
+
+  if ( map_fixed ) {
+    mapping->addr = addr;
+  } else if ( MAP_PRIVATE == (flags & MAP_PRIVATE) ) {
+    /* private mappings of shared memory do not need special treatment. */
+    mapping->is_shared_shm = false;
+    mapping->addr = malloc( len );
+    if ( !mapping->addr ) {
+      free( mapping );
+      errno = ENOMEM;
+      return MAP_FAILED;
+    }
+  }
+
+  /* MAP_FIXED is not supported for shared memory objects with MAP_SHARED. */
+  if ( map_fixed && mapping->is_shared_shm ) {
+    free( mapping );
+    errno = ENOTSUP;
+    return MAP_FAILED;
+  }
+
+  /* Lock access to mmap_mappings. Sets errno on failure. */
   if ( !mmap_mappings_lock_obtain( ) )
     return MAP_FAILED;
 
-  if (( flags & MAP_FIXED ) == MAP_FIXED ) {
+  if ( map_fixed ) {
     rtems_chain_node* 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.
+       * error. POSIX allows us to also return successfully by unmapping
+       * the overlapping prior mappings.
        */
       mapping = (mmap_mapping*) node;
       if ( ( addr >= mapping->addr ) &&
            ( addr < ( mapping->addr + mapping->len )) ) {
+        free( mapping );
         mmap_mappings_lock_release( );
         errno = ENXIO;
         return MAP_FAILED;
@@ -221,74 +306,29 @@ void *mmap(
     }
   }
 
-  mapping = malloc( sizeof( mmap_mapping ));
-  if ( !mapping ) {
-    mmap_mappings_lock_release( );
-    errno = ENOMEM;
-    return NULL;
-  }
-
-  memset( mapping, 0, sizeof( mmap_mapping ));
-
-  mapping->len = len;
-  mapping->flags = flags;
-  mapping->iop = iop;
-  
-  if (( flags & MAP_FIXED ) != MAP_FIXED ) {
-    mapping->addr = malloc( len );
-    if ( !mapping->addr ) {
-      mmap_mappings_lock_release( );
-      free( mapping );
-      errno = ENOMEM;
-      return MAP_FAILED;
-    }
-
+  /* Populate the data */
+  if ( MAP_PRIVATE == (flags & MAP_PRIVATE) ) {
     /*
-     * Do not seek on character devices, pipes, sockets, or memory objects.
+     * Use read() for private mappings. This updates atime as needed.
+     * Changes to the underlying object will NOT be reflected in the mapping.
+     * The underlying object can be removed while the mapping exists.
      */
-    if ( S_ISREG( sb.st_mode ) || S_ISBLK( sb.st_mode ) ) {
-      if ( lseek( fildes, off, SEEK_SET ) < 0 ) {
-        mmap_mappings_lock_release( );
+    r = read( fildes, mapping->addr, len );
+
+    if ( r != len ) {
+      mmap_mappings_lock_release( );
+      if ( !map_fixed ) {
         free( mapping->addr );
-        free( mapping );
-        return MAP_FAILED;
       }
+      free( mapping );
+      errno = ENXIO;
+      return MAP_FAILED;
     }
+  } else if ( MAP_SHARED == (flags & MAP_SHARED) ) {
+    /* Currently only shm objects can be MAP_SHARED. */
+    mapping->addr = shm_mmap(iop, len, prot, off);
   }
 
-  /* read updates atime satisfying POSIX spec for mmap */
-  r = read( fildes, mapping->addr, len );
-
-  if ( r != len ) {
-    mmap_mappings_lock_release( );
-    free( mapping->addr );
-    free( mapping );
-    errno = ENXIO;
-    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? */
-
   /* 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. */
@@ -297,6 +337,6 @@ void *mmap(
   rtems_chain_append( &mmap_mappings, &mapping->node );
 
   mmap_mappings_lock_release( );
-  
+
   return mapping->addr;
 }
diff --git a/cpukit/posix/src/munmap.c b/cpukit/posix/src/munmap.c
index b315034..588562f 100644
--- a/cpukit/posix/src/munmap.c
+++ b/cpukit/posix/src/munmap.c
@@ -18,12 +18,19 @@
 #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;
-  POSIX_Shm_Control *shm;
   
   /*
    * Clear errno.
@@ -51,11 +58,9 @@ int munmap(void *addr, size_t len)
          ( 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);
+       * currently it only matters for MAP_SHARED shm objects. */
+      if ( mapping->is_shared_shm == true ) {
+        shm_munmap(mapping->iop);
       }
       refcnt = rtems_libio_decrement_mapping_refcnt(mapping->iop);
       if ( refcnt == 0 ) {
diff --git a/cpukit/posix/src/shmheap.c b/cpukit/posix/src/shmheap.c
index 3449c15..47f5b47 100644
--- a/cpukit/posix/src/shmheap.c
+++ b/cpukit/posix/src/shmheap.c
@@ -70,6 +70,7 @@ int _POSIX_Shm_Object_resize_from_heap(
   return err;
 }
 
+/* This is identical to _POSIX_Shm_Object_read_from_wkspace */
 int _POSIX_Shm_Object_read_from_heap(
   POSIX_Shm_Object *shm_obj,
   void *buf,
@@ -88,3 +89,22 @@ int _POSIX_Shm_Object_read_from_heap(
   return count;
 }
 
+/* This is identical to _POSIX_Shm_Object_mmap_from_wkspace */
+void * _POSIX_Shm_Object_mmap_from_heap(
+  POSIX_Shm_Object *shm_obj,
+  size_t len,
+  int prot,
+  off_t off
+)
+{
+  if ( shm_obj == NULL || shm_obj->handle == NULL )
+    return 0;
+
+  /* This is already checked by mmap. Maybe make it a debug assert? */
+  if ( shm_obj->size < len + off ) {
+    return NULL;
+  }
+
+  return &(shm_obj->handle[off]);
+}
+
diff --git a/cpukit/posix/src/shmwkspace.c b/cpukit/posix/src/shmwkspace.c
index 6d6cd21..926c14b 100644
--- a/cpukit/posix/src/shmwkspace.c
+++ b/cpukit/posix/src/shmwkspace.c
@@ -75,4 +75,21 @@ int _POSIX_Shm_Object_read_from_workspace(
   return count;
 }
 
+void * _POSIX_Shm_Object_mmap_from_wkspace(
+  POSIX_Shm_Object *shm_obj,
+  size_t len,
+  int prot,
+  off_t off
+)
+{
+  if ( shm_obj == NULL || shm_obj->handle == NULL )
+    return 0;
+
+  /* This is already checked by mmap. Maybe make it a debug assert? */
+  if ( shm_obj->size < len + off ) {
+    return NULL;
+  }
+
+  return &(shm_obj->handle[off]);
+}
 
-- 
2.7.4



More information about the devel mailing list