UCT/IB: Support relaxed-only memory keys - #11649
Conversation
b9d83f5 to
6fcde5e
Compare
|
🤖 Starting review — findings will be posted here when done. |
Signed-off-by: Roie Danino <rdanino@nvidia.com>
Signed-off-by: Roie Danino <rdanino@nvidia.com>
6aa0a43 to
a5fb08f
Compare
roiedanino
changed the title
roiedanino
changed the title
Signed-off-by: Roie Danino <rdanino@nvidia.com>
a5fb08f to
e40d287
Compare
| uct_ib_device_name(&md->dev), | ||
| is_required ? "firmware" : | ||
| "IB_PCI_RELAXED_ORDERING=yes"); | ||
| return is_required ? UCS_ERR_IO_ERROR : UCS_ERR_UNSUPPORTED; |
There was a problem hiding this comment.
why IO_ERROR? IMO this branch always should return UNSUPPORTED
There was a problem hiding this comment.
It can't be UCS_ERR_UNSUPPORTED, because it would mean UCX will try to fall back to mlx5 dv / ibv instead of failing as expected (mlx5dv / ibv won't be able to query fw for that limitation)
There was a problem hiding this comment.
but dv / ibv also !have_relaxed_order -> UCS_ERR_UNSUPPORTED, fallback fails too, UCP fails
do I miss anything?
There was a problem hiding this comment.
not when the requirement comes from firmware, when the env is set to auto, it's only being handled in devx
There was a problem hiding this comment.
I see, the error goes from FW due to incompatible parameter set by user (IB_PCI_RELAXED_ORDERING=yes), right? can we move this check to IB_PCI_RELAXED_ORDERING=yes since it's unsupported by FW
There was a problem hiding this comment.
is_required means FW supports only relaxed-order MKEYs. If false, relaxed ordering is not mandatory, support is determined separately by is_supported
There was a problem hiding this comment.
does it mean that we do not support IB_PCI_RELAXED_ORDERING=yes if is_required (FW supports only relaxed-order MKEYs)? if so, then need to move this FW requirement check to the place where we parse IB_PCI_RELAXED_ORDERING
There was a problem hiding this comment.
No, it doesn't mean that, the error occurs only when relaxed-only mode is required but IBV_ACCESS_RELAXED_ORDERING is unavailable (contradiction).
There was a problem hiding this comment.
ah.. ok, could you pls simplify ucs_error(...)? maybe break on 2 message under different conditions.
Signed-off-by: Roie Danino <rdanino@nvidia.com>
Signed-off-by: Roie Danino <rdanino@nvidia.com>
Signed-off-by: Roie Danino <rdanino@nvidia.com>
Signed-off-by: Roie Danino <rdanino@nvidia.com>
| cmd_hca_cap, cap, mkc_order_write_after_write_ro_only); | ||
| ucs_debug("%s: mkey ordering: read-after-write=%d relaxed-only=%d", | ||
| uct_ib_device_name(dev), | ||
| UCT_IB_MLX5DV_GET(cmd_hca_cap, cap, |
Signed-off-by: Roie Danino <rdanino@nvidia.com>
|
@evgeny-leksikov @gleon99 Is this one good to go? |
4 participants
What?
Add support for devices that require relaxed-only memory keys.
yes,no,try, andautovalues forUCX_IB_PCI_RELAXED_ORDERING.yesrequire relaxed ordering, omit strict-order companion keys and rkeys, and return an error when relaxed ordering is unavailable.tryas the best-effort mode that retains strict-order companion keys when available.autoas the default and honor a firmware relaxed-only requirement before considering CPU preference.nowhen the device requires relaxed-only operation.Why?
Some devices do not support strong-order memory keys. Creating a strict-order companion key on those devices would fail memory registration and prevent UCX from operating.
UCX must recognize this requirement, avoid creating incompatible keys, and reject configuration choices that cannot be satisfied.
How?
Track whether relaxed ordering is enabled and whether it is mandatory as separate memory-domain states.
When relaxed ordering is mandatory, apply relaxed ordering to required registrations, use the default key for atomic operations, omit strict-order companion keys and exported rkeys, and gate paths that depend on strict-order keys. When it is optional, preserve the existing dual-key behavior.