Skip to content

Commit a15b6d3

Browse files
hjelmnhjelmn
committed
pml/ob1: reset req_bytes_packed on start
On start we were not correctly resetting all request fields. This was leading to a double-completion on persistent receives. This commit updates the base start code to reset both the send and receive request req_bytes_packed fields in the START macros. The only pml that appears to be affected is ob1. Signed-off-by: Nathan Hjelm <[email protected]>
1 parent 50952c3 commit a15b6d3

File tree

5 files changed

+49
-41
lines changed

5 files changed

+49
-41
lines changed

‎ompi/mca/pml/base/pml_base_recvreq.h‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
/* -*- Mode: C; c-basic-offset:4 ; indent-tabs-mode:nil -*- */
12
/*
23
* Copyright (c) 2004-2005 The Trustees of Indiana University and Indiana
34
* University Research and Technology
@@ -10,6 +11,8 @@
1011
* Copyright (c) 2004-2005 The Regents of the University of California.
1112
* All rights reserved.
1213
* Copyright (c) 2006 Cisco Systems, Inc. All rights reserved.
14+
* Copyright (c) 2016 Los Alamos National Security, LLC. All rights
15+
* reserved.
1316
* $COPYRIGHT$
1417
*
1518
* Additional copyrights may follow
@@ -87,20 +90,21 @@ OMPI_DECLSPEC OBJ_CLASS_DECLARATION(mca_pml_base_recv_request_t);
8790
*/
8891
#define MCA_PML_BASE_RECV_START( request ) \
8992
do { \
90-
(request)->req_pml_complete = false; \
93+
(request)->req_bytes_packed = 0; \
94+
(request)->req_base.req_pml_complete = false; \
9195
\
9296
/* always set the req_status.MPI_TAG to ANY_TAG before starting the \
9397
* request. This field is used if cancelled to find out if the request \
9498
* has been matched or not. \
9599
*/ \
96-
(request)->req_ompi.req_status.MPI_SOURCE = OMPI_ANY_SOURCE; \
97-
(request)->req_ompi.req_status.MPI_TAG = OMPI_ANY_TAG; \
98-
(request)->req_ompi.req_status.MPI_ERROR = OMPI_SUCCESS; \
99-
(request)->req_ompi.req_status._ucount = 0; \
100-
(request)->req_ompi.req_status._cancelled = 0; \
100+
(request)->req_base.req_ompi.req_status.MPI_SOURCE = OMPI_ANY_SOURCE; \
101+
(request)->req_base.req_ompi.req_status.MPI_TAG = OMPI_ANY_TAG; \
102+
(request)->req_base.req_ompi.req_status.MPI_ERROR = OMPI_SUCCESS; \
103+
(request)->req_base.req_ompi.req_status._ucount = 0; \
104+
(request)->req_base.req_ompi.req_status._cancelled = 0; \
101105
\
102-
(request)->req_ompi.req_complete = REQUEST_PENDING; \
103-
(request)->req_ompi.req_state = OMPI_REQUEST_ACTIVE; \
106+
(request)->req_base.req_ompi.req_complete = REQUEST_PENDING; \
107+
(request)->req_base.req_ompi.req_state = OMPI_REQUEST_ACTIVE; \
104108
} while (0)
105109

