Skip to content
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
754dd7f
UCT/IB/MLX5/RC: Implement put_sgl_zcopy
michal-shalev Jul 19, 2026
88dbcff
UCT/IB/MLX5/RC: Reject strided put_sgl_zcopy
michal-shalev Jul 21, 2026
d8205aa
Merge branch 'master' into mlx5-rc-put-sgl-zcopy
michal-shalev Jul 21, 2026
ec2c4b0
UCT/IB/MLX5/RC: Validate lengths before consuming fence state in put_…
michal-shalev Jul 21, 2026
572312d
UCT/IB/MLX5/RC: Cap put_sgl_zcopy count
michal-shalev Jul 21, 2026
0e7195a
UCT/IB/MLX5/RC: Cap put_sgl_zcopy count by QP and BB limits
michal-shalev Jul 21, 2026
fca8728
Merge branch 'master' into mlx5-rc-put-sgl-zcopy
michal-shalev Jul 22, 2026
685abda
UCT/IB/MLX5: Fix dead-store warning for max_count in put_sgl_zcopy
michal-shalev Jul 22, 2026
276ceb3
Merge branch 'master' into mlx5-rc-put-sgl-zcopy
michal-shalev Jul 22, 2026
63756a4
UCT/IB/MLX5: Validate put_sgl_zcopy lengths before resource checks
michal-shalev Jul 24, 2026
9128ce3
UCT/IB/MLX5: Use fence state for put_sgl_zcopy rkey selection
michal-shalev Jul 24, 2026
9538693
UCT/IB/MLX5: Limit PUT SGL zcopy to half the TX queue
michal-shalev Jul 24, 2026
0c19cfa
UCT/IB/RC: Centralize TX CQ capacity calculation
michal-shalev Jul 24, 2026
e7c4d75
UCT/IB/MLX5: Cap PUT SGL zcopy by usable TX CQ credits
michal-shalev Jul 24, 2026
d05605f
UCT/IB/MLX5: Cap PUT SGL count by usable TX CQ credits
michal-shalev Jul 26, 2026
1af61f3
UCT/RC: Add per-EP CQ reserve for multi-CQE operations
michal-shalev Jul 26, 2026
60c8d46
UCT/IB/MLX5: Handle zero-count PUT SGL operations
michal-shalev Jul 26, 2026
cc94d61
Merge branch 'master' into mlx5-rc-put-sgl-zcopy
michal-shalev Jul 26, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion src/uct/ib/mlx5/rc/rc_mlx5.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright (c) NVIDIA CORPORATION & AFFILIATES, 2001-2019. ALL RIGHTS RESERVED.
* Copyright (c) NVIDIA CORPORATION & AFFILIATES, 2001-2026. ALL RIGHTS RESERVED.
*
* See file LICENSE for terms.
*/
Expand Down Expand Up @@ -123,6 +123,14 @@ ucs_status_t uct_rc_mlx5_base_ep_put_zcopy(uct_ep_h tl_ep, const uct_iov_t *iov,
uct_rkey_t rkey,
uct_completion_t *comp);

ucs_status_t
uct_rc_mlx5_base_ep_put_sgl_zcopy(uct_ep_h tl_ep, void * const *buffers,
const size_t *lengths, uct_mem_h const *memhs,
const uint64_t *remote_addrs,
uct_rkey_t const *rkeys, const size_t *counts,
const size_t *strides, size_t count,
uct_completion_t *comp);

ucs_status_t
uct_rc_mlx5_base_ep_get_bcopy(uct_ep_h tl_ep, uct_unpack_callback_t unpack_cb,
void *arg, size_t length, uint64_t remote_addr,
Expand Down
10 changes: 10 additions & 0 deletions src/uct/ib/mlx5/rc/rc_mlx5.inl
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,16 @@
uct_rc_mlx5_iface_common_t)


static UCS_F_ALWAYS_INLINE size_t
uct_rc_mlx5_base_put_sgl_zcopy_max_count(uct_rc_mlx5_iface_common_t *iface)
{
size_t max_count = ucs_min((size_t)UCT_RC_MLX5_PUT_SGL_ZCOPY_MAX_COUNT,
iface->super.config.tx_qp_len);

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.

Suggested change
max_count = ucs_min(max_count, (size_t)(iface->super.config.tx_cq_len - 2));

In src/uct/ib/rc/base/rc_iface.c:590 it says:

/* Prevent title CQE overwriting */
    self->tx.cq_available       = tx_cq_size - 2;

so the 2 reserved spaces need to be taken into account here too

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch. By default, the SQ length is 256 and the TX CQ length is 4096, so the SQ is normally exhausted first. However, a user can configure a smaller CQ using UCX_RC_TX_CQ_LEN and expose a corner case where an SGL requires more credits than are currently available while cq_available is still nonzero, causing pending_add() to return BUSY and spin without progress.

However, your suggestion of hardcoding tx_cq_len - 2 would duplicate the reserved-CQ magic number. Instead, I added a shared helper as the single source of truth and a per-EP CQ reservation, similar to txqp_reserve in #11361, and ran the SGL gtests to confirm the fix.

return ucs_min(max_count, iface->tx.bb_max);
}


