modified mpool module to contain flags - used to determine if the mpool will

be used in MPI_Alloc_mem operations. Note that we found an interesting bug in
which if memory was allocated by the sm mpool (via mmap) and then registered
via the mvapi mpool, the registration would fail on certain systems. 

Added mca param mpool_base_use_mem_hooks, set to 1 to enable the memory hooks
so that memory is deregistered if the user frees it behind our back. This is
only useful if the mca param mpi_leave_pinned is also set to 1. Otherwise all
registrations are deregistered within the MPI library or via
MPI_Free_buf. After testing we should probably set both mpi_leave_pinned and
mpool_base_use_mem_hooks to default to 1. 

This commit was SVN r7415.

ompi/mca/mpool/base/base.h

@@ -46,54 +46,6 @@ OBJ_CLASS_DECLARATION(mca_mpool_base_selected_module_t);
  * Data structures for the tree of allocated memory
  */
 
-/**
- * The maximum number of mpools a chunk of memorry can be registered with
- */
-#define MCA_MPOOL_BASE_MAX_REG 8
-
-/**
- * Holds the key for the tree
- */
-struct mca_mpool_base_key_t
-{
-    void * bottom;          /**< the bottom of the memory range */
-    void * top;             /**< the top of the memory range */
-};
-typedef struct mca_mpool_base_key_t mca_mpool_base_key_t;
-
-/**
- * Holds a pointer to the mpool the memory is registered with and 
- * a user pointer for mpool specific information
- */
-struct mca_mpool_base_reg_mpool_t
-{
-    mca_mpool_base_module_t * mpool; /**< the registered memory pool */
-    void* user_data; /**< user data */
-    mca_mpool_base_registration_t* mpool_registration; /**< mpool specific info associated w/ registration */
-};
-typedef struct mca_mpool_base_reg_mpool_t mca_mpool_base_reg_mpool_t;
-
-/**
- * Holds all the information about a chunk of registered memory. The whole
- * structure serves as a value in the tree
- */
-struct mca_mpool_base_chunk_t
-{
-    opal_list_item_t super;   /**< the parent class */
-    mca_mpool_base_key_t key; /**< the key which holds the memory pointers */
-    mca_mpool_base_reg_mpool_t mpools[MCA_MPOOL_BASE_MAX_REG]; 
-                              /**< the mpools the memory is registered with */
-};
-typedef struct mca_mpool_base_chunk_t mca_mpool_base_chunk_t;
-
-OBJ_CLASS_DECLARATION(mca_mpool_base_chunk_t);
-
-
-/**
- *  Returns a copy of the chunk.
- */
-mca_mpool_base_chunk_t* mca_mpool_base_find(void* base);
-
 /*
  * Global functions for MCA: overall mpool open and close
  */
@@ -115,9 +67,6 @@ OMPI_DECLSPEC int mca_mpool_base_module_destroy(mca_mpool_base_module_t *module)
 OMPI_DECLSPEC extern int mca_mpool_base_output;
 OMPI_DECLSPEC extern opal_list_t mca_mpool_base_components;
 OMPI_DECLSPEC extern opal_list_t mca_mpool_base_modules;
-OMPI_DECLSPEC extern ompi_free_list_t mca_mpool_base_mem_list;
-OMPI_DECLSPEC extern ompi_rb_tree_t mca_mpool_base_tree;
-OMPI_DECLSPEC extern opal_mutex_t mca_mpool_base_tree_lock; 
 
 #if defined(c_plusplus) || defined(__cplusplus)
 }

ompi/mca/mpool/base/mpool_base_alloc.c

@@ -25,60 +25,6 @@
 #include "mca/mpool/base/base.h"
 #include "opal/threads/mutex.h" 
 
