Skip to content

Commit e39b9b9

Browse files
bosilcaderbeyn
authored andcommitted
Consistently use the request array for all modules (single array stored
in the base). Correctly deal with persistent requests (they must be always freed when they are stored in the request array associated with the communicator). Always use MPI_STATUS_IGNORE for single request waiting functions. (cherry picked from commit 88492a1)
1 parent 7bd8940 commit e39b9b9

23 files changed

+140
-185
lines changed

ompi/mca/coll/base/base.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
* These functions are normally invoked by the back-ends of:
2424
*
2525
* - The back-ends of MPI_Init() and MPI_Finalize()
26-
* - Communuicactor constructors (e.g., MPI_Comm_split()) and
26+
* - Communicator constructors (e.g., MPI_Comm_split()) and
2727
* destructors (e.g., MPI_Comm_free())
2828
* - The laminfo command
2929
*/

ompi/mca/coll/base/coll_base_alltoall.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -652,9 +652,9 @@ int ompi_coll_base_alltoall_intra_basic_linear(const void *sbuf, int scount,
652652
if( MPI_SUCCESS != err ) {
653653
OPAL_OUTPUT( (ompi_coll_base_framework.framework_output,"%s:%4d\tError occurred %d, rank %2d",
654654
__FILE__, line, err, rank) );
655-
/* Free the reqs */
656-
ompi_coll_base_free_reqs(req, nreqs);
657655
}
656+
/* Free the reqs in all cases as they are persistent requests */
657+
ompi_coll_base_free_reqs(req, nreqs);
658658

659659
/* All done */
660660
return err;

ompi/mca/coll/base/coll_base_alltoallv.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -288,9 +288,8 @@ ompi_coll_base_alltoallv_intra_basic_linear(const void *sbuf, const int *scounts
288288
err = ompi_request_wait_all(nreqs, data->mcct_reqs,
289289
MPI_STATUSES_IGNORE);
290290
err_hndl:
291-
if( MPI_SUCCESS != err ) { /* Free the requests. */
292-
ompi_coll_base_free_reqs(data->mcct_reqs, nreqs);
293-
}
291+
/* Free the requests in all cases as they are persistent */
292+
ompi_coll_base_free_reqs(data->mcct_reqs, nreqs);
294293

295294
return err;
296295
}

ompi/mca/coll/base/coll_base_bcast.c

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ ompi_coll_base_bcast_intra_generic( void* buffer,
143143

144144
/* wait for and forward the previous segment to children */
145145
err = ompi_request_wait( &recv_reqs[req_index ^ 0x1],
146-
MPI_STATUSES_IGNORE );
146+
MPI_STATUS_IGNORE );
147147
if (err != MPI_SUCCESS) { line = __LINE__; goto error_hndl; }
148148

149149
for( i = 0; i < tree->tree_nextsize; i++ ) {
@@ -175,7 +175,7 @@ ompi_coll_base_bcast_intra_generic( void* buffer,
175175
}
176176

177177
/* Process the last segment */
178-
err = ompi_request_wait( &recv_reqs[req_index], MPI_STATUSES_IGNORE );
178+
err = ompi_request_wait( &recv_reqs[req_index], MPI_STATUS_IGNORE );
179179
if (err != MPI_SUCCESS) { line = __LINE__; goto error_hndl; }
180180
sendcount = original_count - (ptrdiff_t)(num_segments - 1) * count_by_segment;
181181
for( i = 0; i < tree->tree_nextsize; i++ ) {
@@ -240,8 +240,11 @@ ompi_coll_base_bcast_intra_generic( void* buffer,
240240
error_hndl:
241241
OPAL_OUTPUT( (ompi_coll_base_framework.framework_output,"%s:%4d\tError occurred %d, rank %2d",
242242
__FILE__, line, err, rank) );
243-
if( (MPI_SUCCESS != err) && (NULL != send_reqs) ) {
244-
ompi_coll_base_free_reqs( send_reqs, tree->tree_nextsize);
243+
if( MPI_SUCCESS != err ) {
244+
ompi_coll_base_free_reqs( recv_reqs, 2);
245+
if( NULL != send_reqs ) {
246+
ompi_coll_base_free_reqs( send_reqs, tree->tree_nextsize);
247+
}
245248
}
246249

247250
return err;
@@ -378,7 +381,6 @@ ompi_coll_base_bcast_intra_split_bintree ( void* buffer,
378381
ptrdiff_t type_extent, lb;
379382
ompi_request_t *base_req, *new_req;
380383
ompi_coll_tree_t *tree;
381-
mca_coll_base_comm_t *data = module->base_data;
382384

383385
size = ompi_comm_size(comm);
384386
rank = ompi_comm_rank(comm);
@@ -391,7 +393,7 @@ ompi_coll_base_bcast_intra_split_bintree ( void* buffer,
391393

392394
/* setup the binary tree topology. */
393395
COLL_BASE_UPDATE_BINTREE( comm, module, root );
394-
tree = data->cached_bintree;
396+
tree = module->base_data->cached_bintree;
395397

396398
err = ompi_datatype_type_size( datatype, &type_size );
397399

@@ -501,8 +503,8 @@ ompi_coll_base_bcast_intra_split_bintree ( void* buffer,
501503
comm, &new_req));
502504
if (err != MPI_SUCCESS) { line = __LINE__; goto error_hndl; }
503505

504-
/* wait for and forward current segment */
505-
err = ompi_request_wait_all( 1, &base_req, MPI_STATUSES_IGNORE );
506+
/* wait for and forward the previous segment */
507+
err = ompi_request_wait( &base_req, MPI_STATUS_IGNORE );
506508
for( i = 0; i < tree->tree_nextsize; i++ ) { /* send data to children (segcount[lr]) */
507509
err = MCA_PML_CALL(send( tmpbuf[lr], segcount[lr], datatype,
508510
tree->tree_next[i], MCA_COLL_BASE_TAG_BCAST,
@@ -517,7 +519,7 @@ ompi_coll_base_bcast_intra_split_bintree ( void* buffer,
517519
} /* end of for segindex */
518520

519521
/* wait for the last segment and forward current segment */
520-
err = ompi_request_wait_all( 1, &base_req, MPI_STATUSES_IGNORE );
522+
err = ompi_request_wait( &base_req, MPI_STATUS_IGNORE );
521523
for( i = 0; i < tree->tree_nextsize; i++ ) { /* send data to children */
522524
err = MCA_PML_CALL(send(tmpbuf[lr], sendcount[lr], datatype,
523525
tree->tree_next[i], MCA_COLL_BASE_TAG_BCAST,
@@ -633,10 +635,8 @@ ompi_coll_base_bcast_intra_basic_linear(void *buff, int count,
633635
mca_coll_base_module_t *module)
634636
{
635637
int i, size, rank, err;
636-
mca_coll_base_comm_t *data = module->base_data;
637638
ompi_request_t **preq, **reqs;
638639

639-
640640
size = ompi_comm_size(comm);
641641
rank = ompi_comm_rank(comm);
642642

@@ -651,24 +651,20 @@ ompi_coll_base_bcast_intra_basic_linear(void *buff, int count,
651651
}
652652

653653
/* Root sends data to all others. */
654-
preq = reqs = coll_base_comm_get_reqs(data, size-1);
654+
preq = reqs = coll_base_comm_get_reqs(module->base_data, size-1);
655655
for (i = 0; i < size; ++i) {
656656
if (i == rank) {
657657
continue;
658658
}
659659

660-
err = MCA_PML_CALL(isend_init(buff, count, datatype, i,
661-
MCA_COLL_BASE_TAG_BCAST,
662-
MCA_PML_BASE_SEND_STANDARD,
663-
comm, preq++));
660+
err = MCA_PML_CALL(isend(buff, count, datatype, i,
661+
MCA_COLL_BASE_TAG_BCAST,
662+
MCA_PML_BASE_SEND_STANDARD,
663+
comm, preq++));
664664
if (MPI_SUCCESS != err) { goto err_hndl; }
665665
}
666666
--i;
667667

668-
/* Start your engines. This will never return an error. */
669-
670-
MCA_PML_CALL(start(i, reqs));
671-
672668
/* Wait for them all. If there's an error, note that we don't
673669
* care what the error was -- just that there *was* an error. The
674670
* PML will finish all requests, even if one or more of them fail.

ompi/mca/coll/base/coll_base_frame.c

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -68,15 +68,7 @@ OBJ_CLASS_INSTANCE(mca_coll_base_module_t, opal_object_t,
6868
static void
6969
coll_base_comm_construct(mca_coll_base_comm_t *data)
7070
{
71-
data->mcct_reqs = NULL;
72-
data->mcct_num_reqs = 0;
73-
data->cached_ntree = NULL;
74-
data->cached_bintree = NULL;
75-
data->cached_bmtree = NULL;
76-
data->cached_in_order_bmtree = NULL;
77-
data->cached_chain = NULL;
78-
data->cached_pipeline = NULL;
79-
data->cached_in_order_bintree = NULL;
71+
memset ((char *) data + sizeof (data->super), 0, sizeof (*data) - sizeof (data->super));
8072
}
8173

8274
static void

ompi/mca/coll/basic/coll_basic.h

Lines changed: 4 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -285,29 +285,12 @@ BEGIN_C_DECLS
285285

286286
struct mca_coll_basic_module_t {
287287
mca_coll_base_module_t super;
288-
289-
ompi_request_t **mccb_reqs;
290-
int mccb_num_reqs;
291288
};
292289
typedef struct mca_coll_basic_module_t mca_coll_basic_module_t;
293-
OBJ_CLASS_DECLARATION(mca_coll_basic_module_t);
294-
295-
/* Utility functions */
296-
297-
static inline void mca_coll_basic_free_reqs(ompi_request_t ** reqs, int count)
298-
{
299-
int i;
300-
for (i = 0; i < count; ++i)
301-
if( MPI_REQUEST_NULL != reqs[i] ) {
302-
ompi_request_free(&reqs[i]);
303-
}
304-
}
305-
306-
/**
307-
* Return the array of requests on the data. If the array was not initialized
308-
* or if it's size was too small, allocate it to fit the requested size.
309-
*/
310-
ompi_request_t** mca_coll_basic_get_reqs(mca_coll_basic_module_t* data, int nreqs);
290+
OMPI_DECLSPEC OBJ_CLASS_DECLARATION(mca_coll_basic_module_t);
291+
292+
typedef mca_coll_base_comm_t mca_coll_basic_comm_t;
293+
OMPI_DECLSPEC OBJ_CLASS_DECLARATION(mca_coll_basic_comm_t);
311294

312295
END_C_DECLS
313296

ompi/mca/coll/basic/coll_basic_allgather.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,6 @@ mca_coll_basic_allgather_inter(const void *sbuf, int scount,
5151
char *tmpbuf = NULL, *ptmp;
5252
ptrdiff_t rlb, slb, rextent, sextent, incr;
5353
ompi_request_t *req;
54-
mca_coll_basic_module_t *basic_module = (mca_coll_basic_module_t*) module;
5554
ompi_request_t **reqs = NULL;
5655

5756
rank = ompi_comm_rank(comm);
@@ -80,7 +79,7 @@ mca_coll_basic_allgather_inter(const void *sbuf, int scount,
8079
if (OMPI_SUCCESS != err) { line = __LINE__; goto exit; }
8180

8281
/* Get a requests arrays of the right size */
83-
reqs = mca_coll_basic_get_reqs(basic_module, rsize + 1);
82+
reqs = coll_base_comm_get_reqs(module->base_data, rsize + 1);
8483
if( NULL == reqs ) { line = __LINE__; goto exit; }
8584

8685
/* Do a send-recv between the two root procs. to avoid deadlock */
@@ -156,7 +155,7 @@ mca_coll_basic_allgather_inter(const void *sbuf, int scount,
156155
if( MPI_SUCCESS != err ) {
157156
OPAL_OUTPUT( (ompi_coll_base_framework.framework_output,"%s:%4d\tError occurred %d, rank %2d",
158157
__FILE__, line, err, rank) );
159-
if( NULL != reqs ) mca_coll_basic_free_reqs(reqs, rsize+1);
158+
if( NULL != reqs ) ompi_coll_base_free_reqs(reqs, rsize+1);
160159
}
161160
if (NULL != tmpbuf) {
162161
free(tmpbuf);

ompi/mca/coll/basic/coll_basic_allreduce.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,6 @@ mca_coll_basic_allreduce_inter(const void *sbuf, void *rbuf, int count,
8585
ptrdiff_t true_lb, true_extent;
8686
char *tmpbuf = NULL, *pml_buffer = NULL;
8787
ompi_request_t *req[2];
88-
mca_coll_basic_module_t *basic_module = (mca_coll_basic_module_t*) module;
8988
ompi_request_t **reqs = NULL;
9089

9190
rank = ompi_comm_rank(comm);
@@ -114,7 +113,7 @@ mca_coll_basic_allreduce_inter(const void *sbuf, void *rbuf, int count,
114113
if (NULL == tmpbuf) { err = OMPI_ERR_OUT_OF_RESOURCE; line = __LINE__; goto exit; }
115114
pml_buffer = tmpbuf - true_lb;
116115

117-
reqs = mca_coll_basic_get_reqs(basic_module, rsize - 1);
116+
reqs = coll_base_comm_get_reqs(module->base_data, rsize - 1);
118117
if( NULL == reqs ) { err = OMPI_ERR_OUT_OF_RESOURCE; line = __LINE__; goto exit; }
119118

120119
/* Do a send-recv between the two root procs. to avoid deadlock */
@@ -201,7 +200,7 @@ mca_coll_basic_allreduce_inter(const void *sbuf, void *rbuf, int count,
201200
if( MPI_SUCCESS != err ) {
202201
OPAL_OUTPUT((ompi_coll_base_framework.framework_output,"%s:%4d\tError occurred %d, rank %2d", __FILE__,
203202
line, err, rank));
204-
mca_coll_basic_free_reqs(reqs, rsize - 1);
203+
ompi_coll_base_free_reqs(reqs, rsize - 1);
205204
}
206205
if (NULL != tmpbuf) {
207206
free(tmpbuf);

ompi/mca/coll/basic/coll_basic_alltoall.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ mca_coll_basic_alltoall_inter(const void *sbuf, int scount,
7777

7878
/* Initiate all send/recv to/from others. */
7979
nreqs = size * 2;
80-
req = rreq = mca_coll_basic_get_reqs( (mca_coll_basic_module_t*) module, nreqs);
80+
req = rreq = coll_base_comm_get_reqs( module->base_data, nreqs);
8181
sreq = rreq + size;
8282

8383
prcv = (char *) rbuf;
@@ -88,7 +88,7 @@ mca_coll_basic_alltoall_inter(const void *sbuf, int scount,
8888
err = MCA_PML_CALL(irecv(prcv + (i * rcvinc), rcount, rdtype, i,
8989
MCA_COLL_BASE_TAG_ALLTOALL, comm, rreq));
9090
if (OMPI_SUCCESS != err) {
91-
mca_coll_basic_free_reqs(req, nreqs);
91+
ompi_coll_base_free_reqs(req, nreqs);
9292
return err;
9393
}
9494
}
@@ -99,7 +99,7 @@ mca_coll_basic_alltoall_inter(const void *sbuf, int scount,
9999
MCA_COLL_BASE_TAG_ALLTOALL,
100100
MCA_PML_BASE_SEND_STANDARD, comm, sreq));
101101
if (OMPI_SUCCESS != err) {
102-
mca_coll_basic_free_reqs(req, nreqs);
102+
ompi_coll_base_free_reqs(req, nreqs);
103103
return err;
104104
}
105105
}
@@ -112,7 +112,7 @@ mca_coll_basic_alltoall_inter(const void *sbuf, int scount,
112112
* the error after we free everything. */
113113
err = ompi_request_wait_all(nreqs, req, MPI_STATUSES_IGNORE);
114114
if (OMPI_SUCCESS != err) {
115-
mca_coll_basic_free_reqs(req, nreqs);
115+
ompi_coll_base_free_reqs(req, nreqs);
116116
}
117117

118118
/* All done */

ompi/mca/coll/basic/coll_basic_alltoallv.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ mca_coll_basic_alltoallv_inter(const void *sbuf, const int *scounts, const int *
6868

6969
/* Initiate all send/recv to/from others. */
7070
nreqs = rsize * 2;
71-
preq = mca_coll_basic_get_reqs((mca_coll_basic_module_t*) module, nreqs);
71+
preq = coll_base_comm_get_reqs(module->base_data, nreqs);
7272

7373
/* Post all receives first */
7474
/* A simple optimization: do not send and recv msgs of length zero */
@@ -79,7 +79,7 @@ mca_coll_basic_alltoallv_inter(const void *sbuf, const int *scounts, const int *
7979
i, MCA_COLL_BASE_TAG_ALLTOALLV, comm,
8080
&preq[i]));
8181
if (MPI_SUCCESS != err) {
82-
mca_coll_basic_free_reqs(preq, i);
82+
ompi_coll_base_free_reqs(preq, i);
8383
return err;
8484
}
8585
}
@@ -94,15 +94,15 @@ mca_coll_basic_alltoallv_inter(const void *sbuf, const int *scounts, const int *
9494
MCA_PML_BASE_SEND_STANDARD, comm,
9595
&preq[rsize + i]));
9696
if (MPI_SUCCESS != err) {
97-
mca_coll_basic_free_reqs(preq, rsize + i);
97+
ompi_coll_base_free_reqs(preq, rsize + i);
9898
return err;
9999
}
100100
}
101101
}
102102

103103
err = ompi_request_wait_all(nreqs, preq, MPI_STATUSES_IGNORE);
104104
if (MPI_SUCCESS != err) {
105-
mca_coll_basic_free_reqs(preq, nreqs);
105+
ompi_coll_base_free_reqs(preq, nreqs);
106106
}
107107

108108
/* All done */

0 commit comments

Comments
 (0)