106110
/**

‎ompi/mca/pml/base/pml_base_sendreq.h‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
/* -*- Mode: C; c-basic-offset:4 ; indent-tabs-mode:nil -*- */
12
/*
23
* Copyright (c) 2004-2005 The Trustees of Indiana University and Indiana
34
* University Research and Technology
@@ -12,6 +13,8 @@
1213
* Copyright (c) 2006 Cisco Systems, Inc. All rights reserved.
1314
* Copyright (c) 2015 Research Organization for Information Science
1415
* and Technology (RIST). All rights reserved.
16+
* Copyright (c) 2016 Los Alamos National Security, LLC. All rights
17+
* reserved.
1518
* $COPYRIGHT$
1619
*
1720
* Additional copyrights may follow
@@ -110,6 +113,13 @@ OMPI_DECLSPEC OBJ_CLASS_DECLARATION( mca_pml_base_send_request_t );
110113
} \
111114
}
112115

116+
#define MCA_PML_BASE_SEND_REQUEST_RESET(request) \
117+
if ((request)->req_bytes_packed > 0) { \
118+
opal_convertor_set_position(&(sendreq)->req_send.req_base.req_convertor, \
119+
&(size_t){0}); \
120+
(request)->req_bytes_packed = 0; \
121+
}
122+
113123
/**
114124
* Mark the request as started from the PML base point of view.
115125
*
@@ -118,10 +128,11 @@ OMPI_DECLSPEC OBJ_CLASS_DECLARATION( mca_pml_base_send_request_t );
118128

119129
#define MCA_PML_BASE_SEND_START( request ) \
120130
do { \
121-
(request)->req_pml_complete = false; \
122-
(request)->req_ompi.req_complete = REQUEST_PENDING; \
123-
(request)->req_ompi.req_state = OMPI_REQUEST_ACTIVE; \
124-
(request)->req_ompi.req_status._cancelled = 0; \
131+
(request)->req_base.req_pml_complete = false; \
132+
(request)->req_base.req_ompi.req_complete = REQUEST_PENDING; \
133+
(request)->req_base.req_ompi.req_state = OMPI_REQUEST_ACTIVE; \
134+
(request)->req_base.req_ompi.req_status._cancelled = 0; \
135+
MCA_PML_BASE_SEND_REQUEST_RESET(request); \
125136
} while (0)
126137

127138
/**

‎ompi/mca/pml/ob1/pml_ob1_irecv.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ mca_pml_ob1_imrecv( void *buf,
193193
recvreq->req_pending = false;
194194
recvreq->req_ack_sent = false;
195195

196-
MCA_PML_BASE_RECV_START(&recvreq->req_recv.req_base);
196+
MCA_PML_BASE_RECV_START(&recvreq->req_recv);
197197

198198
/* Note - sequence number already assigned */
199199
recvreq->req_recv.req_base.req_sequence = seq;
@@ -285,7 +285,7 @@ mca_pml_ob1_mrecv( void *buf,
285285
recvreq->req_rdma_idx = 0;
286286
recvreq->req_pending = false;
287287

288-
MCA_PML_BASE_RECV_START(&recvreq->req_recv.req_base);
288+
MCA_PML_BASE_RECV_START(&recvreq->req_recv);
289289

290290
/* Note - sequence number already assigned */
291291
recvreq->req_recv.req_base.req_sequence = seq;

‎ompi/mca/pml/ob1/pml_ob1_recvreq.c‎

Lines changed: 17 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
* Copyright (c) 2008 UT-Battelle, LLC. All rights reserved.
1414
* Copyright (c) 2011 Sandia National Laboratories. All rights reserved.
1515
* Copyright (c) 2012-2015 NVIDIA Corporation. All rights reserved.
16-
* Copyright (c) 2011-2015 Los Alamos National Security, LLC. All rights
16+
* Copyright (c) 2011-2016 Los Alamos National Security, LLC. All rights
1717
* reserved.
1818
* Copyright (c) 2012 FUJITSU LIMITED. All rights reserved.
1919
* Copyright (c) 2014-2015 Research Organization for Information Science
@@ -70,27 +70,25 @@ void mca_pml_ob1_recv_request_process_pending(void)
7070
static int mca_pml_ob1_recv_request_free(struct ompi_request_t** request)
7171
{
7272
mca_pml_ob1_recv_request_t* recvreq = *(mca_pml_ob1_recv_request_t**)request;
73+
assert (false == recvreq->req_recv.req_base.req_free_called);
7374

74-
if(false == recvreq->req_recv.req_base.req_free_called){
75+
recvreq->req_recv.req_base.req_free_called = true;
76+
PERUSE_TRACE_COMM_EVENT( PERUSE_COMM_REQ_NOTIFY,
77+
&(recvreq->req_recv.req_base), PERUSE_RECV );
7578

76-
recvreq->req_recv.req_base.req_free_called = true;
77-
PERUSE_TRACE_COMM_EVENT( PERUSE_COMM_REQ_NOTIFY,
78-
&(recvreq->req_recv.req_base), PERUSE_RECV );
79-
80-
if( true == recvreq->req_recv.req_base.req_pml_complete ) {
81-
/* make buffer defined when the request is compeleted,
82-
and before releasing the objects. */
83-
MEMCHECKER(
84-
memchecker_call(&opal_memchecker_base_mem_defined,
85-
recvreq->req_recv.req_base.req_addr,
86-
recvreq->req_recv.req_base.req_count,
87-
recvreq->req_recv.req_base.req_datatype);
88-
);
89-
90-
MCA_PML_OB1_RECV_REQUEST_RETURN( recvreq );
91-
}
79+
if( true == recvreq->req_recv.req_base.req_pml_complete ) {
80+
/* make buffer defined when the request is compeleted,
81+
and before releasing the objects. */
82+
MEMCHECKER(
83+
memchecker_call(&opal_memchecker_base_mem_defined,
84+
recvreq->req_recv.req_base.req_addr,
85+
recvreq->req_recv.req_base.req_count,
86+
recvreq->req_recv.req_base.req_datatype);
87+
);
9288

89+
MCA_PML_OB1_RECV_REQUEST_RETURN( recvreq );
9390
}
91+
9492
*request = MPI_REQUEST_NULL;
9593
return OMPI_SUCCESS;
9694
}
@@ -1171,7 +1169,7 @@ void mca_pml_ob1_recv_req_start(mca_pml_ob1_recv_request_t *req)
11711169
req->req_pending = false;
11721170
req->req_ack_sent = false;
11731171

1174-
MCA_PML_BASE_RECV_START(&req->req_recv.req_base);
1172+
MCA_PML_BASE_RECV_START(&req->req_recv);
11751173

11761174
OB1_MATCHING_LOCK(&ob1_comm->matching_lock);
11771175
/**

‎ompi/mca/pml/ob1/pml_ob1_sendreq.h‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
* All rights reserved.
1313
* Copyright (c) 2009 Sun Microsystems, Inc. All rights reserved.
1414
* Copyright (c) 2011-2012 NVIDIA Corporation. All rights reserved.
15-
* Copyright (c) 2011-2015 Los Alamos National Security, LLC. All rights
15+
* Copyright (c) 2011-2016 Los Alamos National Security, LLC. All rights
1616
* reserved.
1717
* $COPYRIGHT$
1818
*
@@ -156,12 +156,7 @@ get_request_from_send_pending(mca_pml_ob1_send_pending_t *type)
156156
}
157157

158158
#define MCA_PML_OB1_SEND_REQUEST_RESET(sendreq) \
159-
if ((sendreq)->req_send.req_bytes_packed > 0) { \
160-
size_t _position = 0; \
161-
opal_convertor_set_position(&(sendreq)->req_send.req_base.req_convertor, \
162-
&_position); \
163-
assert( 0 == _position ); \
164-
}
159+
MCA_PML_BASE_SEND_REQUEST_RESET(&(sendreq)->req_send)
165160

166161
static inline void mca_pml_ob1_free_rdma_resources (mca_pml_ob1_send_request_t* sendreq)
167162
{
@@ -460,7 +455,7 @@ mca_pml_ob1_send_request_start_seq (mca_pml_ob1_send_request_t* sendreq, mca_bml
460455
sendreq->req_pending = MCA_PML_OB1_SEND_PENDING_NONE;
461456
sendreq->req_send.req_base.req_sequence = seqn;
462457

463-
MCA_PML_BASE_SEND_START( &sendreq->req_send.req_base );
458+
MCA_PML_BASE_SEND_START( &sendreq->req_send );
464459

465460
for(size_t i = 0; i < mca_bml_base_btl_array_get_size(&endpoint->btl_eager); i++) {
466461
mca_bml_base_btl_t* bml_btl;

0 commit comments

Comments
 (0)