-ompi_rb_tree_t mca_mpool_base_tree;
-ompi_free_list_t mca_mpool_base_mem_list;
-opal_mutex_t mca_mpool_base_tree_lock; 
-
-
-/**
- * Searches the mpool to see if it has allocated the memory that is passed in.
- * If so it returns an array of mpools the memory is registered with.
- *
- * @param base pointer to the memory to lookup
- *
- * @retval NULL if the memory is not in any mpool
- * @retval pointer to an array of type mca_mpool_base_reg_mpool_t
- */
-static inline struct mca_mpool_base_chunk_t * mca_mpool_base_find_nl(void * base)
-{
-    mca_mpool_base_key_t key;
-    key.bottom = base;
-    key.top = base;
-    return (mca_mpool_base_chunk_t *)ompi_rb_tree_find(&mca_mpool_base_tree, &key);
-}
-
-/**
- * Searches the mpool to see if it has allocated the memory that is passed in.
- * If so it returns an array of mpools the memory is registered with.
- *
- * @param base pointer to the memory to lookup
- *
- * @retval NULL if the memory is not in any mpool
- * @retval pointer to an array of type mca_mpool_base_reg_mpool_t
- */
-struct mca_mpool_base_chunk_t * mca_mpool_base_find(void * base)
-{
-    mca_mpool_base_chunk_t* found;
-    mca_mpool_base_chunk_t* copy;
-
-    OPAL_THREAD_LOCK(&mca_mpool_base_tree_lock);
-    if(NULL != (found = mca_mpool_base_find_nl(base))) {
-        mca_mpool_base_reg_mpool_t* reg;
-        copy = OBJ_NEW(mca_mpool_base_chunk_t);
-        *copy = *found;
-        reg = copy->mpools;
-        while(NULL != reg->mpool) {
-            if(NULL != reg->mpool_registration)
-                OBJ_RETAIN(reg->mpool_registration);
-            reg++;
-        }
-    } else {
-        copy = NULL;
-    }
-    OPAL_THREAD_UNLOCK(&mca_mpool_base_tree_lock);
-    return copy;
-}
-
 /**
  * Memory Pool Registration
  */
@@ -102,88 +48,6 @@ OBJ_CLASS_INSTANCE(
     mca_mpool_base_registration_constructor,
     mca_mpool_base_registration_destructor);
 
-/**
- * Function for the red black tree to compare 2 keys
- *
- * @param key1 a pointer to the 1st key
- * @param key2 a pointer to the second key
- *
- * @retval -1 if key1 is below key2
- * @retval 1 if key 1 is above key2
- * @retval 0 if the keys are the same
- */
-int mca_mpool_base_tree_node_compare(void * key1, void * key2)
-{
-    if(((mca_mpool_base_key_t *) key1)->bottom <
-       ((mca_mpool_base_key_t *) key2)->bottom)
-    {
-        return -1;
-    }
-    else if(((mca_mpool_base_key_t *) key1)->bottom >
-            ((mca_mpool_base_key_t *) key2)->top)
-    {
-        return 1;
-    }
-    else
-    {
-        return 0;
-    }
-}
-
-int mca_mpool_base_insert(void * addr, size_t size, 
-                          mca_mpool_base_module_t* mpool, 
-                          void* user_data,
-                          mca_mpool_base_registration_t* registration)
-{
-    opal_list_item_t *item; 
-    int rc; 
-    OMPI_FREE_LIST_GET(&mca_mpool_base_mem_list, item, rc);
-    if(rc != OMPI_SUCCESS) 
-        return rc; 
-    memset( ((mca_mpool_base_chunk_t *) item)->mpools, 0, sizeof(mca_mpool_base_reg_mpool_t) * MCA_MPOOL_BASE_MAX_REG); 
-    
-    ((mca_mpool_base_chunk_t *) item)->key.bottom = addr;
-    ((mca_mpool_base_chunk_t *) item)->key.top = (void *) 
-        ((char *) addr + size - 1);
-    ((mca_mpool_base_chunk_t *) item)->mpools[0].mpool = mpool; 
-    ((mca_mpool_base_chunk_t *) item)->mpools[0].user_data = user_data;
-    ((mca_mpool_base_chunk_t *) item)->mpools[0].mpool_registration = registration;
-    
-    
-    OPAL_THREAD_LOCK(&mca_mpool_base_tree_lock); 
-    rc = ompi_rb_tree_insert(&mca_mpool_base_tree, 
-                        &((mca_mpool_base_chunk_t *)item)->key, item);
-    OPAL_THREAD_UNLOCK(&mca_mpool_base_tree_lock); 
-    if(OMPI_SUCCESS != rc) {
-        OMPI_FREE_LIST_RETURN(&mca_mpool_base_mem_list, item);
-        return rc; 
-    }
-    return OMPI_SUCCESS; 
-}
-
-/**
- * Function to remove previously memory from the tree without freeing it
- *
- * @param base pointer to the memory to free
- *
- * @retval OMPI_SUCCESS
- * @retval OMPI_ERR_BAD_PARAM if the passed base pointer was invalid
- */
-int mca_mpool_base_remove(void * base)
-{
-    int rc; 
-    mca_mpool_base_chunk_t *chunk;
-
-    OPAL_THREAD_LOCK(&mca_mpool_base_tree_lock); 
-    if(NULL == (chunk = mca_mpool_base_find_nl(base))) {
-        OPAL_THREAD_UNLOCK(&mca_mpool_base_tree_lock); 
-        return OMPI_ERR_BAD_PARAM;
-    }
-    rc =  ompi_rb_tree_delete(&mca_mpool_base_tree, &chunk->key); 
-    OPAL_THREAD_UNLOCK(&mca_mpool_base_tree_lock);
-    return rc;
-}
-
 /**
  * Function to allocate special memory according to what the user requests in
  * the info object.
@@ -225,6 +89,7 @@ void * mca_mpool_base_alloc(size_t size, ompi_info_t * info)
             item != opal_list_get_end(&mca_mpool_base_modules);
             item = opal_list_get_next(item)) {
             current = ((mca_mpool_base_selected_module_t *) item);
+            if(current->mpool_module->flags & MCA_MPOOL_FLAGS_MPI_ALLOC_MEM) {
                 if(NULL == current->mpool_module->mpool_register){
                     no_reg_function = current;
                 }
@@ -233,6 +98,7 @@ void * mca_mpool_base_alloc(size_t size, ompi_info_t * info)
                 }
             }
         }
+    }
     else
     {
         ompi_info_get_nkeys(info, &num_keys);
@@ -316,7 +182,7 @@ void * mca_mpool_base_alloc(size_t size, ompi_info_t * info)
         num_modules++;
     }
     
-    while(i < reg_module_num && MCA_MPOOL_BASE_MAX_REG > num_modules)
+    while(i < reg_module_num)
     {
         mca_mpool_base_module_t* mpool = has_reg_function[i]->mpool_module;
         if(OMPI_SUCCESS != mpool->mpool_register(mpool, mem, size, MCA_MPOOL_FLAGS_PERSIST,  &registration))

ompi/mca/mpool/base/mpool_base_close.c

@@ -25,6 +25,8 @@
 #include "mca/mpool/base/base.h"
 #include "mpool_base_mem_cb.h"
 
+extern int mca_mpool_base_use_mem_hooks; 
+
 int mca_mpool_base_close(void)
 {
   opal_list_item_t *item;
@@ -55,9 +57,9 @@ int mca_mpool_base_close(void)
                             &mca_mpool_base_components, NULL);
 
   /* deregister memory free callback */
