Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

prov/efa: More fixes for efa-direct #10811

Merged
merged 2 commits into from
Feb 21, 2025
Merged

prov/efa: More fixes for efa-direct #10811

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6597c8b
prov/efa: Second fix for efa-direct's max_msg_size
shijin-aws Feb 21, 2025
4e5d894
prov/efa: Fix the cntr interface for efa-direct
shijin-aws Feb 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 32 additions & 5 deletions prov/efa/src/efa_cntr.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,10 +199,40 @@ int efa_cntr_open(struct fid_domain *domain, struct fi_cntr_attr *attr,
{
int ret;
struct efa_cntr *cntr;

cntr = calloc(1, sizeof(*cntr));
if (!cntr)
return -FI_ENOMEM;

dlist_init(&cntr->ibv_cq_poll_list);
cntr->need_to_scan_ep_list = false;

ret = ofi_cntr_init(&efa_prov, domain, attr, &cntr->util_cntr,
efa_cntr_progress, context);

if (ret)
goto free;

*cntr_fid = &cntr->util_cntr.cntr_fid;
cntr->util_cntr.cntr_fid.ops = &efa_cntr_ops;
cntr->util_cntr.cntr_fid.fid.ops = &efa_cntr_fi_ops;

return FI_SUCCESS;

free:
free(cntr);
return ret;
}


int efa_rdm_cntr_open(struct fid_domain *domain, struct fi_cntr_attr *attr,
struct fid_cntr **cntr_fid, void *context)
{
int ret;
struct efa_cntr *cntr;
struct efa_domain *efa_domain;
struct fi_cntr_attr shm_cntr_attr = {0};
struct fi_peer_cntr_context peer_cntr_context = {0};
ofi_cntr_progress_func cntr_progress_func;

cntr = calloc(1, sizeof(*cntr));
if (!cntr)
Expand All @@ -213,11 +243,8 @@ int efa_cntr_open(struct fid_domain *domain, struct fi_cntr_attr *attr,
efa_domain = container_of(domain, struct efa_domain,
util_domain.domain_fid);

cntr_progress_func = efa_domain->info->ep_attr->type == FI_EP_RDM
? efa_rdm_cntr_progress
: efa_cntr_progress;
ret = ofi_cntr_init(&efa_prov, domain, attr, &cntr->util_cntr,
cntr_progress_func, context);
efa_rdm_cntr_progress, context);

if (ret)
goto free;
Expand Down
3 changes: 3 additions & 0 deletions prov/efa/src/efa_cntr.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ struct efa_cntr {
int efa_cntr_open(struct fid_domain *domain, struct fi_cntr_attr *attr,
struct fid_cntr **cntr_fid, void *context);

int efa_rdm_cntr_open(struct fid_domain *domain, struct fi_cntr_attr *attr,
struct fid_cntr **cntr_fid, void *context);

void efa_cntr_report_tx_completion(struct util_ep *ep, uint64_t flags);

void efa_cntr_report_rx_completion(struct util_ep *ep, uint64_t flags);
Expand Down
2 changes: 1 addition & 1 deletion prov/efa/src/efa_domain.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ static struct fi_ops_domain efa_domain_ops_rdm = {
.cq_open = efa_rdm_cq_open,
.endpoint = efa_rdm_ep_open,
.scalable_ep = fi_no_scalable_ep,
.cntr_open = efa_cntr_open,
.cntr_open = efa_rdm_cntr_open,
.poll_open = fi_poll_create,
.stx_ctx = fi_no_stx_context,
.srx_ctx = fi_no_srx_context,
Expand Down
6 changes: 0 additions & 6 deletions prov/efa/src/efa_ep.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,12 +227,6 @@ static int efa_ep_bind(struct fid *fid, struct fid *bfid, uint64_t flags)

switch (bfid->fclass) {
case FI_CLASS_CQ:
if (flags & FI_SELECTIVE_COMPLETION) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did we have this in the first place? Because dgram doesn't support FI_SELECTIVE_COMPLETION?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And the efa_ep PR was merged before the FI_CONTEXT2 change, so I didn't remove it.

EFA_WARN(FI_LOG_EP_CTRL,
"Endpoint cannot be bound with selective completion.\n");
return -FI_EBADFLAGS;
}

/* Must bind a CQ to either RECV or SEND completions */
if (!(flags & (FI_RECV | FI_TRANSMIT)))
return -FI_EBADFLAGS;
Expand Down
8 changes: 7 additions & 1 deletion prov/efa/src/efa_prov_info.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,14 @@ void efa_prov_info_set_ep_attr(struct fi_info *prov_info,
prov_info->ep_attr->max_msg_size = device->ibv_port_attr.max_msg_sz;
prov_info->ep_attr->type = ep_type;

if (ep_type == FI_EP_DGRAM)
if (ep_type == FI_EP_RDM) {
/* ep_attr->max_msg_size is the maximum of both MSG and RMA operations */
if (prov_info->caps & FI_RMA)
prov_info->ep_attr->max_msg_size = MAX(device->ibv_port_attr.max_msg_sz, device->max_rdma_size);
} else {
assert(ep_type == FI_EP_DGRAM);
prov_info->ep_attr->msg_prefix_size = 40;
}
}

/**
Expand Down
8 changes: 4 additions & 4 deletions prov/efa/src/efa_user_info.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -380,11 +380,11 @@ int efa_user_info_alter_direct(int version, struct fi_info *info, const struct f
EFA_INFO(FI_LOG_CORE,
"FI_MSG_PREFIX size = %ld\n", info->ep_attr->msg_prefix_size);
}
/* When user requests FI_RMA and it's supported, the max_msg_size should be returned
* as the maximum of both MSG and RMA operations
/* When user doesn't request FI_RMA, the max_msg_size should be returned
* as the MSG only as RMA will not be used.
*/
if (hints->caps & FI_RMA)
info->ep_attr->max_msg_size = MAX(g_device_list[0].ibv_port_attr.max_msg_sz, g_device_list[0].max_rdma_size);
if (!(hints->caps & FI_RMA))
info->ep_attr->max_msg_size = g_device_list[0].ibv_port_attr.max_msg_sz;
}

/* Print a warning and use FI_AV_TABLE if the app requests FI_AV_MAP */
Expand Down
5 changes: 5 additions & 0 deletions prov/efa/test/efa_unit_test_info.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,11 @@ static void test_info_direct_attributes_impl(struct fi_info *hints,
assert_false(info->tx_attr->msg_order & FI_ORDER_SAS);
assert_int_equal(info->domain_attr->progress, FI_PROGRESS_AUTO);
assert_int_equal(info->domain_attr->control_progress, FI_PROGRESS_AUTO);
assert_int_equal(
g_device_list[0].rdm_info->ep_attr->max_msg_size,
(info->caps & FI_RMA) ?
g_device_list[0].max_rdma_size :
g_device_list[0].ibv_port_attr.max_msg_sz);
assert_int_equal(
info->ep_attr->max_msg_size,
(hints->caps & FI_RMA) ?
Expand Down