Revert "coll/basic: fix segmentation fault in neighborhood collectives if the degree"

This partially reverts commit open-mpi/ompi@76204dfafe.

ompi/mca/coll/basic/coll_basic_module.c

@@ -13,7 +13,7 @@
  * Copyright (c) 2012      Sandia National Laboratories. All rights reserved.
  * Copyright (c) 2013      Los Alamos National Security, LLC. All rights
  *                         reserved.
- * Copyright (c) 2014      Research Organization for Information Science
+ * Copyright (c) 2015      Research Organization for Information Science
  *                         and Technology (RIST). All rights reserved.
  * $COPYRIGHT$
  *
@@ -30,8 +30,6 @@
 #include "mpi.h"
 #include "ompi/mca/coll/coll.h"
 #include "ompi/mca/coll/base/base.h"
-#include "ompi/mca/topo/topo.h"
-#include "ompi/mca/topo/base/base.h"
 #include "coll_basic.h"
 
 
@@ -59,7 +57,6 @@ mca_coll_base_module_t *
 mca_coll_basic_comm_query(struct ompi_communicator_t *comm,
                           int *priority)
 {
-    int size;
     mca_coll_basic_module_t *basic_module;
 
     basic_module = OBJ_NEW(mca_coll_basic_module_t);
@@ -67,43 +64,6 @@ mca_coll_basic_comm_query(struct ompi_communicator_t *comm,
 
     *priority = mca_coll_basic_priority;
 
-    /* Allocate the data that hangs off the communicator */
-
-    if (OMPI_COMM_IS_INTER(comm)) {
-        size = ompi_comm_remote_size(comm);
-    } else {
-        size = ompi_comm_size(comm);
-    }
-    size *= 2;
-    if (OMPI_COMM_IS_CART(comm)) {
-        int cart_size;
-        mca_topo_base_comm_cart_2_2_0_t *cart;
-        assert (NULL != comm->c_topo);
-        cart = comm->c_topo->mtc.cart;
-        cart_size = cart->ndims * 4;
-        if (cart_size > size) {
-            size = cart_size;
-        }
-    } else if (OMPI_COMM_IS_GRAPH(comm)) {
-        int rank, degree;
-        assert (NULL != comm->c_topo);
-        rank = ompi_comm_rank (comm);
-        mca_topo_base_graph_neighbors_count (comm, rank, &degree);
-        degree *= 2;
-        if (degree > size) {
-            size = degree;
-        }
-    } else if (OMPI_COMM_IS_DIST_GRAPH(comm)) {
-        int dist_graph_size;
-        mca_topo_base_comm_dist_graph_2_2_0_t *dist_graph;
-        assert (NULL != comm->c_topo);
-        dist_graph = comm->c_topo->mtc.dist_graph;
-        dist_graph_size = dist_graph->indegree + dist_graph->outdegree;
-        if (dist_graph_size > size) {
-            size = dist_graph_size;
-        }
-    }
-
     /* Choose whether to use [intra|inter], and [linear|log]-based
      * algorithms. */
     basic_module->super.coll_module_enable = mca_coll_basic_module_enable;

ompi/mca/topo/base/topo_base_cart_create.c

@@ -160,12 +160,6 @@ int mca_topo_base_cart_create(mca_topo_base_module_t *topo,
         return MPI_ERR_INTERN;
     }
 
-    assert(NULL == new_comm->c_topo);
-    assert(!(new_comm->c_flags & OMPI_COMM_CART));
-    new_comm->c_topo           = topo;
-    new_comm->c_topo->mtc.cart = cart;
-    new_comm->c_topo->reorder  = reorder;
-    new_comm->c_flags         |= OMPI_COMM_CART;
     ret = ompi_comm_enable(old_comm, new_comm,
                            new_rank, num_procs, topo_procs);
     if (OMPI_SUCCESS != ret) {
@@ -180,6 +174,10 @@ int mca_topo_base_cart_create(mca_topo_base_module_t *topo,
         return ret;
     }
 
+    new_comm->c_topo           = topo;
+    new_comm->c_topo->mtc.cart = cart;
+    new_comm->c_topo->reorder  = reorder;
+    new_comm->c_flags         |= OMPI_COMM_CART;
     *comm_topo = new_comm;
 
     if( MPI_UNDEFINED == new_rank ) {

ompi/mca/topo/base/topo_base_dist_graph_create.c

@@ -288,84 +288,28 @@ int mca_topo_base_dist_graph_create(mca_topo_base_module_t* module,
 {
     int err;
 
-    ompi_proc_t **topo_procs = NULL;
-    int num_procs, ret, rank, i;
-    ompi_communicator_t *new_comm;
-    mca_topo_base_comm_dist_graph_2_2_0_t* topo;
-    num_procs = ompi_comm_size(comm_old);
-    num_procs = ompi_comm_size(comm_old);
-    rank = ompi_comm_rank(comm_old);
-    topo_procs = (ompi_proc_t**)malloc(num_procs * sizeof(ompi_proc_t *));
-    if(OMPI_GROUP_IS_DENSE(comm_old->c_local_group)) {
-        memcpy(topo_procs, 
-               comm_old->c_local_group->grp_proc_pointers,
-               num_procs * sizeof(ompi_proc_t *));
-    } else {
-        for(i = 0 ; i < num_procs; i++) {
-            topo_procs[i] = ompi_group_peer_lookup(comm_old->c_local_group,i);
-        }
-    }
-    new_comm = ompi_comm_allocate(num_procs, 0);
-    if (NULL == new_comm) {
-        free(topo_procs);
-        return OMPI_ERR_OUT_OF_RESOURCE;
-    }
-    err = mca_topo_base_dist_graph_distribute(module,
-                                              comm_old,
-                                              n, nodes,
-                                              degrees, targets,
-                                              weights,
-                                              &topo);
-    if( OMPI_SUCCESS != err ) {
-        free(topo_procs);
-        ompi_comm_free(newcomm);
+    if( OMPI_SUCCESS != (err = ompi_comm_create(comm_old,
+                                                comm_old->c_local_group,
+                                                newcomm)) ) {
+        OBJ_RELEASE(module);
         return err;
     }
 
-    rank = ompi_comm_rank(comm_old);
-    if(OMPI_GROUP_IS_DENSE(comm_old->c_local_group)) {
-        memcpy(topo_procs,
-               comm_old->c_local_group->grp_proc_pointers,
-               num_procs * sizeof(ompi_proc_t *));
-    } else {
-        for(i = 0 ; i < num_procs; i++) {
-            topo_procs[i] = ompi_group_peer_lookup(comm_old->c_local_group,i);
-        }
-    }
-    assert(NULL == new_comm->c_topo);
-    new_comm->c_topo             = module;
-    new_comm->c_topo->reorder    = reorder;
-    new_comm->c_flags           |= OMPI_COMM_DIST_GRAPH;
-    new_comm->c_topo->mtc.dist_graph = topo;
+    assert(NULL == (*newcomm)->c_topo);
+    (*newcomm)->c_topo             = module;
+    (*newcomm)->c_topo->reorder    = reorder;
+    (*newcomm)->c_flags           |= OMPI_COMM_DIST_GRAPH;
 
-    ret = ompi_comm_enable(comm_old, new_comm,
-                           rank, num_procs, topo_procs);
-    if (OMPI_SUCCESS != ret) {
-        if ( NULL != topo->in ) {
-            free(topo->in);
-        }
-        if ( NULL != topo->out ) {
-            free(topo->out);
-        }
-        if ( NULL != topo->inw ) {
-            free(topo->inw);
-        }
-        if ( NULL != topo->outw ) {
-            free(topo->outw);
-        }
-        if (MPI_COMM_NULL != new_comm) {
-            new_comm->c_topo->mtc.dist_graph = NULL;
-            new_comm->c_topo             = NULL;
-            new_comm->c_flags           &= ~OMPI_COMM_DIST_GRAPH;
-            ompi_comm_free (&new_comm);
-        }
-        free(topo);
-        free(topo_procs);
-        return ret;
+    err = mca_topo_base_dist_graph_distribute(module,
+                                              *newcomm, 
+                                              n, nodes,
+                                              degrees, targets,
+                                              weights,
+                                              &((*newcomm)->c_topo->mtc.dist_graph));
+    if( OMPI_SUCCESS != err ) {
+        ompi_comm_free(newcomm);
     }
-    *newcomm = new_comm;
-
-    return OMPI_SUCCESS;
+    return err;
 }
 
 static void mca_topo_base_comm_dist_graph_2_2_0_construct(mca_topo_base_comm_dist_graph_2_2_0_t * dist_graph) {

ompi/mca/topo/base/topo_base_graph_create.c

@@ -123,11 +123,6 @@ int mca_topo_base_graph_create(mca_topo_base_module_t *topo,
         return OMPI_ERR_OUT_OF_RESOURCE;
     }
 
-    new_comm->c_topo            = topo;
-    new_comm->c_topo->mtc.graph = graph;
-    new_comm->c_flags          |= OMPI_COMM_GRAPH;
-    new_comm->c_topo->reorder   = reorder;
-
     ret = ompi_comm_enable(old_comm, new_comm,
                            new_rank, num_procs, topo_procs);
     if (OMPI_SUCCESS != ret) {
@@ -140,7 +135,11 @@ int mca_topo_base_graph_create(mca_topo_base_module_t *topo,
         }
         return ret;
     }
-
+    
+    new_comm->c_topo            = topo;
+    new_comm->c_topo->mtc.graph = graph;
+    new_comm->c_flags          |= OMPI_COMM_GRAPH;
+    new_comm->c_topo->reorder   = reorder;
     *comm_topo = new_comm;
 
     if( MPI_UNDEFINED == new_rank ) {