btl/base: push operation->hdr to am_rdma_respond for queued operation #10463
Conversation
This file contains hidden or 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
|
@bwbarrett @hjelmn Can you review this PR? this fixed some segfault in mtt ibm/onesided tests. |
|
Can I get a review for this PR? Thanks! |
bwbarrett
requested changes
Jun 24, 2022
wzamazon
force-pushed
the
btl_base_fix_queue_op
branch
from
d057959 to
4575ac4
Compare
Currently, when calling am_rdma_respond() for a queued operation, amd_rdma_retry_operation() pass NULL for the hdr argument. The idea is that hdr is only used for allocating operation->descriptor. A queued operation should already have a descriptor, therefore does not need hdr. This missed the possibility that the allocation of descriptor in am_rdma_respond() can fail, which will lead to the operation to be queued without a descriptor. This patch make retry_operation() to pass operation->hdr to am_rdma_repsond() to address the issue. It also added an assertion in am_rdma_repsond() about hdr must not be NULL before hdr is being used. Signed-off-by: Wei Zhang <wzam@amazon.com>
wzamazon
force-pushed
the
btl_base_fix_queue_op
branch
from
4575ac4 to
1758e3d
Compare
|
Thank you for the review! I added the assertion in new revision. |
bwbarrett
approved these changes
Jun 28, 2022
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.
Currently, when calling am_rdma_respond() for a queued
operation, amd_rdma_retry_operation() pass NULL for the hdr argument.
The idea is that hdr is only used for allocating operation->descriptor.
A queued operation should already have a descriptor, therefore does
not need hdr.
This missed the possibility that the allocation of descriptor
in am_rdma_respond() can fail, which will lead to the operation
to be queued without a descriptor.
This patch make retry_operation() to pass operation->hdr to
am_rdma_repsond() to address the issue.
Signed-off-by: Wei Zhang wzam@amazon.com