static UCS_F_ALWAYS_INLINE void
uct_rc_mlx5_ep_fence_put(uct_rc_mlx5_iface_common_t *iface, uct_ib_mlx5_txwq_t *txwq,
uct_rkey_t *rkey, uint64_t *addr, uint16_t offset,
Expand Down
5 changes: 4 additions & 1 deletion src/uct/ib/mlx5/rc/rc_mlx5_common.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright (c) NVIDIA CORPORATION & AFFILIATES, 2001-2019. ALL RIGHTS RESERVED.
* Copyright (c) NVIDIA CORPORATION & AFFILIATES, 2001-2026. ALL RIGHTS RESERVED.
*
* See file LICENSE for terms.
*/
Expand Down Expand Up @@ -134,6 +134,9 @@ enum {
UCT_RC_MLX5_CQE_APP_OP_TM_CONSUMED_MSG = 0xA
};

#define UCT_RC_MLX5_PUT_SGL_ZCOPY_MAX_COUNT 128


#define UCT_RC_MLX5_RMA_MAX_IOV(_av_size) \
((UCT_IB_MLX5_MAX_SEND_WQE_SIZE - ((_av_size) + \
sizeof(struct mlx5_wqe_raddr_seg) + sizeof(struct mlx5_wqe_ctrl_seg))) / \
Expand Down
115 changes: 114 additions & 1 deletion src/uct/ib/mlx5/rc/rc_mlx5_ep.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright (c) NVIDIA CORPORATION & AFFILIATES, 2001-2019. ALL RIGHTS RESERVED.
* Copyright (c) NVIDIA CORPORATION & AFFILIATES, 2001-2026. ALL RIGHTS RESERVED.
*
* See file LICENSE for terms.
*/
Expand Down Expand Up @@ -175,6 +175,119 @@ ucs_status_t uct_rc_mlx5_base_ep_put_zcopy(uct_ep_h tl_ep, const uct_iov_t *iov,
return status;
}

ucs_status_t
uct_rc_mlx5_base_ep_put_sgl_zcopy(uct_ep_h tl_ep, void * const *buffers,
const size_t *lengths, uct_mem_h const *memhs,
const uint64_t *remote_addrs,
uct_rkey_t const *rkeys, const size_t *counts,
const size_t *strides, size_t count,
uct_completion_t *comp)
{
UCT_RC_MLX5_BASE_EP_DECL(tl_ep, iface, ep);
uct_ib_mlx5_txwq_t *txwq = &ep->tx.wq;
size_t total = 0;
struct mlx5_wqe_ctrl_seg *ctrl = NULL;
struct mlx5_wqe_raddr_seg *raddr;
struct mlx5_wqe_data_seg *dptr;
size_t wqe_size, i;
uint8_t fm_ce_se, fence_flag;
uint16_t sn, pi, res_count;
uint64_t addr;
uct_rkey_t rkey;
void *curr;
size_t UCS_V_UNUSED max_count;

max_count = uct_rc_mlx5_base_put_sgl_zcopy_max_count(iface);
UCT_CHECK_PARAM(count <= max_count,
"put_sgl_zcopy count(%zu) should be limited by %zu", count,
max_count);
ucs_assert(ep->super.flags & UCT_RC_EP_FLAG_CONNECTED);
ucs_assert(count > 0);

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.

In release the function will continue executing and underflow on count - 1.
I think it's best to check for count == 0 and return UCS_OK (probably best after the UCS_ERR_UNSUPPORTED check)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Used UCT_SKIP_ZERO_LENGTH(count) to be consistent with the sge_cnt check in RC verbs send path (rc_verbs_ep.c).


/* TODO: add strided elements support */
if (ucs_unlikely((counts != NULL) || (strides != NULL))) {
ucs_error("put_sgl_zcopy does not support strided elements");
return UCS_ERR_UNSUPPORTED;
}

if (iface->super.tx.cq_available < (int)count) {
UCS_STATS_UPDATE_COUNTER(iface->super.stats,
UCT_RC_IFACE_STAT_NO_CQE, 1);
UCS_STATS_UPDATE_COUNTER(ep->super.super.stats,
UCT_EP_STAT_NO_RES, 1);
return UCS_ERR_NO_RESOURCE;
}

UCT_RC_CHECK_NUM_RDMA_READ_RET(&iface->super, UCS_ERR_NO_RESOURCE);
Comment thread
bosilca marked this conversation as resolved.
Resolved
UCT_RC_CHECK_TXQP_VALUE_RET(&iface->super, &ep->super,
Comment thread
ColinNV marked this conversation as resolved.
Resolved
UCS_ERR_NO_RESOURCE, count - 1);

/* Validate all lengths before consuming the fence state */
for (i = 0; i < count; i++) {
UCT_CHECK_LENGTH(lengths[i], 0, UCT_IB_MAX_MESSAGE_SIZE,
"put_sgl_zcopy");
}
Comment on lines +216 to +219

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.

It would be better to do this validity check before line 213, because otherwise it might return UCS_ERR_NO_RESOURCE on an operation that would never succeed anyway

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done


wqe_size = sizeof(*ctrl) + sizeof(*raddr) + sizeof(*dptr);
sn = txwq->sw_pi;

ucs_assert(!(txwq->flags & UCT_IB_MLX5_TXWQ_FLAG_FAILED));
ucs_assert(ucs_div_round_up(wqe_size, MLX5_SEND_WQE_BB) == 1);

pi = sn;
curr = txwq->curr;

fence_flag = uct_rc_ep_fm(&iface->super, &txwq->fi, 1) ?
Comment thread
ColinNV marked this conversation as resolved.
Resolved
iface->config.put_fence_flag : 0;

for (i = 0; i < count; i++) {
fm_ce_se = ((i == 0) ? fence_flag : 0) |
((i == count - 1) ? MLX5_WQE_CTRL_CQ_UPDATE : 0);
ctrl = curr;

uct_ib_mlx5_set_ctrl_seg(ctrl, pi, MLX5_OPCODE_RDMA_WRITE, 0,
txwq->super.qp_num, fm_ce_se, 0, wqe_size);

addr = remote_addrs[i];
if (fence_flag != 0) {

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.

should this check the return value of uct_rc_ep_fm() instead of fence_flag?
similar to uct_rc_mlx5_ep_fence_put()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch. The return value of uct_rc_ep_fm() is indeed the correct condition for choosing between the atomic and direct rkey paths. However, unlike uct_rc_mlx5_ep_fence_put(), this function handles multiple WQEs, so it should be called only once before the loop because it updates fence_beat, its result is then reused for the entire batch.

rkey = uct_ib_resolve_atomic_rkey(
rkeys[i], ep->super.atomic_mr_offset, &addr);
} else {
rkey = uct_ib_md_direct_rkey(rkeys[i]);
}

raddr = uct_ib_mlx5_txwq_wrap_none(txwq, ctrl + 1);
uct_ib_mlx5_ep_set_rdma_seg(raddr, addr, rkey);

dptr = uct_ib_mlx5_txwq_wrap_none(txwq, raddr + 1);
uct_ib_mlx5_set_data_seg(dptr, buffers[i], lengths[i],
uct_ib_memh_get_lkey(memhs[i]));

curr = UCS_PTR_BYTE_OFFSET(ctrl, MLX5_SEND_WQE_BB);
curr = uct_ib_mlx5_txwq_wrap_exact(txwq, curr);
pi++;
total += lengths[i];
}

res_count = pi - 1 - txwq->prev_sw_pi;
txwq->prev_sw_pi += res_count;
txwq->sw_pi = pi;
txwq->curr = curr;
txwq->sig_pi = txwq->prev_sw_pi;

uct_rc_txqp_posted(&ep->super.txqp, &iface->super, res_count, 1);
Comment thread
bosilca marked this conversation as resolved.
Resolved
uct_ib_mlx5_txwq_ring_doorbell(txwq, ctrl, txwq->sw_pi, 1);

uct_rc_txqp_add_send_comp(&iface->super, &ep->super.txqp,
uct_rc_ep_send_op_completion_handler, comp, sn,
UCT_RC_IFACE_SEND_OP_FLAG_ZCOPY, NULL, 0, total);

UCT_TL_EP_STAT_OP(&ep->super.super, PUT, ZCOPY, total);
uct_rc_ep_enable_flush_remote(&ep->super);

return UCS_INPROGRESS;
}

ucs_status_t
uct_rc_mlx5_base_ep_get_bcopy(uct_ep_h tl_ep, uct_unpack_callback_t unpack_cb,
void *arg, size_t length, uint64_t remote_addr,
Expand Down
36 changes: 32 additions & 4 deletions src/uct/ib/mlx5/rc/rc_mlx5_iface.c
Original file line number Diff line number Diff line change
Expand Up @@ -1012,13 +1012,41 @@ static UCS_CLASS_DEFINE_NEW_FUNC(uct_rc_mlx5_iface_t, uct_iface_t, uct_md_h,
static UCS_CLASS_DEFINE_DELETE_FUNC(uct_rc_mlx5_iface_t, uct_iface_t);

static ucs_status_t
uct_rc_mlx5_iface_query_v2(uct_iface_h UCS_V_UNUSED iface,
uct_rc_mlx5_ep_put_sgl_zcopy(uct_ep_h ep, void * const *buffers,
const size_t *lengths, uct_mem_h const *memhs,
const uint64_t *remote_addrs,
uct_rkey_t const *rkeys, const size_t *counts,
const size_t *strides, size_t count,
uct_completion_t *comp)
{
ucs_status_t status;

status = uct_ib_mlx5_ext_ep_put_sgl_zcopy(ep, buffers, lengths, memhs,
remote_addrs, rkeys, counts,
strides, count, comp);
if (status == UCS_ERR_UNSUPPORTED) {
status = uct_rc_mlx5_base_ep_put_sgl_zcopy(ep, buffers, lengths, memhs,
remote_addrs, rkeys, counts,
strides, count, comp);
}

return status;
}

static ucs_status_t
uct_rc_mlx5_iface_query_v2(uct_iface_h tl_iface,
uct_iface_attr_v2_t *iface_attr)
{
size_t max_sgl = uct_ib_mlx5_ext_max_put_sgl_zcopy_count();
uct_rc_mlx5_iface_common_t *iface = ucs_derived_of(tl_iface,
uct_rc_mlx5_iface_common_t);
size_t max_sgl = uct_ib_mlx5_ext_max_put_sgl_zcopy_count();

if (max_sgl == 0) {
max_sgl = uct_rc_mlx5_base_put_sgl_zcopy_max_count(iface);
}

if (iface_attr->field_mask & UCT_IFACE_ATTR_FIELD_CAP_FLAGS) {
iface_attr->cap.flags = (max_sgl > 0) ? UCT_IFACE_FLAG_V2_PUT_SGL_ZCOPY : 0;
iface_attr->cap.flags = UCT_IFACE_FLAG_V2_PUT_SGL_ZCOPY;
}

if (iface_attr->field_mask & UCT_IFACE_ATTR_FIELD_MAX_PUT_SGL_ZCOPY_COUNT) {
Expand All @@ -1040,7 +1068,7 @@ static uct_rc_iface_ops_t uct_rc_mlx5_iface_ops = {
.iface_is_reachable_v2 = uct_rc_mlx5_iface_is_reachable_v2,
.ep_is_connected = uct_rc_mlx5_base_ep_is_connected,
.ep_get_device_ep = (uct_ep_get_device_ep_func_t)ucs_empty_function_return_unsupported,
.ep_put_sgl_zcopy = uct_ib_mlx5_ext_ep_put_sgl_zcopy
.ep_put_sgl_zcopy = uct_rc_mlx5_ep_put_sgl_zcopy
},
.create_cq = uct_rc_mlx5_iface_common_create_cq,
.destroy_cq = uct_rc_mlx5_iface_common_destroy_cq,
Expand Down