This is an automated email from the git hooks/post-receive script. It was generated because a ref change was pushed to the repository containing the project "MPICH primary repository". The branch, master has been updated via 7dab7cd3a893e252133c326e8f096986d77b168a (commit) from 95a64b67ad45815bbbcafcd5b50abbf46232122a (commit) Those revisions listed above that are new to this repository have not appeared on any other notification email; so we list those revisions in full, below. - Log ----------------------------------------------------------------- http://git.mpich.org/mpich.git/commitdiff/7dab7cd3a893e252133c326e8f096986d7... commit 7dab7cd3a893e252133c326e8f096986d77b168a Author: Halim Amer <[email protected]> Date: Sun May 31 17:32:37 2015 -0500 Fixed several issues related to Ibsend The implementation of Ibsend had several issues: 1) incorrect ref counting when canceling a request, which leads to handle leaks; 2) writing MPI_REQUEST_NULL directly in the handle field of a request (MPID_Request) in the request pool because the handle field is passed by reference to MPIR_Wait_impl; 3) the memory used in the attached buffer is never freed after a successful cancellation This patch fixes all the above mentioned issues. In addition it fixes and closes ticket:287. Signed-off-by: Ken Raffenetti <[email protected]> diff --git a/src/mpi/pt2pt/bsendutil.c b/src/mpi/pt2pt/bsendutil.c index 1b925a3..55ba130 100644 --- a/src/mpi/pt2pt/bsendutil.c +++ b/src/mpi/pt2pt/bsendutil.c @@ -81,6 +81,7 @@ static int MPIR_Bsend_check_active ( void ); static MPIR_Bsend_data_t *MPIR_Bsend_find_buffer( int ); static void MPIR_Bsend_take_buffer( MPIR_Bsend_data_t *, int ); static int MPIR_Bsend_finalize( void * ); +static void MPIR_Bsend_free_segment( MPIR_Bsend_data_t * ); /* * Attach a buffer. This checks for the error conditions and then @@ -317,6 +318,39 @@ int MPIR_Bsend_isend(const void *buf, int count, MPI_Datatype dtype, } /* + * The following routine looks up the segment used by request req + * and frees it. The request is assumed to be completed. This routine + * is called by only MPIR_Ibsend_free. + */ +#undef FUNCNAME +#define FUNCNAME MPIR_Bsend_free_seg +#undef FCNAME +#define FCNAME MPIDI_QUOTE(FUNCNAME) +int MPIR_Bsend_free_req_seg( MPID_Request* req ) +{ + int mpi_errno = MPI_ERR_INTERN; + MPIR_Bsend_data_t *active = BsendBuffer.active; + + MPIU_DBG_MSG_P(BSEND,TYPICAL,"Checking active starting at %p", active); + while (active) { + + if (active->request == req) { + MPIR_Bsend_free_segment( active ); + mpi_errno = MPI_SUCCESS; + } + + active = active->next;; + + MPIU_DBG_MSG_P(BSEND,TYPICAL,"Next active is %p",active); + } + + fn_exit: + return mpi_errno; + fn_fail: + goto fn_exit; +} + +/* * The following routines are used to manage the allocation of bsend segments * in the user buffer. These routines handle, for example, merging segments * when an active segment that is adjacent to a free segment becomes free. diff --git a/src/mpi/pt2pt/bsendutil.h b/src/mpi/pt2pt/bsendutil.h index d5742dd..ba29964 100644 --- a/src/mpi/pt2pt/bsendutil.h +++ b/src/mpi/pt2pt/bsendutil.h @@ -12,4 +12,5 @@ int MPIR_Bsend_attach( void *, int ); int MPIR_Bsend_detach( void *, int * ); int MPIR_Bsend_isend(const void *, int, MPI_Datatype, int, int, MPID_Comm *, MPIR_Bsend_kind_t, MPID_Request ** ); +int MPIR_Bsend_free_req_seg(MPID_Request * ); diff --git a/src/mpi/pt2pt/ibsend.c b/src/mpi/pt2pt/ibsend.c index af2b5cc..1696759 100644 --- a/src/mpi/pt2pt/ibsend.c +++ b/src/mpi/pt2pt/ibsend.c @@ -46,18 +46,8 @@ PMPI_LOCAL int MPIR_Ibsend_free( void *extra ) { ibsend_req_info *ibsend_info = (ibsend_req_info *)extra; - /* Release the MPID_Request (there is still another ref pending - within the bsendutil functions) */ - /* XXX DJG FIXME-MT should we be checking this? */ - if (MPIU_Object_get_ref(ibsend_info->req) > 1) { - int inuse; - /* Note that this should mean that the request was - cancelled (that would have decremented the ref count) - */ - MPIR_Request_release_ref( ibsend_info->req, &inuse ); - } - MPIU_Free( ibsend_info ); + return MPI_SUCCESS; } #undef FUNCNAME @@ -70,6 +60,7 @@ PMPI_LOCAL int MPIR_Ibsend_cancel( void *extra, int complete ) ibsend_req_info *ibsend_info = (ibsend_req_info *)extra; MPI_Status status; MPID_Request *req = ibsend_info->req; + MPI_Request req_hdl = req->handle; /* FIXME: There should be no unreferenced args! */ /* Note that this value should always be 1 because @@ -81,10 +72,16 @@ PMPI_LOCAL int MPIR_Ibsend_cancel( void *extra, int complete ) /* Try to cancel the underlying request */ mpi_errno = MPIR_Cancel_impl(req); if (mpi_errno) MPIU_ERR_POP(mpi_errno); - mpi_errno = MPIR_Wait_impl( &req->handle, &status ); + mpi_errno = MPIR_Wait_impl( &req_hdl, &status ); if (mpi_errno) MPIU_ERR_POP(mpi_errno); MPIR_Test_cancelled_impl( &status, &ibsend_info->cancelled ); - + + /* If the cancelation is successful, free the memory in the + attached buffer used by the request */ + if (ibsend_info->cancelled) { + mpi_errno = MPIR_Bsend_free_req_seg(ibsend_info->req); + if (mpi_errno) MPIU_ERR_POP(mpi_errno); + } fn_exit: return mpi_errno; fn_fail: @@ -119,8 +116,6 @@ int MPIR_Ibsend_impl(const void *buf, int count, MPI_Datatype datatype, int dest if (mpi_errno) MPIU_ERR_POP(mpi_errno); /* The request is immediately complete because the MPIR_Bsend_isend has already moved the data out of the user's buffer */ - MPIR_Request_add_ref( request_ptr ); - /* Request count is now 2 (set to 1 in Grequest_start) */ MPIR_Grequest_complete_impl(new_request_ptr); MPIU_OBJ_PUBLISH_HANDLE(*request, new_request_ptr->handle); ----------------------------------------------------------------------- Summary of changes: src/mpi/pt2pt/bsendutil.c | 34 ++++++++++++++++++++++++++++++++++ src/mpi/pt2pt/bsendutil.h | 1 + src/mpi/pt2pt/ibsend.c | 25 ++++++++++--------------- 3 files changed, 45 insertions(+), 15 deletions(-) hooks/post-receive -- MPICH primary repository