prov/util: Remove peer rx entry if it's in the unspec_unexp queue #11402
+8
−0
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
The EFA provider AV keeps track of peers that any endpoint bound to the AV has received messages from but were not explicitly inserted into the AV by the application. The provider limits the number of such peers to prevent runaway memory usage. When the EFA provider has to remove such an implicitly inserted peer, it also needs to remove any unmatched entries in the SRX queues associated with the peer. The EFA provider calls free_entry to remove the rx entry. But free_entry does not remove the rx entry from the unspec_unexp_[msg,tag]_queue dlist which corrupts the dlist. This commit adds logic to remove any entry that is about to be freed from the unspec_unexp_[msg,tag]_queue. Signed-off-by: Sai Sunku <[email protected]>
j-xiong
reviewed
Sep 11, 2025
| * the queue does not get corrupted. */ | ||
| if (entry->addr == FI_ADDR_UNSPEC) { | ||
| dlist_remove(&util_entry->d_entry); | ||
| } |
There was a problem hiding this comment.
The braces can be got rid of here.
shijin-aws
reviewed
Sep 11, 2025
| /* If free_entry was called but the addr is still FI_ADDR_UNSPEC, the rx | ||
| * entry must be in the unspec_unexp_[msg,tag]_queue. Remove it so that | ||
| * the queue does not get corrupted. */ | ||
| if (entry->addr == FI_ADDR_UNSPEC) { |
There was a problem hiding this comment.
if an fi_trecv is called with FI_ADDR_UNSPEC, the util_rx_entry will be created with addr as FI_ADDR_UNSPEC as well and put in the expected rx queue. Then is this logic still correct? I forgot when free_entry will be called, it's before matching or after, or both
|
Discussed offline with @aingerson. This PR also needs to handle the case when the rx entry is matched to a packet. This is related to @shijin-aws's comment. There doesn't seem to be an easy fix that covers all cases. My previous PR (#11401) is enough to stabilize our tests on the v2.3.x branch. So I'm removing the v2.3.x label from this PR to unblock the v2.3 release |
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.
The EFA provider AV keeps track of peers that any endpoint bound to the AV has received messages from but were not explicitly inserted into the AV by the application. The provider limits the number of such peers to prevent runaway memory usage.
When the EFA provider has to remove such an implicitly inserted peer, it also needs to remove any unmatched entries in the SRX queues associated with the peer. The EFA provider calls free_entry to remove the rx entry. But free_entry does not remove the rx entry from the unspec_unexp_[msg,tag]_queue dlist which corrupts the dlist.
This commit adds logic to remove any entry that is about to be freed from the unspec_unexp_[msg,tag]_queue.