allow IOBuf::append_user_data_with_meta to specify the lkey of rdma memory region #2005
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
Tuvie
changed the title
|
|
wwbmmm
reviewed
Nov 21, 2022
… rdma memory region
372046933
reviewed
Nov 23, 2022
| uint64_t meta) { | ||
| if (size == 0) { | ||
| LOG(WARNING) << "data_size should not be 0"; | ||
| return -1; |
There was a problem hiding this comment.
Did size == 0 have any side effect? If not, can we return 0 here?
There was a problem hiding this comment.
This should be unexpected. return -1 is a better choice.
There was a problem hiding this comment.
@Tuvie Return 0 has been merged into master
https://github.com/apache/incubator-brpc/blob/b788325528d886639e66aa0d46c571c163ec959a/src/butil/iobuf.cpp#L1214-L1220
|
LGTM |
|
@wwbmmm is this PR qualified to be merged? It seems that the Travis often fails due to some irrelevant reason. |
|
Has resolved conflict. Any plan to merge? |
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.
What problem does this PR solve?
Issue Number: 1995
Problem Summary: When using IOBuf::append_user_data, if the application specify a buffer with an offset over the base address registered with rdma::RegisterMemoryForRdma, brpc reports a failure for invalid memory region.
What is changed and the side effects?
Changed: Add a new function named append_user_data_with_meta in IOBuf. Allow application specifying the lkey as meta to explicitly inform brpc. This removes the necessity to lookup the lkey for the IOBuf Block in rpc processing.
Side effects:
Performance effects(性能影响):
Breaking backward compatibility(向后兼容性): To store the meta/lkey in IOBuf Block, the portal_next field is reused when flags is non-0. It is acceptable because portal_next is only used for tls cache of IOBuf, which does not work for user defined Blocks.
Check List: