Skip to content

prov/efa: Translate FI_AV_MAP completion sources only when they are read - #13197

Open
yinliaws wants to merge 2 commits into
ofiwg:mainfrom
yinliaws:efa-map-lazy-src-translate
Open

yinliaws wants to merge 2 commits into
ofiwg:mainfrom
yinliaws:efa-map-lazy-src-translate

Conversation

@yinliaws

Copy link
Copy Markdown
Contributor

The FI_AV_MAP support for the efa fabric translated the source of every completion written through the peer CQ (by shm and util_srx), from the util_av index to the application fi_addr, as each completion was written. The write wrapper was installed on every RDM CQ, FI_AV_TABLE ones included. The translation is an entry-table lookup that starts with a read barrier, so every intra-node completion paid for it whether or not the application asked for a source.

The FI_AV_MAP support wrapped the peer CQ write ops so every completion
written by shm or util_srx had its source translated from the util_av
index to the application fi_addr as it was written. The wrapper was
installed on every RDM CQ, FI_AV_TABLE ones included, and the
translation is an entry-table lookup with a read barrier, so every
intra-node completion paid for it whether or not the application asked
for a source. Intra-node small-message rates dropped by up to 8%:
fi_rma_pingpong writedata 8-16 B on p4d went from 0.38 to 0.42 us with
FI_AV_TABLE, and Open MPI osu_bw/osu_mbw_mr 1-16 B on c7gn, which uses
FI_AV_MAP, lost 8%.

Keep every completion source as the util_av index instead - efa's own
FI_SOURCE completions included - and translate it in readfrom and
readerr, only for an endpoint on a FI_AV_MAP AV and only when the
application passes a source buffer. FI_AV_TABLE and readers that do
not ask for sources, such as Open MPI, do no translation at all. This
restores the FI_AV_TABLE numbers and leaves Open MPI on FI_AV_MAP
within 2% of them.

Signed-off-by: Yin Li <yinliq@amazon.com>
efa_av_addr_to_entry() is on every FI_AV_MAP send and directed
receive, and it read the global efa_env.strict_mode each time to decide
whether to validate the fi_addr. That is a load from a cache line the
data path does not otherwise touch; on c7gn intra-node osu_bw 16 B it
cost about 1% against FI_AV_TABLE. Copy the flag into the AV when it is
opened, next to the AV type the same lookup already reads.

Signed-off-by: Yin Li <yinliq@amazon.com>
* remote endpoint. These do not have a packet context, nor do they have a
* connid available.
*
* @param[in] ibv_cq_ex extended ibv cq

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: I'm not a fan of commit messages that have specific performance numbers and stats. I think just saying its a performance improvement is fine

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

not a big deal though

Comment thread prov/efa/src/rdm/efa_rdm_cq.c Resolved

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants