From b8cb6e1c6449f8211c3633ba0d9d80e57fc3ef9a Mon Sep 17 00:00:00 2001 From: Galen Shipman Date: Fri, 16 Sep 2005 22:22:03 +0000 Subject: [PATCH] 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 | 51 ------- ompi/mca/mpool/base/mpool_base_alloc.c | 150 ++------------------ ompi/mca/mpool/base/mpool_base_close.c | 8 +- ompi/mca/mpool/base/mpool_base_init.c | 8 +- ompi/mca/mpool/base/mpool_base_lookup.c | 10 +- ompi/mca/mpool/base/mpool_base_open.c | 13 +- ompi/mca/mpool/gm/mpool_gm_module.c | 1 + ompi/mca/mpool/mpool.h | 2 + ompi/mca/mpool/mvapi/mpool_mvapi_module.c | 1 + ompi/mca/mpool/openib/mpool_openib_module.c | 2 +- ompi/mca/mpool/sm/mpool_sm_module.c | 2 +- 11 files changed, 37 insertions(+), 211 deletions(-) diff --git a/ompi/mca/mpool/base/base.h b/ompi/mca/mpool/base/base.h index 75d782dcb1..9e9df155ac 100644 --- a/ompi/mca/mpool/base/base.h +++ b/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) } diff --git a/ompi/mca/mpool/base/mpool_base_alloc.c b/ompi/mca/mpool/base/mpool_base_alloc.c index 3627fe050f..39afad21d1 100644 --- a/ompi/mca/mpool/base/mpool_base_alloc.c +++ b/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,11 +89,13 @@ 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(NULL == current->mpool_module->mpool_register){ - no_reg_function = current; - } - else { - has_reg_function[reg_module_num++] = current; + if(current->mpool_module->flags & MCA_MPOOL_FLAGS_MPI_ALLOC_MEM) { + if(NULL == current->mpool_module->mpool_register){ + no_reg_function = current; + } + else { + has_reg_function[reg_module_num++] = current; + } } } } @@ -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, ®istration)) diff --git a/ompi/mca/mpool/base/mpool_base_close.c b/ompi/mca/mpool/base/mpool_base_close.c index 44373fd573..f9166d84ca 100644 --- a/ompi/mca/mpool/base/mpool_base_close.c +++ b/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; diff --git a/ompi/mca/mpool/base/mpool_base_init.c b/ompi/mca/mpool/base/mpool_base_init.c index 7010b3fda0..5835c63158 100644 --- a/ompi/mca/mpool/base/mpool_base_init.c +++ b/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; } diff --git a/ompi/mca/mpool/base/mpool_base_lookup.c b/ompi/mca/mpool/base/mpool_base_lookup.c index f9810dd615..b981b8fef9 100644 --- a/ompi/mca/mpool/base/mpool_base_lookup.c +++ b/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; } diff --git a/ompi/mca/mpool/base/mpool_base_open.c b/ompi/mca/mpool/base/mpool_base_open.c index e16cedfc12..028c47de9c 100644 --- a/ompi/mca/mpool/base/mpool_base_open.c +++ b/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; @@ -59,13 +60,21 @@ int mca_mpool_base_open(void) &mca_mpool_base_components, true)) { return OMPI_ERROR; } - + /* Initialize the list so that in mca_mpool_base_close(), we can iterate over it (even if it's empty, as in the case of ompi_info) */ 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; diff --git a/ompi/mca/mpool/gm/mpool_gm_module.c b/ompi/mca/mpool/gm/mpool_gm_module.c index df1fc0a4c2..5976c6a558 100644 --- a/ompi/mca/mpool/gm/mpool_gm_module.c +++ b/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; } diff --git a/ompi/mca/mpool/mpool.h b/ompi/mca/mpool/mpool.h index 29415926ec..37827cb06c 100644 --- a/ompi/mca/mpool/mpool.h +++ b/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 diff --git a/ompi/mca/mpool/mvapi/mpool_mvapi_module.c b/ompi/mca/mpool/mvapi/mpool_mvapi_module.c index abed581d64..313fd20a24 100644 --- a/ompi/mca/mpool/mvapi/mpool_mvapi_module.c +++ b/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; } diff --git a/ompi/mca/mpool/openib/mpool_openib_module.c b/ompi/mca/mpool/openib/mpool_openib_module.c index 99291423f8..5ff67ad5fd 100644 --- a/ompi/mca/mpool/openib/mpool_openib_module.c +++ b/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; } diff --git a/ompi/mca/mpool/sm/mpool_sm_module.c b/ompi/mca/mpool/sm/mpool_sm_module.c index 0786da0faa..817d89bfd5 100644 --- a/ompi/mca/mpool/sm/mpool_sm_module.c +++ b/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; } /*