PML/UCX: improved error processing in MPI_Recv #8140
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
brminich
approved these changes
Oct 28, 2020
yosefe
reviewed
Oct 28, 2020
ompi/mca/pml/ucx/pml_ucx.c
Outdated
| @@ -627,15 +630,15 @@ int mca_pml_ucx_recv(void *buf, size_t count, ompi_datatype_t *datatype, int src | |||
| MCA_COMMON_UCX_PROGRESS_LOOP(ompi_pml_ucx.ucp_worker) { | |||
| status = ucp_request_test(req, &info); | |||
| if (status != UCS_INPROGRESS) { | |||
| mca_pml_ucx_set_recv_status_safe(mpi_status, status, &info); | |||
| mca_pml_ucx_set_recv_status(mpi_status, status, &info); | |||
There was a problem hiding this comment.
maybe add return value for mca_pml_ucx_set_recv_status(), instead of extra branch in lines 615-616?
There was a problem hiding this comment.
branch on 615-616 needed to call mca_pml_ucx_set_recv_status. I can move it in to mca_pml_ucx_set_recv_status_safe
|
@yosefe ok to squash? |
yosefe
reviewed
Oct 29, 2020
ompi/mca/pml/ucx/pml_ucx_request.h
Outdated
Comment on lines
197
to
201
| ompi_status_public_t _local_status; | ||
| ompi_status_public_t *mpi_status = (_mpi_status != MPI_STATUS_IGNORE) ? | ||
| _mpi_status : &_local_status; | ||
|
|
||
| return mca_pml_ucx_set_recv_status(mpi_status, ucp_status, info); |
There was a problem hiding this comment.
maybe avoid setting temp mpi_status field if not needed
if (mpi_status != MPI_STATUS_IGNORE) {
return mca_pml_ucx_set_recv_status(mpi_status, ucp_status, info)
} else if (status == UCS_OK || status == CANCELED) {
return UCS_OK;
} else if (status == TRUNCATED)
reutrn ERR_TRUNCATE
} else {
return ERR_INTERN
}
| @@ -627,15 +628,15 @@ int mca_pml_ucx_recv(void *buf, size_t count, ompi_datatype_t *datatype, int src | |||
| MCA_COMMON_UCX_PROGRESS_LOOP(ompi_pml_ucx.ucp_worker) { | |||
| status = ucp_request_test(req, &info); | |||
| if (status != UCS_INPROGRESS) { | |||
| mca_pml_ucx_set_recv_status_safe(mpi_status, status, &info); | |||
| result = mca_pml_ucx_set_recv_status_safe(mpi_status, status, &info); | |||
There was a problem hiding this comment.
do we need this also in other places mca_pml_ucx_set_recv_status_safe is used?
There was a problem hiding this comment.
fixed for MPI_Mrecv call, other locations are non-blocking MPI calls - not relevant
hoopoepg
force-pushed
the
topic/pml-ucx-recv-improved-errhandling
branch
from
72cde39 to
4ddfab2
Compare
|
@yosefe ok to squash? |
yosefe
reviewed
Nov 1, 2020
ompi/mca/pml/ucx/pml_ucx_request.h
Outdated
| { | ||
| if (mpi_status != MPI_STATUS_IGNORE) { | ||
| mca_pml_ucx_set_recv_status(mpi_status, ucp_status, info); | ||
| return mca_pml_ucx_set_recv_status(mpi_status, ucp_status, info); | ||
| } else if ((ucp_status == UCS_OK) || (ucp_status == UCS_ERR_CANCELED)) { |
hoopoepg
force-pushed
the
topic/pml-ucx-recv-improved-errhandling
branch
from
2946abd to
4ef5fc2
Compare
|
@yosefe ok to squash? |
- improved error processing in MPI_Recv implementation of pml UCX - added error handling for pml_ucx_mrecv call Signed-off-by: Sergey Oblomov <sergeyo@nvidia.com>
hoopoepg
force-pushed
the
topic/pml-ucx-recv-improved-errhandling
branch
from
4ef5fc2 to
eb9405d
Compare
|
@yosefe ok to merge? |
yosefe
approved these changes
Nov 3, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
of pml UCX
Signed-off-by: Sergey Oblomov sergeyo@mellanox.com