Skip to content

GitLab

Commit dc9275be authored by Xin Zhao's avatar Xin Zhao
Browse files

Fix #1701 - cleanup code for zero-size data transfer. 



Delete code for zero-size data transfer in packet handlers
of Put/Accumulate/Accumulate_Immed/Get_AccumulateResp/GetResp/
LockPutUnlock/LockAccumUnlock, because they are redundant.

(Note that packet handlers of LockPutUnlock and LockAccumUnlock
are for single operation optimization in passive RMA)

Zero-size data transfer has already been handled when issuing
RMA operations (L146, L258, L369 in src/mpid/ch3/src/ch3u_rma_ops.c
and L50 in src/mpid/ch3/src/ch3u_rma_acc_ops.c). RMA operation
routines will directly exit if data size is zero.
Signed-off-by: default avatarWesley Bland <wbland@mcs.anl.gov>
parent b9531d3d
Hide whitespace changes
Inline Side-by-side
src/mpid/ch3/src/ch3u_rma_sync.c
View file @ dc9275be
...@@ -3398,13 +3398,6 @@ int MPIDI_CH3_PktHandler_Put( MPIDI_VC_t *vc, MPIDI_CH3_Pkt_t *pkt, ...@@ -3398,13 +3398,6 @@ int MPIDI_CH3_PktHandler_Put( MPIDI_VC_t *vc, MPIDI_CH3_Pkt_t *pkt,
type_size); type_size);
req->dev.recv_data_sz = type_size * put_pkt->count; req->dev.recv_data_sz = type_size * put_pkt->count;
if (req->dev.recv_data_sz == 0) {
MPIDI_CH3U_Request_complete( req );
*buflen = sizeof(MPIDI_CH3_Pkt_t);
*rreqp = NULL;
goto fn_exit;
}
mpi_errno = MPIDI_CH3U_Receive_data_found(req, data_buf, &data_len, mpi_errno = MPIDI_CH3U_Receive_data_found(req, data_buf, &data_len,
&complete); &complete);
MPIU_ERR_CHKANDJUMP1(mpi_errno, mpi_errno, MPI_ERR_OTHER, "**ch3|postrecv", MPIU_ERR_CHKANDJUMP1(mpi_errno, mpi_errno, MPI_ERR_OTHER, "**ch3|postrecv",
...@@ -3703,39 +3696,32 @@ int MPIDI_CH3_PktHandler_Accumulate( MPIDI_VC_t *vc, MPIDI_CH3_Pkt_t *pkt, ...@@ -3703,39 +3696,32 @@ int MPIDI_CH3_PktHandler_Accumulate( MPIDI_VC_t *vc, MPIDI_CH3_Pkt_t *pkt,
MPID_Datatype_get_size_macro(accum_pkt->datatype, type_size); MPID_Datatype_get_size_macro(accum_pkt->datatype, type_size);
req->dev.recv_data_sz = type_size * accum_pkt->count; req->dev.recv_data_sz = type_size * accum_pkt->count;
if (req->dev.recv_data_sz == 0) { mpi_errno = MPIDI_CH3U_Receive_data_found(req, data_buf, &data_len,
MPIDI_CH3U_Request_complete(req); &complete);
*buflen = sizeof(MPIDI_CH3_Pkt_t); MPIU_ERR_CHKANDJUMP1(mpi_errno, mpi_errno, MPI_ERR_OTHER, "**ch3|postrecv",
*rreqp = NULL; "**ch3|postrecv %s", "MPIDI_CH3_PKT_ACCUMULATE");
} /* FIXME: Only change the handling of completion if
else { post_data_receive reset the handler. There should
mpi_errno = MPIDI_CH3U_Receive_data_found(req, data_buf, &data_len, be a cleaner way to do this */
&complete); if (!req->dev.OnDataAvail) {
MPIU_ERR_CHKANDJUMP1(mpi_errno, mpi_errno, MPI_ERR_OTHER, "**ch3|postrecv", req->dev.OnDataAvail = MPIDI_CH3_ReqHandler_PutAccumRespComplete;
"**ch3|postrecv %s", "MPIDI_CH3_PKT_ACCUMULATE"); }
/* FIXME: Only change the handling of completion if /* return the number of bytes processed in this function */
post_data_receive reset the handler. There should *buflen = data_len + sizeof(MPIDI_CH3_Pkt_t);
be a cleaner way to do this */
if (!req->dev.OnDataAvail) { if (complete)
req->dev.OnDataAvail = MPIDI_CH3_ReqHandler_PutAccumRespComplete; {
} mpi_errno = MPIDI_CH3_ReqHandler_PutAccumRespComplete(vc, req, &complete);
/* return the number of bytes processed in this function */ if (mpi_errno) MPIU_ERR_POP(mpi_errno);
*buflen = data_len + sizeof(MPIDI_CH3_Pkt_t); if (complete)
if (complete)
{ {
mpi_errno = MPIDI_CH3_ReqHandler_PutAccumRespComplete(vc, req, &complete); *rreqp = NULL;
if (mpi_errno) MPIU_ERR_POP(mpi_errno); MPIU_INSTR_DURATION_END(rmapkt_acc_predef);
if (complete) goto fn_exit;
{
*rreqp = NULL;
MPIU_INSTR_DURATION_END(rmapkt_acc_predef);
goto fn_exit;
}
} }
MPIU_INSTR_DURATION_END(rmapkt_acc_predef); }
} MPIU_INSTR_DURATION_END(rmapkt_acc_predef);
} }
else else
{ {
...@@ -3831,52 +3817,45 @@ int MPIDI_CH3_PktHandler_Accumulate_Immed( MPIDI_VC_t *vc, MPIDI_CH3_Pkt_t *pkt, ...@@ -3831,52 +3817,45 @@ int MPIDI_CH3_PktHandler_Accumulate_Immed( MPIDI_VC_t *vc, MPIDI_CH3_Pkt_t *pkt,
MPID_Datatype_get_extent_macro(accum_pkt->datatype, extent); MPID_Datatype_get_extent_macro(accum_pkt->datatype, extent);
/* size == 0 should never happen */ MPIU_INSTR_DURATION_START(rmapkt_acc_immed_op);
if (accum_pkt->count == 0 || extent == 0) { if (win_ptr->shm_allocated == TRUE)
; MPIDI_CH3I_SHM_MUTEX_LOCK(win_ptr);
/* Data is already present */
if (accum_pkt->op == MPI_REPLACE) {
/* no datatypes required */
int len;
MPIU_Assign_trunc(len, (accum_pkt->count * extent), int);
/* FIXME: use immediate copy because this is short */
MPIUI_Memcpy( accum_pkt->addr, accum_pkt->data, len );
} }
else { else {
MPIU_INSTR_DURATION_START(rmapkt_acc_immed_op); if (HANDLE_GET_KIND(accum_pkt->op) == HANDLE_KIND_BUILTIN) {
if (win_ptr->shm_allocated == TRUE) MPI_User_function *uop;
MPIDI_CH3I_SHM_MUTEX_LOCK(win_ptr); /* get the function by indexing into the op table */
/* Data is already present */ uop = MPIR_OP_HDL_TO_FN(accum_pkt->op);
if (accum_pkt->op == MPI_REPLACE) { (*uop)(accum_pkt->data, accum_pkt->addr,
/* no datatypes required */ &(accum_pkt->count), &(accum_pkt->datatype));
int len; }
MPIU_Assign_trunc(len, (accum_pkt->count * extent), int); else {
/* FIXME: use immediate copy because this is short */ MPIU_ERR_SETANDJUMP1(mpi_errno,MPI_ERR_OP, "**opnotpredefined",
MPIUI_Memcpy( accum_pkt->addr, accum_pkt->data, len ); "**opnotpredefined %d", accum_pkt->op );
} }
else { }
if (HANDLE_GET_KIND(accum_pkt->op) == HANDLE_KIND_BUILTIN) { if (win_ptr->shm_allocated == TRUE)
MPI_User_function *uop; MPIDI_CH3I_SHM_MUTEX_UNLOCK(win_ptr);
/* get the function by indexing into the op table */ MPIU_INSTR_DURATION_END(rmapkt_acc_immed_op);
uop = MPIR_OP_HDL_TO_FN(accum_pkt->op);
(*uop)(accum_pkt->data, accum_pkt->addr,
&(accum_pkt->count), &(accum_pkt->datatype));
}
else {
MPIU_ERR_SETANDJUMP1(mpi_errno,MPI_ERR_OP, "**opnotpredefined",
"**opnotpredefined %d", accum_pkt->op );
}
}
if (win_ptr->shm_allocated == TRUE)
MPIDI_CH3I_SHM_MUTEX_UNLOCK(win_ptr);
MPIU_INSTR_DURATION_END(rmapkt_acc_immed_op);
/* There are additional steps to take if this is a passive /* There are additional steps to take if this is a passive
target RMA or the last operation from the source */ target RMA or the last operation from the source */
/* Here is the code executed in PutAccumRespComplete after the
accumulation operation */
MPID_Win_get_ptr(accum_pkt->target_win_handle, win_ptr);
mpi_errno = MPIDI_CH3_Finish_rma_op_target(vc, win_ptr, TRUE, /* Here is the code executed in PutAccumRespComplete after the
accum_pkt->flags, accumulation operation */
accum_pkt->source_win_handle); MPID_Win_get_ptr(accum_pkt->target_win_handle, win_ptr);
if (mpi_errno) { MPIU_ERR_POP(mpi_errno); }
} mpi_errno = MPIDI_CH3_Finish_rma_op_target(vc, win_ptr, TRUE,
accum_pkt->flags,
accum_pkt->source_win_handle);
if (mpi_errno) { MPIU_ERR_POP(mpi_errno); }
fn_exit: fn_exit:
MPIU_INSTR_DURATION_END(rmapkt_acc_immed); MPIU_INSTR_DURATION_END(rmapkt_acc_immed);
...@@ -4187,26 +4166,16 @@ int MPIDI_CH3_PktHandler_Get_AccumResp( MPIDI_VC_t *vc, MPIDI_CH3_Pkt_t *pkt, ...@@ -4187,26 +4166,16 @@ int MPIDI_CH3_PktHandler_Get_AccumResp( MPIDI_VC_t *vc, MPIDI_CH3_Pkt_t *pkt,
MPID_Datatype_get_size_macro(req->dev.datatype, type_size); MPID_Datatype_get_size_macro(req->dev.datatype, type_size);
req->dev.recv_data_sz = type_size * req->dev.user_count; req->dev.recv_data_sz = type_size * req->dev.user_count;
/* FIXME: It is likely that this cannot happen (never perform *rreqp = req;
a get with a 0-sized item). In that case, change this mpi_errno = MPIDI_CH3U_Receive_data_found(req, data_buf, &data_len, &complete);
to an MPIU_Assert (and do the same for accumulate and put) */ MPIU_ERR_CHKANDJUMP1(mpi_errno, mpi_errno, MPI_ERR_OTHER, "**ch3|postrecv",
if (req->dev.recv_data_sz == 0) { "**ch3|postrecv %s", "MPIDI_CH3_PKT_GET_ACCUM_RESP");
MPIDI_CH3U_Request_complete( req ); if (complete) {
*buflen = sizeof(MPIDI_CH3_Pkt_t); MPIDI_CH3U_Request_complete(req);
*rreqp = NULL; *rreqp = NULL;
} }
else { /* return the number of bytes processed in this function */
*rreqp = req; *buflen = data_len + sizeof(MPIDI_CH3_Pkt_t);
mpi_errno = MPIDI_CH3U_Receive_data_found(req, data_buf, &data_len, &complete);
MPIU_ERR_CHKANDJUMP1(mpi_errno, mpi_errno, MPI_ERR_OTHER, "**ch3|postrecv",
"**ch3|postrecv %s", "MPIDI_CH3_PKT_GET_ACCUM_RESP");
if (complete) {
MPIDI_CH3U_Request_complete(req);
*rreqp = NULL;
}
/* return the number of bytes processed in this function */
*buflen = data_len + sizeof(MPIDI_CH3_Pkt_t);
}
fn_exit: fn_exit:
MPIU_INSTR_DURATION_END(rmapkt_get_accum); MPIU_INSTR_DURATION_END(rmapkt_get_accum);
...@@ -4392,38 +4361,30 @@ int MPIDI_CH3_PktHandler_LockPutUnlock( MPIDI_VC_t *vc, MPIDI_CH3_Pkt_t *pkt, ...@@ -4392,38 +4361,30 @@ int MPIDI_CH3_PktHandler_LockPutUnlock( MPIDI_VC_t *vc, MPIDI_CH3_Pkt_t *pkt,
req->dev.user_buf = new_ptr->pt_single_op->data; req->dev.user_buf = new_ptr->pt_single_op->data;
req->dev.lock_queue_entry = new_ptr; req->dev.lock_queue_entry = new_ptr;
} }
if (req->dev.recv_data_sz == 0) { int (*fcn)( MPIDI_VC_t *, struct MPID_Request *, int * );
*buflen = sizeof(MPIDI_CH3_Pkt_t); fcn = req->dev.OnDataAvail;
MPIDI_CH3U_Request_complete(req); mpi_errno = MPIDI_CH3U_Receive_data_found(req, data_buf, &data_len,
*rreqp = NULL; &complete);
if (mpi_errno != MPI_SUCCESS) {
MPIU_ERR_SETFATALANDJUMP1(mpi_errno,MPI_ERR_OTHER,
"**ch3|postrecv", "**ch3|postrecv %s",
"MPIDI_CH3_PKT_LOCK_PUT_UNLOCK");
} }
else { req->dev.OnDataAvail = fcn;
int (*fcn)( MPIDI_VC_t *, struct MPID_Request *, int * ); *rreqp = req;
fcn = req->dev.OnDataAvail;
mpi_errno = MPIDI_CH3U_Receive_data_found(req, data_buf, &data_len,
&complete);
if (mpi_errno != MPI_SUCCESS) {
MPIU_ERR_SETFATALANDJUMP1(mpi_errno,MPI_ERR_OTHER,
"**ch3|postrecv", "**ch3|postrecv %s",
"MPIDI_CH3_PKT_LOCK_PUT_UNLOCK");
}
req->dev.OnDataAvail = fcn;
*rreqp = req;
if (complete) if (complete)
{
mpi_errno = fcn(vc, req, &complete);
if (complete)
{ {
mpi_errno = fcn(vc, req, &complete); *rreqp = NULL;
if (complete)
{
*rreqp = NULL;
}
} }
}
/* return the number of bytes processed in this function */
*buflen = data_len + sizeof(MPIDI_CH3_Pkt_t); /* return the number of bytes processed in this function */
} *buflen = data_len + sizeof(MPIDI_CH3_Pkt_t);
if (mpi_errno != MPI_SUCCESS) { if (mpi_errno != MPI_SUCCESS) {
MPIU_ERR_SETFATALANDJUMP1(mpi_errno,MPI_ERR_OTHER, MPIU_ERR_SETFATALANDJUMP1(mpi_errno,MPI_ERR_OTHER,
...@@ -4650,34 +4611,27 @@ int MPIDI_CH3_PktHandler_LockAccumUnlock( MPIDI_VC_t *vc, MPIDI_CH3_Pkt_t *pkt, ...@@ -4650,34 +4611,27 @@ int MPIDI_CH3_PktHandler_LockAccumUnlock( MPIDI_VC_t *vc, MPIDI_CH3_Pkt_t *pkt,
req->dev.lock_queue_entry = new_ptr; req->dev.lock_queue_entry = new_ptr;
*rreqp = req; *rreqp = req;
if (req->dev.recv_data_sz == 0) { mpi_errno = MPIDI_CH3U_Receive_data_found(req, data_buf, &data_len,
*buflen = sizeof(MPIDI_CH3_Pkt_t); &complete);
MPIDI_CH3U_Request_complete(req); /* FIXME: Only change the handling of completion if
*rreqp = NULL; post_data_receive reset the handler. There should
be a cleaner way to do this */
if (!req->dev.OnDataAvail) {
req->dev.OnDataAvail = MPIDI_CH3_ReqHandler_SinglePutAccumComplete;
} }
else { if (mpi_errno != MPI_SUCCESS) {
mpi_errno = MPIDI_CH3U_Receive_data_found(req, data_buf, &data_len, MPIU_ERR_SET1(mpi_errno,MPI_ERR_OTHER,"**ch3|postrecv",
&complete); "**ch3|postrecv %s", "MPIDI_CH3_PKT_LOCK_ACCUM_UNLOCK");
/* FIXME: Only change the handling of completion if }
post_data_receive reset the handler. There should /* return the number of bytes processed in this function */
be a cleaner way to do this */ *buflen = data_len + sizeof(MPIDI_CH3_Pkt_t);
if (!req->dev.OnDataAvail) {
req->dev.OnDataAvail = MPIDI_CH3_ReqHandler_SinglePutAccumComplete;
}
if (mpi_errno != MPI_SUCCESS) {
MPIU_ERR_SET1(mpi_errno,MPI_ERR_OTHER,"**ch3|postrecv",
"**ch3|postrecv %s", "MPIDI_CH3_PKT_LOCK_ACCUM_UNLOCK");
}
/* return the number of bytes processed in this function */
*buflen = data_len + sizeof(MPIDI_CH3_Pkt_t);
if (complete) if (complete)
{
mpi_errno = MPIDI_CH3_ReqHandler_SinglePutAccumComplete(vc, req, &complete);
if (complete)
{ {
mpi_errno = MPIDI_CH3_ReqHandler_SinglePutAccumComplete(vc, req, &complete); *rreqp = NULL;
if (complete)
{
*rreqp = NULL;
}
} }
} }
fn_fail: fn_fail:
...@@ -4714,27 +4668,18 @@ int MPIDI_CH3_PktHandler_GetResp( MPIDI_VC_t *vc ATTRIBUTE((unused)), ...@@ -4714,27 +4668,18 @@ int MPIDI_CH3_PktHandler_GetResp( MPIDI_VC_t *vc ATTRIBUTE((unused)),
MPID_Datatype_get_size_macro(req->dev.datatype, type_size); MPID_Datatype_get_size_macro(req->dev.datatype, type_size);
req->dev.recv_data_sz = type_size * req->dev.user_count; req->dev.recv_data_sz = type_size * req->dev.user_count;
/* FIXME: It is likely that this cannot happen (never perform *rreqp = req;
a get with a 0-sized item). In that case, change this mpi_errno = MPIDI_CH3U_Receive_data_found(req, data_buf,
to an MPIU_Assert (and do the same for accumulate and put) */ &data_len, &complete);
if (req->dev.recv_data_sz == 0) { MPIU_ERR_CHKANDJUMP1(mpi_errno, mpi_errno, MPI_ERR_OTHER, "**ch3|postrecv", "**ch3|postrecv %s", "MPIDI_CH3_PKT_GET_RESP");
MPIDI_CH3U_Request_complete( req ); if (complete)
*buflen = sizeof(MPIDI_CH3_Pkt_t); {
*rreqp = NULL; MPIDI_CH3U_Request_complete(req);
} *rreqp = NULL;
else {
*rreqp = req;
mpi_errno = MPIDI_CH3U_Receive_data_found(req, data_buf,
&data_len, &complete);
MPIU_ERR_CHKANDJUMP1(mpi_errno, mpi_errno, MPI_ERR_OTHER, "**ch3|postrecv", "**ch3|postrecv %s", "MPIDI_CH3_PKT_GET_RESP");
if (complete)
{
MPIDI_CH3U_Request_complete(req);
*rreqp = NULL;
}
/* return the number of bytes processed in this function */
*buflen = data_len + sizeof(MPIDI_CH3_Pkt_t);
} }
/* return the number of bytes processed in this function */
*buflen = data_len + sizeof(MPIDI_CH3_Pkt_t);
fn_exit: fn_exit:
MPIDI_FUNC_EXIT(MPID_STATE_MPIDI_CH3_PKTHANDLER_GETRESP); MPIDI_FUNC_EXIT(MPID_STATE_MPIDI_CH3_PKTHANDLER_GETRESP);
return mpi_errno; return mpi_errno;
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment