issue: 5094361 Enable relaxed ordering for XLIO memory registrations - #656
issue: 5094361 Enable relaxed ordering for XLIO memory registrations#656BasharRadya wants to merge 1 commit into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (11)
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review. 📝 WalkthroughWalkthroughXLIO adds the ChangesRelaxed Ordering Support
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: 🔵 Low · up to The change may apply relaxed ordering to TX user-buffer registrations even though that path does not depend on the affected ordering behavior, creating a bounded risk of an unnecessary runtime behavior change. The PR is otherwise mergeable with explicit owner follow-up. Sequence Diagram(s)sequenceDiagram
participant ConfigurationRegistry
participant mce_sys_var
participant mem_reg
participant ibv_reg_mr
ConfigurationRegistry->>mce_sys_var: Initialize enable_relaxed_ordering
mce_sys_var->>mem_reg: Provide relaxed-ordering setting
mem_reg->>ibv_reg_mr: Register memory with access flags
ibv_reg_mr-->>mem_reg: Return registration result
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 8 files. (3 skipped: 3 unsupported.) ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR adds an opt-in relaxed-ordering mode for XLIO memory registrations while retaining strict-ordering requests by default.
Confidence Score: 5/5The PR appears safe to merge because no blocking failure remains in the eligible follow-up review scope. No blocking failure remains. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[Legacy environment or structured config] --> B[enable_relaxed_ordering]
B --> C{Enabled and verbs flag available?}
C -- Yes --> D[Add IBV_ACCESS_RELAXED_ORDERING]
C -- No --> E[Keep existing access flags]
D --> F[ibv_reg_mr]
E --> F
Reviews (3): Last reviewed commit: "issue: 5094361 Enable relaxed ordering f..." | Re-trigger Greptile |
pasis
left a comment
There was a problem hiding this comment.
Looks good in general. Small changes needed.
| - [`core.syscall.fork_support`](#coresyscallfork_support) — Enable fork support | ||
| - [`core.syscall.sendfile_cache_limit`](#coresyscallsendfile_cache_limit) — Sendfile byte limit | ||
| - **[HARDWARE_FEATURES](#hardware_features)** | ||
| - [`hardware_features.memory_registration.relaxed_ordering`](#hardware_featuresmemory_registrationrelaxed_ordering) — Enable relaxed ordering for memory registration |
There was a problem hiding this comment.
[comment] Please see whether we already have existing category related to memory registration, PCI or similar. Create new category only if the new config doesn't fit anything existing.
There was a problem hiding this comment.
no related category, i can simply remove the memory_Registeration and put relaxed oredering underas hardware features
WDYT?
Request IBV_ACCESS_RELAXED_ORDERING for XLIO-owned host memory registrations so per_mkey mode can improve performance without a device-wide mlxconfig change. Expose the request through XLIO_RELAXED_ORDERING and the structured hardware configuration, disabled by default to preserve existing MR access flags. Report when the request cannot be applied and distinguish XLIO's request from device-wide force_relaxed behavior. Signed-off-by: Bashar Abdelgafer <babdelgafer@nvidia.com>
d0dcce2 to
2ed87a0
Compare
Request IBV_ACCESS_RELAXED_ORDERING for XLIO-owned host memory registrations so per_mkey mode can improve performance without a device-wide mlxconfig change.
Keep the runtime switch disabled by default to preserve existing ordering behavior. Older verbs builds continue with regular ordering, while modern libibverbs treats the flag as optional when kernel support is unavailable.
Document both configuration paths and their user-visible behavior.
Description
Please provide a summary of the change.
What
Subject: what this PR is doing in one line.
Why ?
Justification for the PR. If there is existing issue/bug please reference.
How ?
It is optional but for complex PRs please provide information about the design,
architecture, approach, etc.
Change type
What kind of change does this PR introduce?
Check list
Summary by CodeRabbit
New Features
hardware_features.memory_registration.relaxed_orderingorXLIO_ENABLE_RELAXED_ORDERING.Documentation
Diagnostics