Remove inter-dependencies between OSC modules.

The osc monitoring component needed to include other OSC components header in order to be able tu access communicator through the component specific ompi_osc_*_module_t structures. This commit remove the dependency, and resolve the issue open-mpi#4523. Extend the common monitoring API. * Now it's possible to translate from local rank to world rank from both the communicator and the group. * Remove useless hashtable as we directly use the w_group contained in window structure. Signed-off-by: Clement Foyer <clement.foyer@inria.fr>
clementFoyer · Jan 10, 2018 · 0f00945 · 0f00945
1 parent a4755b6
commit 0f00945
Show file tree

Hide file tree

Showing 6 changed files with 31 additions and 46 deletions.
diff --git a/ompi/mca/common/monitoring/common_monitoring.h b/ompi/mca/common/monitoring/common_monitoring.h
@@ -66,14 +66,14 @@ OMPI_DECLSPEC void mca_common_monitoring_record_osc(int world_rank, size_t data_
 /* Records COLL communications. */
 OMPI_DECLSPEC void mca_common_monitoring_record_coll(int world_rank, size_t data_size);
 
-/* Translate the rank from the given communicator of a process to its rank in MPI_COMM_RANK. */
-static inline int mca_common_monitoring_get_world_rank(int dst, struct ompi_communicator_t*comm,
-                                                       int*world_rank)
+/* Translate the rank from the given rank of a process to its rank in MPI_COMM_RANK. */
+static inline int mca_common_monitoring_get_world_rank_grp(int dest, ompi_group_t *group,
+                                                           int *world_rank)
 {
     opal_process_name_t tmp;
 
     /* find the processor of the destination */
-    ompi_proc_t *proc = ompi_group_get_proc_ptr(comm->c_remote_group, dst, true);
+    ompi_proc_t *proc = ompi_group_get_proc_ptr(group, dest, true);
     if( ompi_proc_is_sentinel(proc) ) {
         tmp = ompi_proc_sentinel_to_name((uintptr_t)proc);
     } else {
@@ -94,6 +94,13 @@ static inline int mca_common_monitoring_get_world_rank(int dst, struct ompi_comm
     return ret;
 }
 
+/* Translate the rank from the given communicator of a process to its rank in MPI_COMM_RANK. */
+static inline int mca_common_monitoring_get_world_rank(int dst, struct ompi_communicator_t*comm,
+                                                       int*world_rank)
+{
+    return mca_common_monitoring_get_world_rank_grp(dst, comm->c_remote_group, world_rank);
+}
+
 /* Return the current status of the monitoring system 0 if off or the
  * seperation between internal tags and external tags is disabled. Any
  * other positive value if the segregation between point-to-point and

diff --git a/ompi/mca/osc/monitoring/osc_monitoring_accumulate.h b/ompi/mca/osc/monitoring/osc_monitoring_accumulate.h
@@ -29,7 +29,7 @@
          * If this fails the destination is not part of my MPI_COM_WORLD \
          * Lookup its name in the rank hastable to get its MPI_COMM_WORLD rank \
          */                                                             \
-        if(OPAL_SUCCESS == mca_common_monitoring_get_world_rank(target_rank, ompi_osc_monitoring_## template ##_get_comm(win), &world_rank)) { \
+        if(OPAL_SUCCESS == mca_common_monitoring_get_world_rank_grp(target_rank, win->w_group, &world_rank)) { \
             size_t type_size;                                           \
             ompi_datatype_type_size(dt, &type_size);                    \
             mca_common_monitoring_record_osc(world_rank, type_size, SEND); \
@@ -56,7 +56,7 @@
          * If this fails the destination is not part of my MPI_COM_WORLD \
          * Lookup its name in the rank hastable to get its MPI_COMM_WORLD rank \
          */                                                             \
-        if(OPAL_SUCCESS == mca_common_monitoring_get_world_rank(target_rank, ompi_osc_monitoring_## template ##_get_comm(win), &world_rank)) { \
+        if(OPAL_SUCCESS == mca_common_monitoring_get_world_rank_grp(target_rank, win->w_group, &world_rank)) { \
             size_t type_size, data_size;                                \
             ompi_datatype_type_size(origin_datatype, &type_size);       \
             data_size = origin_count*type_size;                         \
@@ -88,7 +88,7 @@
          * If this fails the destination is not part of my MPI_COM_WORLD \
          * Lookup its name in the rank hastable to get its MPI_COMM_WORLD rank \
          */                                                             \
-        if(OPAL_SUCCESS == mca_common_monitoring_get_world_rank(target_rank, ompi_osc_monitoring_## template ##_get_comm(win), &world_rank)) { \
+        if(OPAL_SUCCESS == mca_common_monitoring_get_world_rank_grp(target_rank, win->w_group, &world_rank)) { \
             size_t type_size, data_size;                                \
             ompi_datatype_type_size(origin_datatype, &type_size);       \
             data_size = origin_count*type_size;                         \
@@ -116,7 +116,7 @@
          * If this fails the destination is not part of my MPI_COM_WORLD \
          * Lookup its name in the rank hastable to get its MPI_COMM_WORLD rank \
          */                                                             \
-        if(OPAL_SUCCESS == mca_common_monitoring_get_world_rank(target_rank, ompi_osc_monitoring_## template ##_get_comm(win), &world_rank)) { \
+        if(OPAL_SUCCESS == mca_common_monitoring_get_world_rank_grp(target_rank, win->w_group, &world_rank)) { \
             size_t type_size, data_size;                                \
             ompi_datatype_type_size(origin_datatype, &type_size);       \
             data_size = origin_count*type_size;                         \
@@ -140,7 +140,7 @@
          * If this fails the destination is not part of my MPI_COM_WORLD \
          * Lookup its name in the rank hastable to get its MPI_COMM_WORLD rank \
          */                                                             \
-        if(OPAL_SUCCESS == mca_common_monitoring_get_world_rank(target_rank, ompi_osc_monitoring_## template ##_get_comm(win), &world_rank)) { \
+        if(OPAL_SUCCESS == mca_common_monitoring_get_world_rank_grp(target_rank, win->w_group, &world_rank)) { \
             size_t type_size, data_size;                                \
             ompi_datatype_type_size(origin_datatype, &type_size);       \
             data_size = origin_count*type_size;                         \
@@ -162,7 +162,7 @@
          * If this fails the destination is not part of my MPI_COM_WORLD \
          * Lookup its name in the rank hastable to get its MPI_COMM_WORLD rank \
          */                                                             \
-        if(OPAL_SUCCESS == mca_common_monitoring_get_world_rank(target_rank, ompi_osc_monitoring_## template ##_get_comm(win), &world_rank)) { \
+        if(OPAL_SUCCESS == mca_common_monitoring_get_world_rank_grp(target_rank, win->w_group, &world_rank)) { \
             size_t type_size;                                           \
             ompi_datatype_type_size(dt, &type_size);                    \
             mca_common_monitoring_record_osc(world_rank, type_size, SEND); \

diff --git a/ompi/mca/osc/monitoring/osc_monitoring_comm.h b/ompi/mca/osc/monitoring/osc_monitoring_comm.h
@@ -30,7 +30,7 @@
          * If this fails the destination is not part of my MPI_COM_WORLD \
          * Lookup its name in the rank hastable to get its MPI_COMM_WORLD rank \
          */                                                             \
-        if(OPAL_SUCCESS == mca_common_monitoring_get_world_rank(target_rank, ompi_osc_monitoring_## template ##_get_comm(win), &world_rank)) { \
+        if(OPAL_SUCCESS == mca_common_monitoring_get_world_rank_grp(target_rank, win->w_group, &world_rank)) { \
             size_t type_size, data_size;                                \
             ompi_datatype_type_size(origin_datatype, &type_size);       \
             data_size = origin_count*type_size;                         \
@@ -55,7 +55,7 @@
          * If this fails the destination is not part of my MPI_COM_WORLD \
          * Lookup its name in the rank hastable to get its MPI_COMM_WORLD rank \
          */                                                             \
-        if(OPAL_SUCCESS == mca_common_monitoring_get_world_rank(target_rank, ompi_osc_monitoring_## template ##_get_comm(win), &world_rank)) { \
+        if(OPAL_SUCCESS == mca_common_monitoring_get_world_rank_grp(target_rank, win->w_group, &world_rank)) { \
             size_t type_size, data_size;                                \
             ompi_datatype_type_size(origin_datatype, &type_size);       \
             data_size = origin_count*type_size;                         \
@@ -78,7 +78,7 @@
          * If this fails the destination is not part of my MPI_COM_WORLD \
          * Lookup its name in the rank hastable to get its MPI_COMM_WORLD rank \
          */                                                             \
-        if(OPAL_SUCCESS == mca_common_monitoring_get_world_rank(source_rank, ompi_osc_monitoring_## template ##_get_comm(win), &world_rank)) { \
+        if(OPAL_SUCCESS == mca_common_monitoring_get_world_rank_grp(source_rank, win->w_group, &world_rank)) { \
             size_t type_size, data_size;                                \
             ompi_datatype_type_size(origin_datatype, &type_size);       \
             data_size = origin_count*type_size;                         \
@@ -103,7 +103,7 @@
          * If this fails the destination is not part of my MPI_COM_WORLD \
          * Lookup its name in the rank hastable to get its MPI_COMM_WORLD rank \
          */                                                             \
-        if(OPAL_SUCCESS == mca_common_monitoring_get_world_rank(source_rank, ompi_osc_monitoring_## template ##_get_comm(win), &world_rank)) { \
+        if(OPAL_SUCCESS == mca_common_monitoring_get_world_rank_grp(source_rank, win->w_group, &world_rank)) { \
             size_t type_size, data_size;                                \
             ompi_datatype_type_size(origin_datatype, &type_size);       \
             data_size = origin_count*type_size;                         \

diff --git a/ompi/mca/osc/monitoring/osc_monitoring_component.c b/ompi/mca/osc/monitoring/osc_monitoring_component.c
@@ -8,6 +8,7 @@
  */
 
 #include <ompi_config.h>
+#include <string.h>
 #include "osc_monitoring.h"
 #include <ompi/constants.h>
 #include <ompi/communicator/communicator.h>
@@ -22,23 +23,10 @@
 /* Include template generating macros */
 #include "osc_monitoring_template.h"
 
-#include <ompi/mca/osc/rdma/osc_rdma.h>
-OSC_MONITORING_MODULE_TEMPLATE_GENERATE(rdma, ompi_osc_rdma_module_t, comm)
-#undef GET_MODULE
-
-#include <ompi/mca/osc/sm/osc_sm.h>
-OSC_MONITORING_MODULE_TEMPLATE_GENERATE(sm, ompi_osc_sm_module_t, comm)
-#undef GET_MODULE
-
-#include <ompi/mca/osc/pt2pt/osc_pt2pt.h>
-OSC_MONITORING_MODULE_TEMPLATE_GENERATE(pt2pt, ompi_osc_pt2pt_module_t, comm)
-#undef GET_MODULE
-
-#ifdef OMPI_WITH_OSC_PORTALS4
-#include <ompi/mca/osc/portals4/osc_portals4.h>
-OSC_MONITORING_MODULE_TEMPLATE_GENERATE(portals4, ompi_osc_portals4_module_t, comm)
-#undef GET_MODULE
-#endif /* OMPI_WITH_OSC_PORTALS4 */
+OSC_MONITORING_MODULE_TEMPLATE_GENERATE(rdma)
+OSC_MONITORING_MODULE_TEMPLATE_GENERATE(sm)
+OSC_MONITORING_MODULE_TEMPLATE_GENERATE(pt2pt)
+OSC_MONITORING_MODULE_TEMPLATE_GENERATE(portals4)
 
 /***************************************/
 

diff --git a/ompi/mca/osc/monitoring/osc_monitoring_module.h b/ompi/mca/osc/monitoring/osc_monitoring_module.h
@@ -40,10 +40,10 @@
 
 /* Define and set the ompi_osc_monitoring_## template ##_template
  * variable. The functions recorded here are linked to the original
- * functions of the original {template} module that were replaced.
+ * functions of the original {template} module that was replaced.
  */
 #define MCA_OSC_MONITORING_MODULE_TEMPLATE_GENERATE(template)           \
-    static ompi_osc_base_module_t OMPI_OSC_MONITORING_TEMPLATE_VARIABLE(template) = { \
+    static const ompi_osc_base_module_t OMPI_OSC_MONITORING_TEMPLATE_VARIABLE(template) = { \
         .osc_win_attach = ompi_osc_monitoring_## template ##_attach,    \
         .osc_win_detach = ompi_osc_monitoring_## template ##_detach,    \
         .osc_free = ompi_osc_monitoring_## template ##_free,            \

diff --git a/ompi/mca/osc/monitoring/osc_monitoring_template.h b/ompi/mca/osc/monitoring/osc_monitoring_template.h
@@ -23,10 +23,7 @@
 #include "osc_monitoring_module.h"
 #include "osc_monitoring_passive_target.h"
 
-/* module_type correspond to the ompi_osc_## template ##_module_t type 
- * comm correspond to the comm field name in ompi_osc_## template ##_module_t
- * 
- * The magic used here is that for a given module type (given with the
+/* The magic used here is that for a given module type (given with the
  * {template} parameter), we generate a set of every functions defined
  * in ompi_osc_base_module_t, the ompi_osc_monitoring_module_##
  * template ##_template variable recording the original set of
@@ -37,17 +34,11 @@
  * the original function that had been saved in the
  * ompi_osc_monitoring_module_## template ##_template variable.
  */
-#define OSC_MONITORING_MODULE_TEMPLATE_GENERATE(template, module_type, comm) \
+#define OSC_MONITORING_MODULE_TEMPLATE_GENERATE(template)               \
     /* Generate the proper symbol for the                               \
        ompi_osc_monitoring_module_## template ##_template variable */   \
     OMPI_OSC_MONITORING_MODULE_GENERATE(template);                      \
-    OMPI_OSC_MONITORING_MODULE_INIT_GENERATE(template);			\
-    /* Generate module specific module->comm accessor */                \
-    static inline struct ompi_communicator_t*                           \
-    ompi_osc_monitoring_## template ##_get_comm(ompi_win_t*win)         \
-    {                                                                   \
-        return ((module_type*)win->w_osc_module)->comm;                 \
-    }                                                                   \
+    OMPI_OSC_MONITORING_MODULE_INIT_GENERATE(template);                 \
     /* Generate each module specific functions */                       \
     OSC_MONITORING_GENERATE_TEMPLATE_ACCUMULATE(template)               \
     OSC_MONITORING_GENERATE_TEMPLATE_ACTIVE_TARGET(template)            \
@@ -78,4 +69,3 @@
     ompi_osc_monitoring_## template ##_set_template(module)
 
 #endif /* MCA_OSC_MONITORING_TEMPLATE_H */
-