-  /* if(opal_mem_free_is_supported()) {  */
-/*       opal_mem_free_unregister_handler(mca_mpool_base_mem_cb);  */
-/*   } */
+  if(mca_mpool_base_use_mem_hooks && opal_mem_free_is_supported()) {
+      opal_mem_free_unregister_handler(mca_mpool_base_mem_cb);
+  }
   /* All done */
 
   return OMPI_SUCCESS;

ompi/mca/mpool/base/mpool_base_init.c

@@ -44,12 +44,6 @@ int mca_mpool_base_init(bool enable_progress_threads, bool enable_mpi_threads)
 {
     mca_mpool_enable_progress_threads = enable_progress_threads;
     mca_mpool_enable_mpi_threads = enable_mpi_threads;
-    OBJ_CONSTRUCT(&mca_mpool_base_mem_list, ompi_free_list_t);
-    ompi_free_list_init(&mca_mpool_base_mem_list, sizeof(mca_mpool_base_chunk_t),
-                        OBJ_CLASS(mca_mpool_base_chunk_t), 0, -1 , 128, NULL);
-    OBJ_CONSTRUCT(&mca_mpool_base_tree, ompi_rb_tree_t);
-    OBJ_CONSTRUCT(&mca_mpool_base_tree_lock, opal_mutex_t); 
-
-    return ompi_rb_tree_init(&mca_mpool_base_tree, mca_mpool_base_tree_node_compare);
+    return OMPI_SUCCESS;
 }
 

ompi/mca/mpool/base/mpool_base_lookup.c

@@ -25,6 +25,7 @@
 #include "mca/mpool/base/base.h"
 #include "mpool_base_mem_cb.h"
 
