Skip to content

Commit 07fca59

Browse files
kawashima-fjbwbarrett
authored andcommitted
pml/ob1: Fix data corruption of MPI_BSEND
Data transferred by `MPI_BSEND` may corrupt if all of the following conditions are met. - The message size is less than the eager limit. - The `btl_alloc` function in the BTL interface returns `NULL` for some reason. - The MPI program overwrites the send buffer after `MPI_BSEND` returns. The problem is in the way of pending a send request in ob1 PML. The `mca_pml_ob1_send_request_start_copy` function retruns `OMPI_ERR_OUT_OF_RESOURCE` if `mca_bml_base_alloc` function returns `des = NULL`. In this case, the send request is added to the `send_pending` list and `MPI_BSEND` returns immediately. Next time the `mca_pml_ob1_send_request_start_copy` function tries sending, the user buffer may have been overwritten by the MPI program. Call hierarchy of `MPI_BSEND`: ``` MPI_Bsend mca_pml_ob1_send if (MCA_PML_BASE_SEND_BUFFERED == sendmode) mca_pml_ob1_isend MCA_PML_OB1_SEND_REQUEST_START_W_SEQ mca_pml_ob1_send_request_start_seq mca_pml_ob1_send_request_start_btl if (size <= eager_limit) if (req_send_mode == MCA_PML_BASE_SEND_BUFFERED) mca_pml_ob1_send_request_start_copy mca_bml_base_alloc btl_alloc if (OMPI_ERR_OUT_OF_RESOURCE == rc) add_request_to_send_pending ompi_request_free ``` To solve this problem, we should save the data to the buffer attached by `MPI_BUFFER_ATTACH` before leaving `MPI_BSEND`. This problem was introduced by ob1 optimization (commits 2b57f42 and a06e491) in v1.8 series. Signed-off-by: KAWASHIMA Takahiro <[email protected]> (cherry picked from commit 0021616)
1 parent 3548b36 commit 07fca59

File tree

2 files changed

+34
-20
lines changed

2 files changed

+34
-20
lines changed

ompi/mca/pml/ob1/pml_ob1_sendreq.c

Lines changed: 23 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
* Copyright (c) 2015 Cisco Systems, Inc. All rights reserved.
1919
* Copyright (c) 2016 Research Organization for Information Science
2020
* and Technology (RIST). All rights reserved.
21+
* Copyright (c) 2018 FUJITSU LIMITED. All rights reserved.
2122
* $COPYRIGHT$
2223
*
2324
* Additional copyrights may follow
@@ -417,29 +418,31 @@ int mca_pml_ob1_send_request_start_buffered(
417418
des->des_cbfunc = mca_pml_ob1_rndv_completion;
418419
des->des_cbdata = sendreq;
419420

420-
/* buffer the remainder of the message */
421-
rc = mca_pml_base_bsend_request_alloc((ompi_request_t*)sendreq);
422-
if( OPAL_UNLIKELY(OMPI_SUCCESS != rc) ) {
423-
mca_bml_base_free(bml_btl, des);
424-
return rc;
425-
}
421+
/* buffer the remainder of the message if it is not buffered yet */
422+
if( OPAL_LIKELY(sendreq->req_send.req_addr == sendreq->req_send.req_base.req_addr) ) {
423+
rc = mca_pml_base_bsend_request_alloc((ompi_request_t*)sendreq);
424+
if( OPAL_UNLIKELY(OMPI_SUCCESS != rc) ) {
425+
mca_bml_base_free(bml_btl, des);
426+
return rc;
427+
}
426428

427-
iov.iov_base = (IOVBASE_TYPE*)(((unsigned char*)sendreq->req_send.req_addr) + max_data);
428-
iov.iov_len = max_data = sendreq->req_send.req_bytes_packed - max_data;
429+
iov.iov_base = (IOVBASE_TYPE*)(((unsigned char*)sendreq->req_send.req_addr) + max_data);
430+
iov.iov_len = max_data = sendreq->req_send.req_bytes_packed - max_data;
429431

430-
if((rc = opal_convertor_pack( &sendreq->req_send.req_base.req_convertor,
431-
&iov,
432-
&iov_count,
433-
&max_data)) < 0) {
434-
mca_bml_base_free(bml_btl, des);
435-
return rc;
436-
}
432+
if((rc = opal_convertor_pack( &sendreq->req_send.req_base.req_convertor,
433+
&iov,
434+
&iov_count,
435+
&max_data)) < 0) {
436+
mca_bml_base_free(bml_btl, des);
437+
return rc;
438+
}
437439

438-
/* re-init convertor for packed data */
439-
opal_convertor_prepare_for_send( &sendreq->req_send.req_base.req_convertor,
440-
&(ompi_mpi_byte.dt.super),
441-
sendreq->req_send.req_bytes_packed,
442-
sendreq->req_send.req_addr );
440+
/* re-init convertor for packed data */
441+
opal_convertor_prepare_for_send( &sendreq->req_send.req_base.req_convertor,
442+
&(ompi_mpi_byte.dt.super),
443+
sendreq->req_send.req_bytes_packed,
444+
sendreq->req_send.req_addr );
445+
}
443446

444447
/* wait for ack and completion */
445448
sendreq->req_state = 2;

ompi/mca/pml/ob1/pml_ob1_sendreq.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
* Copyright (c) 2011-2012 NVIDIA Corporation. All rights reserved.
1515
* Copyright (c) 2011-2016 Los Alamos National Security, LLC. All rights
1616
* reserved.
17+
* Copyright (c) 2018 FUJITSU LIMITED. All rights reserved.
1718
* $COPYRIGHT$
1819
*
1920
* Additional copyrights may follow
@@ -468,6 +469,16 @@ mca_pml_ob1_send_request_start_seq (mca_pml_ob1_send_request_t* sendreq, mca_bml
468469
if( OPAL_LIKELY(OMPI_ERR_OUT_OF_RESOURCE != rc) )
469470
return rc;
470471
}
472+
if(MCA_PML_BASE_SEND_BUFFERED == sendreq->req_send.req_send_mode &&
473+
sendreq->req_send.req_addr == sendreq->req_send.req_base.req_addr) {
474+
/* in the buffered mode, the send buffer must be saved to
475+
* the attached buffer before returning it to the user */
476+
int rc;
477+
rc = mca_pml_base_bsend_request_start((ompi_request_t*)sendreq);
478+
if(OMPI_SUCCESS != rc){
479+
return rc;
480+
}
481+
}
471482
add_request_to_send_pending(sendreq, MCA_PML_OB1_SEND_PENDING_START, true);
472483

473484
return OMPI_SUCCESS;

0 commit comments

Comments
 (0)