Skip to content

Commit

Permalink
prov/efa: Post rx buffer in the first fi_cntr_read
Browse files Browse the repository at this point in the history
It may be possible that application
only use fi_cntr_read to poll completions without calling
fi_cq_read. In this case, the first fi_cntr_read should
post rx buffers to all bounded eps as well.

Signed-off-by: Shi Jin <sjina@amazon.com>
  • Loading branch information
shijin-aws committed Jul 2, 2024
1 parent e433434 commit 85e204b
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 0 deletions.
19 changes: 19 additions & 0 deletions prov/efa/src/efa_cntr.c
Original file line number Diff line number Diff line change
Expand Up @@ -147,11 +147,29 @@ static void efa_rdm_cntr_progress(struct util_cntr *cntr)
struct efa_cntr *efa_cntr;
struct efa_domain *efa_domain;
struct efa_ibv_cq_poll_list_entry *poll_list_entry;
struct efa_rdm_ep *efa_rdm_ep;
struct fid_list_entry *fid_entry;

ofi_genlock_lock(&cntr->ep_list_lock);
efa_cntr = container_of(cntr, struct efa_cntr, util_cntr);
efa_domain = container_of(efa_cntr->util_cntr.domain, struct efa_domain, util_domain);

/**
* TODO: It's better to just post the initial batch of internal rx pkts during ep enable
* so we don't have to iterate cq->ep_list here.
* However, it is observed that doing that will hurt performance if application opens
* some idle endpoints and never poll completions for them. Move these initial posts to
* the first cq read call before having a long term fix.
*/
if (!efa_cntr->initial_rx_to_all_eps_posted) {
dlist_foreach(&cntr->ep_list, item) {
fid_entry = container_of(item, struct fid_list_entry, entry);
efa_rdm_ep = container_of(fid_entry->fid, struct efa_rdm_ep, base_ep.util_ep.ep_fid.fid);
efa_rdm_ep_post_internal_rx_pkts(efa_rdm_ep);
}
efa_cntr->initial_rx_to_all_eps_posted = true;
}

dlist_foreach(&efa_cntr->ibv_cq_poll_list, item) {
poll_list_entry = container_of(item, struct efa_ibv_cq_poll_list_entry, entry);
efa_rdm_cq_poll_ibv_cq(efa_env.efa_cq_read_size, poll_list_entry->cq);
Expand All @@ -175,6 +193,7 @@ int efa_cntr_open(struct fid_domain *domain, struct fi_cntr_attr *attr,
return -FI_ENOMEM;

dlist_init(&cntr->ibv_cq_poll_list);
cntr->initial_rx_to_all_eps_posted = false;
efa_domain = container_of(domain, struct efa_domain,
util_domain.domain_fid);

Expand Down
2 changes: 2 additions & 0 deletions prov/efa/src/efa_cntr.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ struct efa_cntr {
struct util_cntr util_cntr;
struct fid_cntr *shm_cntr;
struct dlist_entry ibv_cq_poll_list;
/* Only used by RDM EP type */
bool initial_rx_to_all_eps_posted;
};

int efa_cntr_open(struct fid_domain *domain, struct fi_cntr_attr *attr,
Expand Down
43 changes: 43 additions & 0 deletions prov/efa/test/efa_unit_test_cntr.c
Original file line number Diff line number Diff line change
Expand Up @@ -95,4 +95,47 @@ void test_efa_rdm_cntr_ibv_cq_poll_list_separate_tx_rx_cq_single_ep(struct efa_r
fi_close(&cntr->fid);
}

void test_efa_cntr_post_initial_rx_pkts(struct efa_resource **state)
{
struct efa_resource *resource = *state;
struct efa_rdm_ep *efa_rdm_ep;
struct fid_cntr *cntr;
struct fi_cntr_attr cntr_attr = {0};
struct efa_cntr *efa_cntr;
uint64_t cnt;

efa_unit_test_resource_construct_ep_not_enabled(resource, FI_EP_RDM);
efa_rdm_ep = container_of(resource->ep, struct efa_rdm_ep, base_ep.util_ep.ep_fid);

/* At this time, rx pkts are not growed and posted */
assert_int_equal(efa_rdm_ep->efa_rx_pkts_to_post, 0);
assert_int_equal(efa_rdm_ep->efa_rx_pkts_posted, 0);
assert_int_equal(efa_rdm_ep->efa_rx_pkts_held, 0);

assert_int_equal(fi_cntr_open(resource->domain, &cntr_attr, &cntr, NULL), 0);

/* TODO: expand this test to all flags */
assert_int_equal(fi_ep_bind(resource->ep, &cntr->fid, FI_TRANSMIT), 0);

assert_int_equal(fi_enable(resource->ep), 0);

efa_cntr = container_of(cntr, struct efa_cntr, util_cntr.cntr_fid);

assert_false(efa_cntr->initial_rx_to_all_eps_posted);

cnt = fi_cntr_read(cntr);
/* No completion should be read */
assert_int_equal(cnt, 0);

/* At this time, rx pool size number of rx pkts are posted */
assert_int_equal(efa_rdm_ep->efa_rx_pkts_posted, efa_rdm_ep_get_rx_pool_size(efa_rdm_ep));
assert_int_equal(efa_rdm_ep->efa_rx_pkts_to_post, 0);
assert_int_equal(efa_rdm_ep->efa_rx_pkts_held, 0);

assert_true(efa_cntr->initial_rx_to_all_eps_posted);
/* ep must be closed before cq/av/eq... */
fi_close(&resource->ep->fid);
resource->ep = NULL;

fi_close(&cntr->fid);
}
1 change: 1 addition & 0 deletions prov/efa/test/efa_unit_tests.c
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ int main(void)
cmocka_unit_test_setup_teardown(test_efa_rdm_cq_post_initial_rx_pkts, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown),
cmocka_unit_test_setup_teardown(test_efa_rdm_cntr_ibv_cq_poll_list_same_tx_rx_cq_single_ep, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown),
cmocka_unit_test_setup_teardown(test_efa_rdm_cntr_ibv_cq_poll_list_separate_tx_rx_cq_single_ep, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown),
cmocka_unit_test_setup_teardown(test_efa_cntr_post_initial_rx_pkts, efa_unit_test_mocks_setup, efa_unit_test_mocks_teardown),
};

cmocka_set_message_output(CM_OUTPUT_XML);
Expand Down
1 change: 1 addition & 0 deletions prov/efa/test/efa_unit_tests.h
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ void test_efa_rdm_cq_ibv_cq_poll_list_separate_tx_rx_cq_single_ep();
void test_efa_rdm_cq_post_initial_rx_pkts();
void test_efa_rdm_cntr_ibv_cq_poll_list_same_tx_rx_cq_single_ep();
void test_efa_rdm_cntr_ibv_cq_poll_list_separate_tx_rx_cq_single_ep();
void test_efa_cntr_post_initial_rx_pkts();

static inline
int efa_unit_test_get_dlist_length(struct dlist_entry *head)
Expand Down

0 comments on commit 85e204b

Please sign in to comment.