Fix Libfabric MR caching issues #13327
Merged
+202
−95
Conversation
This file contains hidden or 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
This was an optimization around a bug in the EFA provider. The EFA provider shouldn't be caching explicit registrations anyway, so avoiding the double cache is silly (and breaks when EFA fixes the explicit registration cache bug). Signed-off-by: Brian Barrett <bbarrett@amazon.com>
The OFI MTL exports a memory monitor to Libfabric (so that OMPI's patcher wins), but in cases where OB1 is directly selected, that code won't run. So make sure to also configure Libfabric so that it won't try to use a suboptimial memory monitor in the case that only the OFI BTL is used. Signed-off-by: Brian Barrett <bbarrett@amazon.com>
Rather than use the CXI provider name to disable explicit hmem registration, use the FI_MR_HMEM flag. Signed-off-by: Brian Barrett <bbarrett@amazon.com>
The OFI MTL was creating a registration for every operation that used HMEM when FI_MR_HMEM is required. This is really performance inefficient, since creating registrations is expensive. So stick a rcache in front of the registrations. Signed-off-by: Brian Barrett <bbarrett@amazon.com>
|
@hppritcha can you give this a whirl on a CXI system with |
hppritcha
approved these changes
Jul 10, 2025
shijin-aws
approved these changes
Jul 14, 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.
Fix a set of bugs in both the OFI BTL and OFI MTL around caching MRs. The Libfabric EFA provider used to (erroneously) cache explicitly created MRs and will stop doing so in Libfabric 2.2. This caused a performance regression in both the OFI MTL (with HMEM) and BTL (always) over EFA because bad behaviors had snuck in OMPI w.r.t assuming the provider caches MRs. So we stop disabling the BTL rcache for EFA and add an rcache for HMEM MRs for the OFI MTL. The OFI MTL requires the provider not require FI_MR_LOCAL, so we don't need to worry about caching general MRs there.
While I was fixing that, noticed two other issues in the OFI code that I cleaned up. First, we should use the state of FI_MR_HMEM instead of a provider name for avoiding creating HMEM MRs in the OFI MTL. Second, in the case that the OFI BTL is used without the OFI MTL, we were not properly coupling the OMPI memory monitor with the Libfabric memory monitor.