prov/efa: Remove efa_av->ep_type in favor of efa_domain->info_type #10827
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
sunkuamzn
force-pushed
the
rdm-av-ep-type
branch
from
10071c7 to
e07e1ee
Compare
sunkuamzn
force-pushed
the
rdm-av-ep-type
branch
from
7a60ade to
9081c7e
Compare
shijin-aws
reviewed
Feb 26, 2025
fabtests/functional/av_xfer.c
Outdated
| @@ -233,8 +233,7 @@ int main(int argc, char **argv) | |||
| if (optind < argc) | |||
| opts.dst_addr = argv[optind]; | |||
|
|
|||
| hints->caps = hints->ep_attr->type == FI_EP_RDM ? | |||
| FI_TAGGED : FI_MSG; | |||
| hints->caps = FI_MSG; | |||
There was a problem hiding this comment.
This should be a separate PR or commit, at least we shouldn't mention efa-direct as the motivation of this common change
There was a problem hiding this comment.
Even if the test doesn't require tag matching, it's better to allow FI_MSG and FI_TAGGED toggling via -o
There was a problem hiding this comment.
moved the fabtests commit to #10828
sunkuamzn
force-pushed
the
rdm-av-ep-type
branch
2 times, most recently
from
c57482e to
3665a63
Compare
shijin-aws
reviewed
Feb 27, 2025
| * | ||
| * @param[in] state struct efa_resource that is managed by the framework | ||
| */ | ||
| void test_av_ep_type_efa_rdm(struct efa_resource **state) |
There was a problem hiding this comment.
This change should belong to your next commit?
prov/efa/src/efa_av.c
Outdated
| @@ -272,8 +272,13 @@ int efa_conn_rdm_init(struct efa_av *av, struct efa_conn *conn, bool insert_shm_ | |||
| assert(!dlist_empty(&av->util_av.ep_list)); | |||
| efa_rdm_ep = container_of(av->util_av.ep_list.next, struct efa_rdm_ep, base_ep.util_ep.av_entry); | |||
|
|
|||
| peer = &conn->rdm_peer; | |||
| efa_rdm_peer_construct(peer, efa_rdm_ep, conn); | |||
| peer = (struct efa_rdm_peer *)calloc(1, sizeof(struct efa_rdm_peer)); | |||
There was a problem hiding this comment.
peer is a big object, should we use bufferpool to optimize the allocation?
There was a problem hiding this comment.
Tests are timing out now, so bufpool is not optional
sunkuamzn
force-pushed
the
rdm-av-ep-type
branch
from
3665a63 to
6f9963f
Compare
shijin-aws
reviewed
Feb 27, 2025
prov/efa/src/efa_av.c
Outdated
| .flags = OFI_BUFPOOL_NO_TRACK, | ||
| }; | ||
|
|
||
| ret = ofi_bufpool_create_attr(&rdm_peer_pool_attr, &av->rdm_peer_pool); |
There was a problem hiding this comment.
Why it is only created when we have shm? Should it be moved to L916?
sunkuamzn
force-pushed
the
rdm-av-ep-type
branch
from
6f9963f to
e3f5982
Compare
domain->rdm_ep can't distinguish between efa-direct and dgram code paths Signed-off-by: Sai Sunku <sunkusa@amazon.com>
efa_av->ep_type was used to distinguish between the RDM path and the DGRAM path. The efa-direct path has FI_EP_RDM ep type but it does not require a efa_conn->rdm_peer. But the efa-direct path still uses unique connid based on timestamp. Use efa_domain->info_type to distinguish the three code paths. Signed-off-by: Sai Sunku <sunkusa@amazon.com>
Use a separate bufpool to allocate the peer struct in the rdm path Signed-off-by: Sai Sunku <sunkusa@amazon.com>
sunkuamzn
force-pushed
the
rdm-av-ep-type
branch
from
e3f5982 to
3920757
Compare
|
bot:aws:retest |
jiaxiyan
approved these changes
Mar 3, 2025
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.
prov/efa: Do not allocate rdm_peer struct for efa-direct and dgram paths
prov/efa: Remove efa_av->ep_type in favor of efa_domain->info_type
efa_av->ep_type was used to distinguish between the RDM path and the
DGRAM path. The efa-direct path has FI_EP_RDM ep type but it does not
require a efa_conn->rdm_peer. But the efa-direct path still uses unique connid
based on timestamp.
Use efa_domain->info_type to distinguish the three code paths.
prov/efa: Replace domain->rdm_ep with domain->info_type
domain->rdm_ep can't distinguish between efa-direct and dgram code paths