+extern int mca_mpool_base_use_mem_hooks;
 
 mca_mpool_base_component_t* mca_mpool_base_component_lookup(const char* name)
 {
@@ -79,10 +80,11 @@ mca_mpool_base_module_t* mca_mpool_base_module_create(
     sm->mpool_resources = resources;
     opal_list_append(&mca_mpool_base_modules, (opal_list_item_t*) sm); 
     /* on the very first creation of a module we init the memory callback*/ 
-    /* if(opal_list_get_size(&mca_mpool_base_modules) == 1 && */
-/*        opal_mem_free_is_supported()) {  */
-/*         opal_mem_free_register_handler(mca_mpool_base_mem_cb, NULL);  */
-/*     } */
+    if(mca_mpool_base_use_mem_hooks &&
+       opal_list_get_size(&mca_mpool_base_modules) == 1 &&
+       opal_mem_free_is_supported()) {
+        opal_mem_free_register_handler(mca_mpool_base_mem_cb, NULL);
+    }
     return module; 
 }
 

ompi/mca/mpool/base/mpool_base_open.c

@@ -41,6 +41,7 @@
  * Global variables
  */
 int mca_mpool_base_output = -1;
+int mca_mpool_base_use_mem_hooks = 0; 
 opal_list_t mca_mpool_base_components;
 opal_list_t mca_mpool_base_modules;
 
@@ -66,6 +67,14 @@ int mca_mpool_base_open(void)
 
   OBJ_CONSTRUCT(&mca_mpool_base_modules, opal_list_t);
   
+  mca_base_param_reg_int_name("mpool_base", 
+                              "use_mem_hooks", 
+                              "use memory hooks for deregistering freed memory",
+                              false, 
+                              false, 
+                              0,
+                              &mca_mpool_base_use_mem_hooks); 
+        
   /* All done */
 
   return OMPI_SUCCESS;

ompi/mca/mpool/gm/mpool_gm_module.c

@@ -40,6 +40,7 @@ void mca_mpool_gm_module_init(mca_mpool_gm_module_t* mpool)
     mpool->super.mpool_finalize = NULL; 
     mpool->super.rcache = 
         mca_rcache_base_module_create(mca_mpool_gm_component.rcache_name);
+    mpool->super.flags = MCA_MPOOL_FLAGS_MPI_ALLOC_MEM; 
 }
 
 

ompi/mca/mpool/mpool.h

@@ -26,6 +26,7 @@
 
 #define MCA_MPOOL_FLAGS_CACHE 0x1
 #define MCA_MPOOL_FLAGS_PERSIST 0x2 
+#define MCA_MPOOL_FLAGS_MPI_ALLOC_MEM 0x4
 
 struct mca_mpool_base_resources_t;
 
@@ -186,6 +187,7 @@ struct mca_mpool_base_module_t {
     mca_mpool_base_module_release_fn_t mpool_release; /**< release a regisrtation from the cache */ 
     mca_mpool_base_module_finalize_fn_t mpool_finalize;  /**< finalize */
     struct mca_rcache_base_module_t *rcache; /* the rcache associated with this mpool */ 
+    uint32_t flags; /**< mpool flags */
 };
 /**
  * Convenience typedef

ompi/mca/mpool/mvapi/mpool_mvapi_module.c

@@ -45,6 +45,7 @@ void mca_mpool_mvapi_module_init(mca_mpool_mvapi_module_t* mpool)
     mpool->super.mpool_finalize = NULL; 
     mpool->super.rcache = 
         mca_rcache_base_module_create(mca_mpool_mvapi_component.rcache_name);
+    mpool->super.flags = MCA_MPOOL_FLAGS_MPI_ALLOC_MEM; 
 }
 
 

ompi/mca/mpool/openib/mpool_openib_module.c

@@ -42,7 +42,7 @@ void mca_mpool_openib_module_init(mca_mpool_openib_module_t* mpool)
     mpool->super.mpool_finalize = NULL; 
     mpool->super.rcache = 
         mca_rcache_base_module_create(mca_mpool_openib_component.rcache_name);
-    
+    mpool->super.flags = MCA_MPOOL_FLAGS_MPI_ALLOC_MEM; 
 }
 
 

ompi/mca/mpool/sm/mpool_sm_module.c

@@ -34,7 +34,7 @@ void mca_mpool_sm_module_init(mca_mpool_sm_module_t* mpool)
   mpool->super.mpool_register = NULL; 
   mpool->super.mpool_deregister = NULL; 
   mpool->super.mpool_finalize = NULL; 
-
+  mpool->super.flags = 0;
 }
 
 /*