[jeynmann]

What?

Associate the completion with the last WQE in an SGL PUT so partial recovery cannot complete the operation prematurely.

Why?

During FT, comp could called before all WQEs consumed, since it's associated with 1st WQE.

How?

Remove the unrelated RDMA read resource check from the PUT path.

[svc-nvidia-pr-review]

🤖 Starting review — findings will be posted here when done.

[svc-nvidia-pr-review]

Change 1 — sn → txwq->sig_pi in uct_rc_txqp_add_send_comp: Correct and an improvement. Only the last WQE in the batch carries MLX5_WQE_CTRL_CQ_UPDATE, and its completion reports hw_ci = ntohs(cqe->wqe_counter) equal to that WQE's first BB index (prev_sw_pi after the loop). txwq->sig_pi is set to exactly txwq->prev_sw_pi, so UCS_CIRCULAR_COMPARE16(sig_pi, <=, hw_ci) matches on the right CQE. This aligns the SGL path with the signaled-PI convention used by uct_rc_mlx5_common_post_send, _flush, and _post_check. Removing the now-unused sn local is clean.

[svc-nvidia-pr-review]

Residual coverage note: there is no test change in the diff. Since this touches completion-PI matching and a resource-check removal, confirm existing put_sgl_zcopy coverage (the extended/plugin path plus any RC mlx5 SGL tests) exercises multi-element batches and the no-resource path; if not, a focused regression test on multi-element completion and the pending/ordering case would be warranted.

[svc-nvidia-pr-review]

🤖 Starting review — findings will be posted here when done.

[svc-nvidia-pr-review]

Residual coverage note: The diff contains no test change. This is a completion-PI semantics change on the SGL zcopy path. Existing coverage exists in test/gtest/uct/v2/test_uct_sgl.cc (and the cuda_ipc device test), which exercise uct_ep_put_sgl_zcopy. Worth confirming those tests run multi-element batches with completion callbacks against the RC mlx5 path so the sig_pi-based completion is actually exercised; if the gtest only covers single-element or the plugin/ext path, the base multi-WQE completion change would be untested.

[svc-nvidia-pr-review]

🤖 Starting review — findings will be posted here when done.

[svc-nvidia-pr-review]

Completion PI change looks correct: switching the completion sn from sw_pi (start-of-batch PI) to txwq->sig_pi (the PI of the last, signaled WQE, S + count - 1) matches the canonical convention already used by the flush and DC paths, and it is the value the CQE's wqe_counter reports for the batch's single signaled WQE, so the completion fires exactly on that CQE. sig_pi is copied by value into op->sn, so a later post overwriting txwq->sig_pi cannot affect an already-queued op. No issue here.

Residual coverage note: the diff contains no test change. This is a completion-PI semantics change plus a resource-check edit on the multi-WQE put_sgl_zcopy path. Existing coverage in test/gtest/uct/v2/test_uct_sgl.cc (and the cuda_ipc device test) exercises uct_ep_put_sgl_zcopy; it is worth confirming those tests run multi-element batches with a completion callback against the RC mlx5 base path so the sig_pi-based completion is actually exercised end to end. If the gtest only covers single-element or the plugin/ext path, the multi-WQE completion change stays untested.

[svc-nvidia-pr-review]

🤖 Starting review — findings will be posted here when done.

[svc-nvidia-pr-review]

Residual coverage note (already raised previously): the diff still contains no test change for the multi-WQE sig_pi completion path.

[svc-nvidia-pr-review]

🤖 Starting review — findings will be posted here when done.

[svc-nvidia-pr-review]

🤖 Review complete — no issues found.