Merge pull request #6683 from bosilca/topic/spc_init

Fixed SPC/MPI_T initialization error.
2019-05-30 17:50:58 -04:00 · 2019-05-30 17:50:58 -04:00 · e3eb6b5897
--- a/ompi/mpi/man/man3/MPI_T_cvar_handle_alloc.3in
+++ b/ompi/mpi/man/man3/MPI_T_cvar_handle_alloc.3in
@ -20,6 +20,7 @@ int MPI_T_cvar_handle_alloc(int \fIcvar_index\fP, void *\fIobj_handle\fP,

 int MPI_T_cvar_handle_free(MPI_T_cvar_handle *\fIhandle\fP)

+.fi
 .SH DESCRIPTION
 .ft R
 MPI_T_cvar_handle_alloc binds the control variable specified in \fIcvar_index\fP to the MPI
--- a/ompi/mpi/man/man3/MPI_T_finalize.3in
+++ b/ompi/mpi/man/man3/MPI_T_finalize.3in
@ -15,6 +15,7 @@
 #include <mpi.h>
 int MPI_T_finalize(void)

+.fi
 .SH DESCRIPTION
 .ft R
 MPI_T_finalize() finalizes the MPI tool information interface and must be called the same
--- a/ompi/mpi/man/man3/MPI_T_pvar_handle_alloc.3in
+++ b/ompi/mpi/man/man3/MPI_T_pvar_handle_alloc.3in
@ -15,17 +15,28 @@
 .SH C Syntax
 .nf
 #include <mpi.h>
-int MPI_T_pvar_handle_alloc(int \fIpvar_index\fP, void *\fIobj_handle\fP,
+int MPI_T_pvar_handle_alloc(int \fIsession\fP, int \fIpvar_index\fP, void *\fIobj_handle\fP,
                            MPI_T_pvar_handle *\fIhandle\fP, int *\fIcount\fP)

-int MPI_T_pvar_handle_free(MPI_T_pvar_handle *\fIhandle\fP)
+int MPI_T_pvar_handle_free(int \fIsession\fP, MPI_T_pvar_handle *\fIhandle\fP)

 .SH DESCRIPTION
 .ft R
 MPI_T_pvar_handle_alloc binds the performance variable specified in \fIpvar_index\fP to the MPI
-object specified in \fIobj_handle\fP. If MPI_T_pvar_get_info returns MPI_T_BIND_NO_OBJECT
-as the binding for the variable the \fIobj_handle\fP argument is ignored. The number of
-values represented by this performance variable is returned in the \fIcount\fP parameter.
+object specified in \fIobj_handle\fP in the session identified by the parameter
+\fIsession\fP. The object is passed in the argument \fIobj_handle\fP as an
+address to a local variable that stores the object’s handle. If
+MPI_T_pvar_get_info returns MPI_T_BIND_NO_OBJECT as the binding
+for the variable the \fIobj_handle\fP argument is ignored. The handle
+allocated to reference the variable is returned in the argument \fIhandle\fP. Upon successful
+return, \fIcount\fP contains the number of elements (of the datatype returned by a previous
+MPI_T_PVAR_GET_INFO call) used to represent this variable.
+
+The value of \fIpvar_index\fP should be in the range 0 to \fInum_pvar - 1\fP,
+where \fInum_pvar\fP is the number of available performance variables as
+determined from a prior call to \fIMPI_T_PVAR_GET_NUM\fP. The type of the
+MPI object it references must be consistent with the type returned in the
+bind argument in a prior call to \fIMPI_T_PVAR_GET_INFO\fP.

 MPI_T_pvar_handle_free frees a handle allocated by MPI_T_pvar_handle_alloc and sets the
 \fIhandle\fP argument to MPI_T_PVAR_HANDLE_NULL.
@ -50,11 +61,12 @@ MPI_T_pvar_handle_free() will fail if:
 The MPI Tools interface not initialized
 .TP 1i
 [MPI_T_ERR_INVALID_HANDLE]
-The handle is invalid
+The handle is invalid or the handle argument passed in is not associated with the session argument


 .SH SEE ALSO
 .ft R
 .nf
 MPI_T_pvar_get_info
+MPI_T_pvar_get_num

--- a/ompi/mpi/man/man3/MPI_T_pvar_session_create.3in
+++ b/ompi/mpi/man/man3/MPI_T_pvar_session_create.3in
@ -19,6 +19,7 @@ int MPI_T_pvar_session_create(MPI_T_pvar_session *\fIsession\fP)

 int MPI_T_pvar_session_free(MPI_T_pvar_session *\fIsession\fP)

+.fi
 .SH DESCRIPTION
 .ft R
 MPI_T_pvar_session_create creates a session for accessing performance variables. The
--- a/ompi/mpi/man/man3/MPI_T_pvar_start.3in
+++ b/ompi/mpi/man/man3/MPI_T_pvar_start.3in
@ -19,6 +19,7 @@ int MPI_T_pvar_start(MPI_T_pvar_session \fIsession\fP, MPI_T_pvar_handle \fIhand

 int MPI_T_pvar_stop(MPI_T_pvar_session \fIsession\fP, MPI_T_pvar_handle \fIhandle\fP)

+.fi
 .SH INPUT PARAMETERS
 .ft R
 .TP 1i
--- a/ompi/mpi/man/man3/MPI_T_pvar_write.3in
+++ b/ompi/mpi/man/man3/MPI_T_pvar_write.3in
@ -33,7 +33,7 @@ Initial address of storage location for variable value.
 .SH DESCRIPTION
 .ft R
 MPI_T_pvar_write attempts to set the value of the performance variable identified by
-the handle specified in \fIhandle\fP in the session specified in \fPsession\fI. The
+the handle specified in \fIhandle\fP in the session specified in \fIsession\fP. The
 value to be written is specified in \fIbuf\fP. The caller must ensure that the buffer
 specified in \fIbuf\fP is large enough to hold the entire value of the performance
 variable.
--- a/ompi/runtime/ompi_spc.c
+++ b/ompi/runtime/ompi_spc.c
@ -1,11 +1,13 @@
 /*
- * Copyright (c) 2018      The University of Tennessee and The University
+ * Copyright (c) 2018-2019 The University of Tennessee and The University
 *                         of Tennessee Research Foundation.  All rights
 *                         reserved.
 *
 * Copyright (c) 2018      Cisco Systems, Inc.  All rights reserved
 * Copyright (c) 2018      Research Organization for Information Science
 *                         and Technology (RIST).  All rights reserved.
+ * Copyright (c) 2019      Mellanox Technologies, Inc.
+ *                         All rights reserved.
 * $COPYRIGHT$
 *
 * Additional copyrights may follow
@ -20,10 +22,8 @@ opal_timer_t sys_clock_freq_mhz = 0;
 static void ompi_spc_dump(void);

 /* Array for converting from SPC indices to MPI_T indices */
-OMPI_DECLSPEC int mpi_t_offset = -1;
-OMPI_DECLSPEC bool mpi_t_enabled = false;
-
-OPAL_DECLSPEC ompi_communicator_t *ompi_spc_comm = NULL;
+static bool mpi_t_enabled = false;
+static ompi_communicator_t *ompi_spc_comm = NULL;

 typedef struct ompi_spc_event_t {
    const char* counter_name;
@ -185,6 +185,8 @@ static int ompi_spc_notify(mca_base_pvar_t *pvar, mca_base_pvar_event_t event, v
        return MPI_SUCCESS;
    }

+    index = (int)(uintptr_t)pvar->ctx;  /* Convert from MPI_T pvar index to SPC index */
+
    /* For this event, we need to set count to the number of long long type
     * values for this counter.  All SPC counters are one long long, so we
     * always set count to 1.
@ -194,14 +196,10 @@ static int ompi_spc_notify(mca_base_pvar_t *pvar, mca_base_pvar_event_t event, v
    }
    /* For this event, we need to turn on the counter */
    else if(MCA_BASE_PVAR_HANDLE_START == event) {
-        /* Convert from MPI_T pvar index to SPC index */
-        index = pvar->pvar_index - mpi_t_offset;
        SET_SPC_BIT(ompi_spc_attached_event, index);
    }
    /* For this event, we need to turn off the counter */
    else if(MCA_BASE_PVAR_HANDLE_STOP == event) {
-        /* Convert from MPI_T pvar index to SPC index */
-        index = pvar->pvar_index - mpi_t_offset;
        CLEAR_SPC_BIT(ompi_spc_attached_event, index);
    }

@ -231,7 +229,7 @@ static int ompi_spc_get_count(const struct mca_base_pvar_t *pvar, void *value, v
    }

    /* Convert from MPI_T pvar index to SPC index */
-    int index = pvar->pvar_index - mpi_t_offset;
+    int index = (int)(uintptr_t)pvar->ctx;
    /* Set the counter value to the current SPC value */
    *counter_value = (long long)ompi_spc_events[index].value;
    /* If this is a timer-based counter, convert from cycles to microseconds */
@ -276,7 +274,7 @@ void ompi_spc_events_init(void)
 */
 void ompi_spc_init(void)
 {
-    int i, j, ret, found = 0, all_on = 0;
+    int i, j, ret, found = 0, all_on = 0, matched = 0;

    /* Initialize the clock frequency variable as the CPU's frequency in MHz */
    sys_clock_freq_mhz = opal_timer_base_get_freq() / 1000000;
@ -296,52 +294,43 @@ void ompi_spc_init(void)
        }
    }

-    /* Turn on only the counters that were specified in the MCA parameter */
    for(i = 0; i < OMPI_SPC_NUM_COUNTERS; i++) {
-        if(all_on) {
-            SET_SPC_BIT(ompi_spc_attached_event, i);
-            mpi_t_enabled = true;
-            found++;
-        } else {
-            /* Note: If no arguments were given, this will be skipped */
+        /* Reset all timer-based counters */
+        CLEAR_SPC_BIT(ompi_spc_timer_event, i);
+        matched = all_on;
+
+        if( !matched ) {
+            /* Turn on only the counters that were specified in the MCA parameter */
            for(j = 0; j < num_args; j++) {
                if( 0 == strcmp(ompi_spc_events_names[i].counter_name, arg_strings[j]) ) {
-                    SET_SPC_BIT(ompi_spc_attached_event, i);
-                    mpi_t_enabled = true;
-                    found++;
+                    matched = 1;
                    break;
                }
            }
        }

-        /* ########################################################################
-         * ################## Add Timer-Based Counter Enums Here ##################
-         * ########################################################################
-         */
-        CLEAR_SPC_BIT(ompi_spc_timer_event, i);
+        if (matched) {
+            SET_SPC_BIT(ompi_spc_attached_event, i);
+            mpi_t_enabled = true;
+            found++;
+        }

        /* Registers the current counter as an MPI_T pvar regardless of whether it's been turned on or not */
        ret = mca_base_pvar_register("ompi", "runtime", "spc", ompi_spc_events_names[i].counter_name, ompi_spc_events_names[i].counter_description,
                                     OPAL_INFO_LVL_4, MPI_T_PVAR_CLASS_SIZE,
                                     MCA_BASE_VAR_TYPE_UNSIGNED_LONG_LONG, NULL, MPI_T_BIND_NO_OBJECT,
                                     MCA_BASE_PVAR_FLAG_READONLY | MCA_BASE_PVAR_FLAG_CONTINUOUS,
-                                     ompi_spc_get_count, NULL, ompi_spc_notify, NULL);
-
-        /* Check to make sure that ret is a valid index and not an error code.
-         */
-        if( ret >= 0 ) {
-            if( mpi_t_offset == -1 ) {
-                mpi_t_offset = ret;
-            }
-        }
-        if( (ret < 0) || (ret != (mpi_t_offset + found - 1)) ) {
+                                     ompi_spc_get_count, NULL, ompi_spc_notify, (void*)(uintptr_t)i);
+        if( ret < 0 ) {
            mpi_t_enabled = false;
            opal_show_help("help-mpi-runtime.txt", "spc: MPI_T disabled", true);
            break;
        }
    }
-    /* If this is a timer event, sent the corresponding timer_event entry to 1 */
+
+    /* If this is a timer event, set the corresponding timer_event entry */
    SET_SPC_BIT(ompi_spc_timer_event, OMPI_SPC_MATCH_TIME);
+
    opal_argv_free(arg_strings);
 }

--- a/test/spc/spc_test.c
+++ b/test/spc/spc_test.c
@ -44,8 +44,8 @@ int main(int argc, char **argv)
    char name[256], description[256];

    /* Counter names to be read by ranks 0 and 1 */
-    char *counter_names[] = { "runtime_spc_OMPI_BYTES_SENT_USER",
-                              "runtime_spc_OMPI_BYTES_RECEIVED_USER" };
+    char *counter_names[] = { "runtime_spc_OMPI_SPC_BYTES_SENT_USER",
+                              "runtime_spc_OMPI_SPC_BYTES_RECEIVED_USER" };

    MPI_Init(NULL, NULL);
    MPI_result = MPI_T_init_thread(MPI_THREAD_SINGLE, &provided);