diff --git a/ompi/mpi/tool/mpit-internal.h b/ompi/mpi/tool/mpit-internal.h index 112802f3f0..557472743b 100644 --- a/ompi/mpi/tool/mpit-internal.h +++ b/ompi/mpi/tool/mpit-internal.h @@ -37,6 +37,7 @@ void mpit_unlock (void); extern volatile uint32_t mpit_init_count; int ompit_var_type_to_datatype (mca_base_var_type_t type, MPI_Datatype *datatype); +int ompit_opal_to_mpit_error (int rc); static inline int mpit_is_initialized (void) { diff --git a/ompi/mpi/tool/mpit_common.c b/ompi/mpi/tool/mpit_common.c index 296d20adcb..e492c3b595 100644 --- a/ompi/mpi/tool/mpit_common.c +++ b/ompi/mpi/tool/mpit_common.c @@ -82,3 +82,18 @@ int ompit_var_type_to_datatype (mca_base_var_type_t type, MPI_Datatype *datatype return OMPI_SUCCESS; } + +int ompit_opal_to_mpit_error (int rc) +{ + switch (rc) { + case OPAL_SUCCESS: + return MPI_SUCCESS; + case OPAL_ERR_OUT_OF_RESOURCE: + return MPI_T_ERR_MEMORY; + case OPAL_ERR_VALUE_OUT_OF_BOUNDS: + case OPAL_ERR_NOT_BOUND: + return MPI_T_ERR_INVALID_HANDLE; + default: + return MPI_ERR_UNKNOWN; + } +} diff --git a/ompi/mpi/tool/pvar_handle_alloc.c b/ompi/mpi/tool/pvar_handle_alloc.c index cffe3a54f1..0a9450affc 100644 --- a/ompi/mpi/tool/pvar_handle_alloc.c +++ b/ompi/mpi/tool/pvar_handle_alloc.c @@ -50,16 +50,9 @@ int MPI_T_pvar_handle_alloc(MPI_T_pvar_session session, int pvar_index, ret = mca_base_pvar_handle_alloc (session, pvar_index, obj_handle, handle, count); - if (OPAL_ERR_OUT_OF_RESOURCE == ret) { - ret = MPI_T_ERR_MEMORY; - } else if (OPAL_ERR_VALUE_OUT_OF_BOUNDS == ret) { - ret = MPI_T_ERR_INVALID_HANDLE; - } else if (OPAL_SUCCESS != ret) { - ret = MPI_ERR_UNKNOWN; - } } while (0); mpit_unlock (); - return ret; + return ompit_opal_to_mpit_error(ret); } diff --git a/ompi/mpi/tool/pvar_read.c b/ompi/mpi/tool/pvar_read.c index e5f1941b9e..67e13c234f 100644 --- a/ompi/mpi/tool/pvar_read.c +++ b/ompi/mpi/tool/pvar_read.c @@ -40,5 +40,5 @@ int MPI_T_pvar_read(MPI_T_pvar_session session, MPI_T_pvar_handle handle, mpit_unlock (); - return ret; + return ompit_opal_to_mpit_error (ret); } diff --git a/ompi/mpi/tool/pvar_reset.c b/ompi/mpi/tool/pvar_reset.c index b3ebe9e085..22b259e45d 100644 --- a/ompi/mpi/tool/pvar_reset.c +++ b/ompi/mpi/tool/pvar_reset.c @@ -46,5 +46,5 @@ int MPI_T_pvar_reset(MPI_T_pvar_session session, MPI_T_pvar_handle handle) mpit_unlock (); - return ret; + return ompit_opal_to_mpit_error (ret); } diff --git a/ompi/mpi/tool/pvar_session_create.c b/ompi/mpi/tool/pvar_session_create.c index fece47e492..9ae24b3117 100644 --- a/ompi/mpi/tool/pvar_session_create.c +++ b/ompi/mpi/tool/pvar_session_create.c @@ -31,13 +31,10 @@ int MPI_T_pvar_session_create(MPI_T_pvar_session *session) mpit_lock (); - do { - *session = OBJ_NEW(mca_base_pvar_session_t); - if (NULL == *session) { - ret = MPI_ERR_NO_MEM; - break; - } - } while (0); + *session = OBJ_NEW(mca_base_pvar_session_t); + if (NULL == *session) { + ret = MPI_ERR_NO_MEM; + } mpit_unlock (); diff --git a/ompi/mpi/tool/pvar_start.c b/ompi/mpi/tool/pvar_start.c index 8c45edb66b..901bde6f37 100644 --- a/ompi/mpi/tool/pvar_start.c +++ b/ompi/mpi/tool/pvar_start.c @@ -55,5 +55,5 @@ int MPI_T_pvar_start(MPI_T_pvar_session session, MPI_T_pvar_handle handle) mpit_unlock (); - return ret; + return ompit_opal_to_mpit_error (ret); } diff --git a/ompi/mpi/tool/pvar_stop.c b/ompi/mpi/tool/pvar_stop.c index 5f8ee1dddd..52cbaa6de8 100644 --- a/ompi/mpi/tool/pvar_stop.c +++ b/ompi/mpi/tool/pvar_stop.c @@ -57,5 +57,5 @@ int MPI_T_pvar_stop(MPI_T_pvar_session session, MPI_T_pvar_handle handle) mpit_unlock (); - return ret; + return ompit_opal_to_mpit_error (ret); } diff --git a/ompi/mpi/tool/pvar_write.c b/ompi/mpi/tool/pvar_write.c index 14ac6abf1f..9940a69534 100644 --- a/ompi/mpi/tool/pvar_write.c +++ b/ompi/mpi/tool/pvar_write.c @@ -40,5 +40,5 @@ int MPI_T_pvar_write(MPI_T_pvar_session session, MPI_T_pvar_handle handle, mpit_unlock (); - return ret; + return ompit_opal_to_mpit_error (ret); } diff --git a/opal/mca/base/mca_base_pvar.c b/opal/mca/base/mca_base_pvar.c index 5aeb9f5bd3..cfc8c813d3 100644 --- a/opal/mca/base/mca_base_pvar.c +++ b/opal/mca/base/mca_base_pvar.c @@ -463,26 +463,8 @@ int mca_base_pvar_handle_alloc (mca_base_pvar_session_t *session, int index, voi break; } - if (mca_base_pvar_is_sum (pvar)) { - /* for sums (counters, timers, etc) we need to keep track of - what the last value of the underlying counter was. this allows - us to push the computation of handle values from the event(s) - (which could be in a critical path) to pvar read/stop/reset/etc */ - pvar_handle->last_value = calloc (*count, datatype_size); - if (NULL == pvar_handle->last_value) { - ret = OPAL_ERR_OUT_OF_RESOURCE; - break; - } - } - if (!mca_base_pvar_is_continuous (pvar) || mca_base_pvar_is_sum (pvar) || mca_base_pvar_is_watermark (pvar)) { - pvar_handle->tmp_value = calloc (*count, datatype_size); - if (NULL == pvar_handle->tmp_value) { - ret = OPAL_ERR_OUT_OF_RESOURCE; - break; - } - /* if a variable is not continuous we will need to keep track of its last value to support start->stop->read correctly. use calloc to initialize the current value to 0. */ @@ -493,20 +475,37 @@ int mca_base_pvar_handle_alloc (mca_base_pvar_session_t *session, int index, voi } } - /* get the current value of the performance variable if this is a - continuous sum or watermark. if this variable needs to be started first the - current value is not relevant. */ - if (mca_base_pvar_is_continuous (pvar) && (mca_base_pvar_is_sum (pvar) || mca_base_pvar_is_sum (pvar))) { - if (mca_base_pvar_is_sum (pvar)) { - /* the initial value of a sum is 0 */ - ret = pvar->get_value (pvar, pvar_handle->last_value, pvar_handle->obj_handle); - } else { - /* the initial value of a watermark is the current value of the variable */ - ret = pvar->get_value (pvar, pvar_handle->current_value, pvar_handle->obj_handle); + if (mca_base_pvar_is_sum (pvar) || mca_base_pvar_is_watermark (pvar)) { + /* for sums (counters, timers, etc) we need to keep track of + what the last value of the underlying counter was. this allows + us to push the computation of handle values from the event(s) + (which could be in a critical path) to pvar read/stop/reset/etc */ + pvar_handle->tmp_value = calloc (*count, datatype_size); + if (NULL == pvar_handle->tmp_value) { + ret = OPAL_ERR_OUT_OF_RESOURCE; + break; + } + + pvar_handle->last_value = calloc (*count, datatype_size); + if (NULL == pvar_handle->last_value) { + ret = OPAL_ERR_OUT_OF_RESOURCE; + break; } - if (OPAL_SUCCESS != ret) { - return ret; + /* get the current value of the performance variable if this is a + continuous sum or watermark. if this variable needs to be started first the + current value is not relevant. */ + if (mca_base_pvar_is_continuous (pvar)) { + if (mca_base_pvar_is_sum (pvar)) { + ret = pvar->get_value (pvar, pvar_handle->last_value, pvar_handle->obj_handle); + } else { + /* the initial value of a watermark is the current value of the variable */ + ret = pvar->get_value (pvar, pvar_handle->current_value, pvar_handle->obj_handle); + } + + if (OPAL_SUCCESS != ret) { + return ret; + } } } @@ -533,14 +532,6 @@ int mca_base_pvar_handle_alloc (mca_base_pvar_session_t *session, int index, voi int mca_base_pvar_handle_free (mca_base_pvar_handle_t *handle) { - if (handle->session) { - opal_list_remove_item (&handle->session->handles, &handle->super); - } - - if (handle->pvar) { - opal_list_remove_item (&handle->pvar->bound_handles, &handle->list2); - } - OBJ_RELEASE(handle); return OPAL_SUCCESS; @@ -551,6 +542,10 @@ int mca_base_pvar_handle_update (mca_base_pvar_handle_t *handle) int i, ret; void *tmp; + if (mca_base_pvar_is_invalid (handle->pvar)) { + return OPAL_ERR_NOT_BOUND; + } + if (!mca_base_pvar_handle_is_running (handle)) { return OPAL_SUCCESS; } @@ -657,6 +652,10 @@ int mca_base_pvar_handle_read_value (mca_base_pvar_handle_t *handle, void *value { int ret; + if (mca_base_pvar_is_invalid (handle->pvar)) { + return OPAL_ERR_NOT_BOUND; + } + /* ensure this handle's value is up to date. */ ret = mca_base_pvar_handle_update (handle); if (OPAL_SUCCESS != ret) { @@ -679,6 +678,10 @@ int mca_base_pvar_handle_write_value (mca_base_pvar_handle_t *handle, const void { int ret; + if (mca_base_pvar_is_invalid (handle->pvar)) { + return OPAL_ERR_NOT_BOUND; + } + if (mca_base_pvar_is_readonly (handle->pvar)) { return OPAL_ERR_PERM; } @@ -735,6 +738,10 @@ int mca_base_pvar_handle_stop (mca_base_pvar_handle_t *handle) { int ret; + if (mca_base_pvar_is_invalid (handle->pvar)) { + return OPAL_ERR_NOT_BOUND; + } + /* Can't stop a continuous or an already stopped variable */ if (!mca_base_pvar_handle_is_running (handle) || mca_base_pvar_is_continuous (handle->pvar)) { return OPAL_ERR_NOT_SUPPORTED; @@ -758,6 +765,10 @@ int mca_base_pvar_handle_reset (mca_base_pvar_handle_t *handle) { int ret = OPAL_SUCCESS; + if (mca_base_pvar_is_invalid (handle->pvar)) { + return OPAL_ERR_NOT_BOUND; + } + /* reset this handle to a state analagous to when it was created */ if (mca_base_pvar_is_sum (handle->pvar)) { /* reset the running sum to 0 */ @@ -908,7 +919,8 @@ static void ompi_mpi_pvar_session_destructor (mca_base_pvar_session_t *session) mca_base_pvar_handle_t *handle, *next; /* it is likely a user error if there are any allocated handles when the session - is freed. clean it up anyway. */ + * is freed. clean it up anyway. The handle destructor will remove the handle from + * the session's handle list. */ OPAL_LIST_FOREACH_SAFE(handle, next, &session->handles, mca_base_pvar_handle_t) { OBJ_DESTRUCT(handle); } @@ -945,7 +957,15 @@ static void mca_base_pvar_handle_destructor (mca_base_pvar_handle_t *handle) free (handle->tmp_value); } + /* remove this handle from the pvar list */ + opal_list_remove_item (&handle->pvar->bound_handles, &handle->list2); + OBJ_DESTRUCT(&handle->list2); + + /* remove this handle from the session */ + if (handle->session) { + opal_list_remove_item (&handle->session->handles, &handle->super); + } } OBJ_CLASS_INSTANCE(mca_base_pvar_handle_t, opal_list_item_t, mca_base_pvar_handle_constructor, diff --git a/opal/mca/base/mca_base_pvar.h b/opal/mca/base/mca_base_pvar.h index a935228428..2a53763670 100644 --- a/opal/mca/base/mca_base_pvar.h +++ b/opal/mca/base/mca_base_pvar.h @@ -451,6 +451,11 @@ static inline bool mca_base_pvar_is_atomic (const mca_base_pvar_t *pvar) return !!(pvar->flags & MCA_BASE_PVAR_FLAG_ATOMIC); } +static inline bool mca_base_pvar_is_invalid (const mca_base_pvar_t *pvar) +{ + return !!(pvar->flags & MCA_BASE_PVAR_FLAG_INVALID); +} + /* Handle functions */ /**