From 08e559fc45ea218ad65bccb97cdf5f291e38435d Mon Sep 17 00:00:00 2001 From: Sergey Gubanov Date: Tue, 31 Oct 2017 16:54:46 +0300 Subject: [PATCH] Fix bugs in the shm posix active message implementation --- src/mpid/ch4/shm/posix/posix_am.h | 10 ++---- src/mpid/ch4/shm/posix/posix_pre.h | 22 ++++++------ src/mpid/ch4/shm/posix/posix_progress.h | 47 +++++++++++++++---------- src/mpid/ch4/src/ch4_request.h | 2 ++ src/mpid/ch4/src/ch4r_request.h | 2 ++ 5 files changed, 47 insertions(+), 36 deletions(-) diff --git a/src/mpid/ch4/shm/posix/posix_am.h b/src/mpid/ch4/shm/posix/posix_am.h index 6c53ed75b2b..a48b8add6ec 100644 --- a/src/mpid/ch4/shm/posix/posix_am.h +++ b/src/mpid/ch4/shm/posix/posix_am.h @@ -58,7 +58,7 @@ MPIDI_POSIX_am_isend(int rank, MPIDI_POSIX_am_header_t *msg_hdr_p = &msg_hdr; MPIDI_POSIX_am_request_header_t *curr_sreq_hdr = NULL; - if (unlikely(!dt_contig)) { + if (unlikely(!dt_contig && (data_sz > 0))) { size_t segment_first; MPI_Aint last; struct MPIR_Segment *segment_ptr = NULL; @@ -105,11 +105,7 @@ MPIDI_POSIX_am_isend(int rank, struct iovec *iov_left_ptr = iov_left; - size_t iov_num_left = 2; - - if (!data || !count) { - iov_num_left = 1; - } + size_t iov_num_left = (data_sz > 0) ? 2 : 1; if (unlikely(MPIDI_POSIX_global.postponed_queue)) { goto enqueue_request; @@ -189,7 +185,7 @@ MPIDI_POSIX_am_isend(int rank, } if (iov_num_left == 1) { - if (!data || !count) { + if (data_sz == 0) { curr_sreq_hdr->iov[0].iov_base = curr_sreq_hdr->am_hdr; curr_sreq_hdr->iov[0].iov_len = curr_sreq_hdr->am_hdr_sz; } diff --git a/src/mpid/ch4/shm/posix/posix_pre.h b/src/mpid/ch4/shm/posix/posix_pre.h index 3ce3769b115..dc20e4048f6 100644 --- a/src/mpid/ch4/shm/posix/posix_pre.h +++ b/src/mpid/ch4/shm/posix/posix_pre.h @@ -21,7 +21,7 @@ #define MPIDI_POSIX_AM_DATA_SZ_BITS (48) #define MPIDI_POSIX_AM_MSG_HEADER_SIZE (sizeof(MPIDI_POSIX_am_header_t)) -#define MPIDI_POSIX_MAX_IOV_NUM (8) +#define MPIDI_POSIX_MAX_IOV_NUM (2) typedef enum { MPIDI_POSIX_EAGER_RECV_POSTED_HOOK_STATE_INITIALIZED, @@ -45,8 +45,7 @@ typedef struct { } MPIDI_POSIX_op_t; typedef struct { - MPIDI_POSIX_EAGER_recv_posted_hook_state_t eager_recv_posted_hook_state; - int eager_recv_posted_hook_grank; + int dummy; } MPIDI_POSIX_request_t; typedef struct MPIDI_POSIX_am_header_t { @@ -79,7 +78,6 @@ typedef struct { struct iovec *iov_ptr; struct iovec iov[MPIDI_POSIX_MAX_IOV_NUM]; size_t iov_num; - size_t iov_num_total; int is_contig; @@ -109,29 +107,31 @@ typedef struct { #ifdef POSIX_AM_REQUEST_INLINE MPIDI_POSIX_am_request_header_t req_hdr_buffer; #endif /* POSIX_AM_REQUEST_INLINE */ + MPIDI_POSIX_EAGER_recv_posted_hook_state_t eager_recv_posted_hook_state; + int eager_recv_posted_hook_grank; } MPIDI_POSIX_am_request_t; #define MPIDI_POSIX_EAGER_RECV_INITIALIZE_HOOK(request)\ do { \ - (request)->dev.ch4.shm.posix.eager_recv_posted_hook_state = MPIDI_POSIX_EAGER_RECV_POSTED_HOOK_STATE_INITIALIZED; \ + (request)->dev.ch4.am.shm_am.posix.eager_recv_posted_hook_state = MPIDI_POSIX_EAGER_RECV_POSTED_HOOK_STATE_INITIALIZED; \ } while (0) #define MPIDI_POSIX_EAGER_RECV_POSTED_HOOK(request,rank,communicator)\ do { \ - if ((request) && ((request)->dev.ch4.shm.posix.eager_recv_posted_hook_state == MPIDI_POSIX_EAGER_RECV_POSTED_HOOK_STATE_INITIALIZED)) { \ + if ((request) && ((request)->dev.ch4.am.shm_am.posix.eager_recv_posted_hook_state == MPIDI_POSIX_EAGER_RECV_POSTED_HOOK_STATE_INITIALIZED)) { \ int grank = ((rank) >= 0) ? MPIDI_CH4U_rank_to_lpid((rank), (communicator)) : (rank); \ - (request)->dev.ch4.shm.posix.eager_recv_posted_hook_state = MPIDI_POSIX_EAGER_RECV_POSTED_HOOK_STATE_REGISTERED; \ - (request)->dev.ch4.shm.posix.eager_recv_posted_hook_grank = grank; \ + (request)->dev.ch4.am.shm_am.posix.eager_recv_posted_hook_state = MPIDI_POSIX_EAGER_RECV_POSTED_HOOK_STATE_REGISTERED; \ + (request)->dev.ch4.am.shm_am.posix.eager_recv_posted_hook_grank = grank; \ MPIDI_POSIX_eager_recv_posted_hook(grank); \ } \ } while(0) #define MPIDI_POSIX_EAGER_RECV_COMPLETED_HOOK(request)\ do { \ - if ((request)->dev.ch4.shm.posix.eager_recv_posted_hook_state == MPIDI_POSIX_EAGER_RECV_POSTED_HOOK_STATE_REGISTERED) { \ - MPIDI_POSIX_eager_recv_completed_hook((request)->dev.ch4.shm.posix.eager_recv_posted_hook_grank); \ + if ((request)->dev.ch4.am.shm_am.posix.eager_recv_posted_hook_state == MPIDI_POSIX_EAGER_RECV_POSTED_HOOK_STATE_REGISTERED) { \ + MPIDI_POSIX_eager_recv_completed_hook((request)->dev.ch4.am.shm_am.posix.eager_recv_posted_hook_grank); \ } \ - (request)->dev.ch4.shm.posix.eager_recv_posted_hook_state = MPIDI_POSIX_EAGER_RECV_POSTED_HOOK_STATE_FINALIZED; \ + (request)->dev.ch4.am.shm_am.posix.eager_recv_posted_hook_state = MPIDI_POSIX_EAGER_RECV_POSTED_HOOK_STATE_FINALIZED; \ } while (0) #endif /* POSIX_PRE_H_INCLUDED */ diff --git a/src/mpid/ch4/shm/posix/posix_progress.h b/src/mpid/ch4/shm/posix/posix_progress.h index 50c7e1ba483..8f7d2437564 100644 --- a/src/mpid/ch4/shm/posix/posix_progress.h +++ b/src/mpid/ch4/shm/posix/posix_progress.h @@ -76,8 +76,16 @@ MPL_STATIC_INLINE_PREFIX int MPIDI_POSIX_progress(int blocking) payload_left -= msg_hdr->am_hdr_sz; if (rreq) { - if ((!p_data || !p_data_sz) && target_cmpl_cb) { - target_cmpl_cb(rreq); + if ((p_data_sz == 0) && (in_total_data_sz == 0)) { + /* zero message size optimization */ + + MPIR_STATUS_SET_COUNT(rreq->status, 0); + + rreq->status.MPI_ERROR = MPI_SUCCESS; + + if (target_cmpl_cb) { + target_cmpl_cb(rreq); + } MPIDI_POSIX_eager_recv_commit(&transaction); @@ -99,7 +107,10 @@ MPL_STATIC_INLINE_PREFIX int MPIDI_POSIX_progress(int blocking) MPIR_STATUS_SET_COUNT(rreq->status, recv_data_sz); MPIDI_POSIX_eager_recv_memcpy(&transaction, p_data, payload, recv_data_sz); - target_cmpl_cb(rreq); + + if (target_cmpl_cb) { + target_cmpl_cb(rreq); + } MPIDI_POSIX_eager_recv_commit(&transaction); @@ -120,27 +131,25 @@ MPL_STATIC_INLINE_PREFIX int MPIDI_POSIX_progress(int blocking) curr_rreq_hdr->dst_grank = transaction.src_grank; if (is_contig) { - curr_rreq_hdr->iov[0].iov_base = p_data; - curr_rreq_hdr->iov[0].iov_len = p_data_sz; + curr_rreq_hdr->iov_ptr = curr_rreq_hdr->iov; + + curr_rreq_hdr->iov_ptr[0].iov_base = p_data; + curr_rreq_hdr->iov_ptr[0].iov_len = p_data_sz; curr_rreq_hdr->iov_num = 1; - curr_rreq_hdr->iov_num_total = 1; recv_data_sz = p_data_sz; } else { - for (i = 0; i < p_data_sz; i++) { - curr_rreq_hdr->iov[i] = ((struct iovec *) p_data)[i]; + curr_rreq_hdr->iov_ptr = ((struct iovec *) p_data); - recv_data_sz += curr_rreq_hdr->iov[i].iov_len; + for (i = 0; i < p_data_sz; i++) { + recv_data_sz += curr_rreq_hdr->iov_ptr[i].iov_len; } curr_rreq_hdr->iov_num = p_data_sz; - curr_rreq_hdr->iov_num_total = p_data_sz; } - curr_rreq_hdr->iov_ptr = curr_rreq_hdr->iov; - /* Set final request status */ if (in_total_data_sz > recv_data_sz) { @@ -182,7 +191,7 @@ MPL_STATIC_INLINE_PREFIX int MPIDI_POSIX_progress(int blocking) for (i = 0; i < curr_rreq_hdr->iov_num; i++) { if (payload_left < curr_rreq_hdr->iov_ptr[i].iov_len) { MPIDI_POSIX_eager_recv_memcpy(&transaction, - curr_rreq_hdr->iov[i].iov_base, payload, payload_left); + curr_rreq_hdr->iov_ptr[i].iov_base, payload, payload_left); curr_rreq_hdr->iov_ptr[i].iov_base += payload_left; curr_rreq_hdr->iov_ptr[i].iov_len -= payload_left; @@ -206,13 +215,15 @@ MPL_STATIC_INLINE_PREFIX int MPIDI_POSIX_progress(int blocking) iov_done++; } - curr_rreq_hdr->iov_num -= iov_done; - - if (curr_rreq_hdr->in_total_data_sz) { - curr_rreq_hdr->iov_ptr = - &(curr_rreq_hdr->iov[curr_rreq_hdr->iov_num_total - curr_rreq_hdr->iov_num]); + if (curr_rreq_hdr->iov_num) { + curr_rreq_hdr->iov_num -= iov_done; + curr_rreq_hdr->iov_ptr += iov_done; } else { + curr_rreq_hdr->in_total_data_sz -= payload_left; + } + + if (curr_rreq_hdr->in_total_data_sz == 0) { /* All fragments have been received */ MPIDI_POSIX_global.active_rreq[transaction.src_grank] = NULL; diff --git a/src/mpid/ch4/src/ch4_request.h b/src/mpid/ch4/src/ch4_request.h index e9c71055ef5..c20e3ade131 100644 --- a/src/mpid/ch4/src/ch4_request.h +++ b/src/mpid/ch4/src/ch4_request.h @@ -102,7 +102,9 @@ MPL_STATIC_INLINE_PREFIX int MPID_Request_complete(MPIR_Request * req) MPIDI_CH4R_release_buf(MPIDI_CH4U_REQUEST(req, req)); MPIDI_CH4U_REQUEST(req, req) = NULL; MPIDI_NM_am_request_finalize(req); +#ifdef MPIDI_BUILD_CH4_SHM MPIDI_SHM_am_request_finalize(req); +#endif /* MPIDI_BUILD_CH4_SHM */ } MPIR_Request_free(req); } diff --git a/src/mpid/ch4/src/ch4r_request.h b/src/mpid/ch4/src/ch4r_request.h index be0bb671a36..485dbe4d839 100644 --- a/src/mpid/ch4/src/ch4r_request.h +++ b/src/mpid/ch4/src/ch4r_request.h @@ -41,7 +41,9 @@ static inline MPIR_Request *MPIDI_CH4I_am_request_create(MPIR_Request_kind_t kin MPIR_Request_add_ref(req); MPIDI_NM_am_request_init(req); +#ifdef MPIDI_BUILD_CH4_SHM MPIDI_SHM_am_request_init(req); +#endif /* MPIDI_BUILD_CH4_SHM */ CH4_COMPILE_TIME_ASSERT(sizeof(MPIDI_CH4U_req_ext_t) <= MPIDI_CH4I_BUF_POOL_SZ); MPIDI_CH4U_REQUEST(req, req) =