prov/efa: Fix fi_sendmsg/fi_writemsg with FI_INJECT #10347
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
shijin-aws
reviewed
Aug 28, 2024
jiaxiyan
force-pushed
the
fi_inject
branch
2 times, most recently
from
0fbfa87
to
c9c6b03
Compare
shijin-aws
approved these changes
Aug 28, 2024
prov/efa/src/rdm/efa_rdm_pke_utils.c
Outdated
| * and we use 1st iov for header. | ||
| * 2. EFA can directly access the memory | ||
| * Note that FI_INJECT requires outbound buffer to be reusable | ||
| * immediately after function returns, so without the inline HW support, |
There was a problem hiding this comment.
Can you rephrase the without the inline HW support, to unless using inline
There was a problem hiding this comment.
Also can we change EFA can directly access the memory to p2p is available or using inline here
prov/efa/src/rdm/efa_rdm_pke_utils.c
Outdated
| @@ -61,34 +62,36 @@ ssize_t efa_rdm_pke_init_payload_from_ope(struct efa_rdm_pke *pke, | |||
| assert(tx_iov_index < ope->iov_count); | |||
| assert(tx_iov_offset < ope->iov[tx_iov_index].iov_len); | |||
| iov_mr = ope->desc[tx_iov_index]; | |||
|
|
|||
| p2p_available = false; | |||
| inline_send = false; | |||
| /* When using EFA device, EFA device can access memory that | |||
There was a problem hiding this comment.
Since we are removing efa_can_access_memory, I think we can remove these comments, just explain how copy can be removed based on the p2p_available and inline_send status later.
Currently EFA provider doesn't handle the fi_sendmsg call with FI_INJECT flags correctly. It allows user to pass in a buffer with valid MR while doing inject - outbound buffer should be reusable immediately after function returns. That being said, without the inline HW support, EFA provider should do a copy from the user buffer to the internal bounce buffer even if application provides a valid MR for the buffer. Signed-off-by: Jessie Yang <[email protected]>
shijin-aws
reviewed
Aug 28, 2024
fi_writemsg with FI_INJECT needs to copy from outbound buffer to internal bounce buffer. The outbound buffer can be non-system. Extract and call a helper function for device to host copy. Signed-off-by: Jessie Yang <[email protected]>
|
bot:aws:retest |
1 similar comment
|
bot:aws:retest |
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 EFA provider doesn't handle the *msg call with FI_INJECT flags correctly. It allows user to pass in a buffer with valid MR while doing inject - outbound buffer should be reusable immediately after function returns.
That being said, without the inline HW support, EFA provider should do a copy from the user buffer to the internal bounce buffer even if application provides a valid MR for the buffer.
The inject rdma write path assumes the outbound buffer is always FI_HMEM_SYSTEM, but it doesn't have to.