diff --git a/prov/efa/src/efa_av.c b/prov/efa/src/efa_av.c index f6e339d8f35..b0539287333 100644 --- a/prov/efa/src/efa_av.c +++ b/prov/efa/src/efa_av.c @@ -685,6 +685,7 @@ int efa_av_init_base(struct efa_av *av, struct efa_domain *efa_domain, av->domain = efa_domain; av->type = attr->type; + av->strict_mode = efa_env.strict_mode; return 0; } diff --git a/prov/efa/src/efa_av.h b/prov/efa/src/efa_av.h index 9744d9e958e..3fac4772aa2 100644 --- a/prov/efa/src/efa_av.h +++ b/prov/efa/src/efa_av.h @@ -134,6 +134,9 @@ struct efa_prv_reverse_av { struct efa_av { struct efa_domain *domain; enum fi_av_type type; + /* efa_env.strict_mode at open, so the FI_AV_MAP lookup does not read the + * global on every operation */ + bool strict_mode; /* cur_reverse_av is a map from (ahn + qpn) to the current (latest) * efa_av_entry. */ struct efa_av_array *cur_reverse_av; @@ -192,7 +195,7 @@ static inline struct efa_av_entry *efa_av_addr_to_entry(struct efa_av *av, fi_ad /* Using a removed fi_addr in a data transfer is an application error * (fi_av(3)); checking it on every operation would cost the lookup that * FI_AV_MAP saves, so only strict mode does. */ - if (OFI_UNLIKELY(efa_env.strict_mode) && !efa_av_map_entry_is_valid(av, entry)) + if (OFI_UNLIKELY(av->strict_mode) && !efa_av_map_entry_is_valid(av, entry)) return NULL; return entry; } diff --git a/prov/efa/src/rdm/efa_rdm_cq.c b/prov/efa/src/rdm/efa_rdm_cq.c index 065030e6b70..cdb148f8826 100644 --- a/prov/efa/src/rdm/efa_rdm_cq.c +++ b/prov/efa/src/rdm/efa_rdm_cq.c @@ -204,10 +204,11 @@ static void efa_rdm_cq_proc_ibv_recv_rdma_with_imm_completion( if (ep->base_ep.util_ep.caps & FI_SOURCE) { /* Only check the explicit AV when writing completions */ - src_addr = efa_rdm_av_reverse_lookup(efa_av, + src_addr = efa_av_fi_addr_to_index(efa_av, + efa_rdm_av_reverse_lookup(efa_av, efa_ibv_cq_wc_read_slid(ibv_cq), efa_ibv_cq_wc_read_src_qp(ibv_cq), - NULL); + NULL)); ret = ofi_cq_write_src(target_cq, NULL, flags, len, NULL, imm_data, 0, src_addr); } else { ret = ofi_cq_write(target_cq, NULL, flags, len, NULL, imm_data, 0); @@ -1105,7 +1106,23 @@ static ssize_t efa_rdm_cq_readfrom(struct fid_cq *cq_fid, void *buf, size_t coun ret = ofi_cq_readfrom(&cq->efa_cq.util_cq.cq_fid, buf, count, src_addr); out: + /* sources are stored as util_av indices; hand FI_AV_MAP fi_addrs out */ + if (ret > 0 && src_addr && cq->map_av) { + for (ssize_t i = 0; i < ret; i++) + src_addr[i] = efa_av_index_to_fi_addr(cq->map_av, src_addr[i]); + } + return ret; +} + +static ssize_t efa_rdm_cq_readerr(struct fid_cq *cq_fid, struct fi_cq_err_entry *buf, + uint64_t flags) +{ + struct efa_rdm_cq *cq = container_of(cq_fid, struct efa_rdm_cq, efa_cq.util_cq.cq_fid.fid); + ssize_t ret; + ret = ofi_cq_readerr(cq_fid, buf, flags); + if (ret > 0 && cq->map_av && buf->src_addr != FI_ADDR_NOTAVAIL) + buf->src_addr = efa_av_index_to_fi_addr(cq->map_av, buf->src_addr); return ret; } @@ -1249,7 +1266,7 @@ static struct fi_ops_cq efa_rdm_cq_ops = { .size = sizeof(struct fi_ops_cq), .read = ofi_cq_read, .readfrom = efa_rdm_cq_readfrom, - .readerr = ofi_cq_readerr, + .readerr = efa_rdm_cq_readerr, .sread = efa_rdm_cq_sread, .sreadfrom = efa_rdm_cq_sreadfrom, .signal = efa_rdm_cq_signal, @@ -1399,46 +1416,6 @@ static int efa_rdm_cq_init_entry_size(struct efa_rdm_cq *cq, return FI_SUCCESS; } -/** - * @brief the application fi_addr of a completion source written through the - * peer CQ - * - * shm and util_srx report the source as the util_av index; with FI_AV_MAP it - * must be the entry's address. Unchanged for FI_AV_TABLE. - */ -static inline fi_addr_t efa_rdm_cq_src_index_to_fi_addr(struct fid_peer_cq *peer_cq, - fi_addr_t src) -{ - struct util_cq *util_cq = peer_cq->fid.context; - struct efa_rdm_cq *cq = container_of(util_cq, struct efa_rdm_cq, efa_cq.util_cq); - - return cq->map_av ? efa_av_index_to_fi_addr(cq->map_av, src) : src; -} - -static ssize_t efa_rdm_cq_peer_write(struct fid_peer_cq *peer_cq, void *context, - uint64_t flags, size_t len, void *buf, - uint64_t data, uint64_t tag, fi_addr_t src) -{ - struct util_cq *util_cq = peer_cq->fid.context; - struct efa_rdm_cq *cq = container_of(util_cq, struct efa_rdm_cq, efa_cq.util_cq); - - return cq->util_owner_ops->write(peer_cq, context, flags, len, buf, data, - tag, efa_rdm_cq_src_index_to_fi_addr(peer_cq, src)); -} - -static ssize_t efa_rdm_cq_peer_writeerr(struct fid_peer_cq *peer_cq, - const struct fi_cq_err_entry *err_entry) -{ - struct util_cq *util_cq = peer_cq->fid.context; - struct efa_rdm_cq *cq = container_of(util_cq, struct efa_rdm_cq, efa_cq.util_cq); - struct fi_cq_err_entry entry = *err_entry; - - entry.src_addr = (err_entry->src_addr == FI_ADDR_NOTAVAIL) - ? FI_ADDR_NOTAVAIL - : efa_rdm_cq_src_index_to_fi_addr(peer_cq, err_entry->src_addr); - return cq->util_owner_ops->writeerr(peer_cq, &entry); -} - /** * @brief translate peer-CQ completion sources for an endpoint on a FI_AV_MAP AV * @@ -1515,13 +1492,10 @@ int efa_rdm_cq_open(struct fid_domain *domain, struct fi_cq_attr *attr, if (ret) goto close_util_cq; - /* wrap the peer CQ writes so completion sources can be translated */ + /* completions keep the util_av index as their source, whoever writes + * them (efa, shm, util_srx); FI_AV_MAP sources are translated when the + * application reads them, see efa_rdm_cq_readfrom() */ cq->map_av = NULL; - cq->util_owner_ops = cq->efa_cq.util_cq.peer_cq->owner_ops; - cq->owner_ops = *cq->util_owner_ops; - cq->owner_ops.write = efa_rdm_cq_peer_write; - cq->owner_ops.writeerr = efa_rdm_cq_peer_writeerr; - cq->efa_cq.util_cq.peer_cq->owner_ops = &cq->owner_ops; ret = efa_cq_open_ibv_cq( attr, rdm_domain->efa_domain.device->ibv_ctx, &cq->efa_cq.ibv_cq, diff --git a/prov/efa/src/rdm/efa_rdm_cq.h b/prov/efa/src/rdm/efa_rdm_cq.h index f9dbac975e2..0af126266f7 100644 --- a/prov/efa/src/rdm/efa_rdm_cq.h +++ b/prov/efa/src/rdm/efa_rdm_cq.h @@ -22,14 +22,12 @@ struct efa_rdm_cq { size_t entry_size; bool need_to_scan_ep_list; /* - * FI_AV_MAP: completions written through the peer CQ (by shm and by - * util_srx) carry the util_av index as their source; map_av turns it into - * the application fi_addr. Set when an endpoint bound to a FI_AV_MAP AV is + * FI_AV_MAP: every completion carries the util_av index as its source; + * map_av turns it into the application fi_addr when the application reads + * it (readfrom/readerr). Set when an endpoint bound to a FI_AV_MAP AV is * enabled on this CQ. */ struct efa_av *map_av; - const struct fi_ops_cq_owner *util_owner_ops; - struct fi_ops_cq_owner owner_ops; }; void efa_rdm_cq_set_map_av(struct efa_rdm_cq *cq, struct efa_av *av); diff --git a/prov/efa/src/rdm/efa_rdm_ope.c b/prov/efa/src/rdm/efa_rdm_ope.c index ab77c00e824..55f0b4a0a27 100644 --- a/prov/efa/src/rdm/efa_rdm_ope.c +++ b/prov/efa/src/rdm/efa_rdm_ope.c @@ -1298,7 +1298,7 @@ void efa_rdm_rxe_report_completion(struct efa_rdm_ope *rxe) rxe->cq_entry.buf, rxe->cq_entry.data, rxe->cq_entry.tag, - efa_rdm_peer_fi_addr(rxe->peer)); + efa_rdm_peer_av_index(rxe->peer)); else ret = ofi_cq_write(rx_cq, rxe->cq_entry.op_context, diff --git a/prov/efa/test/efa_unit_test_av.c b/prov/efa/test/efa_unit_test_av.c index b81f4f8bc90..24af418f619 100644 --- a/prov/efa/test/efa_unit_test_av.c +++ b/prov/efa/test/efa_unit_test_av.c @@ -532,12 +532,13 @@ static void test_av_map_insert_remove_lookup_impl(void **state, char *fabric_nam /* a removed FI_AV_MAP fi_addr is rejected, not dereferenced */ assert_int_equal(fi_av_lookup(map_av, fi_addr, &raw_addr_out, &raw_addr_len), -FI_EINVAL); assert_int_equal(fi_av_remove(map_av, &fi_addr, 1, 0), -FI_EINVAL); - /* data transfers only check it in strict mode */ - strict_mode_orig = efa_env.strict_mode; - efa_env.strict_mode = 1; + /* data transfers only check it in strict mode, which the AV samples + * when it is opened */ + strict_mode_orig = av->strict_mode; + av->strict_mode = true; assert_null(efa_av_addr_to_entry(av, fi_addr)); assert_int_equal(efa_av_fi_addr_to_index(av, fi_addr), FI_ADDR_NOTAVAIL); - efa_env.strict_mode = strict_mode_orig; + av->strict_mode = strict_mode_orig; err = fi_close(&map_av->fid); assert_int_equal(err, 0);