Repository navigation
UCT/IB/MLX5: support short dm in purge outstanding - #12058
Conversation
|
🤖 Starting review — findings will be posted here when done. |
|
🤖 Starting review — findings will be posted here when done. |
| void uct_rc_ep_short_dm_desc_handler(uct_rc_iface_send_op_t *op, | ||
| const void *UCS_V_UNUSED resp) | ||
| { | ||
| ucs_mpool_put(op); |
There was a problem hiding this comment.
ucs_mpool_put_inline
|
🤖 Starting review — findings will be posted here when done. |
|
🤖 CI Triage Agent — TL;DR: The ASan "roce on worker 0" job did not actually fail a test — it was externally cancelled mid- Full analysisSummary: Azure Pipelines job "roce on worker 0" (AddressSanitizer) terminated with Root cause: External cancellation, not a test or build failure. Evidence from the log:
The abrupt cancellation at a point of healthy progress, with the pipeline then reporting Implicated commit: none — commit File: not applicable — failure occurred in CI orchestration, not in a source file. Last log position: Suggested fix: No code change required. Re-run / re-queue the "AddressSanitizer roce on worker 0" job and let it reach the gtest stage before triaging. Before re-running, confirm whether a newer commit was pushed to Related: #11932 ("CI: Do not start PR pipelines for draft pull requests") is relevant background on PR-pipeline start/cancel behavior, though not a direct duplicate. No existing issue matches this cancellation signature.
|
|
🤖 Starting review — findings will be posted here when done. |
| ucs_assert(desc->super.buffer == | ||
| UCS_PTR_BYTE_OFFSET(iface->dm.dm->start_va, | ||
| (uintptr_t)wqe_buffer)); | ||
| return desc->super.buffer; |
There was a problem hiding this comment.
mlx5 DM reads require 4-byte accesses. Maybe copy it to host mem w aligned reads before parsing or exposing it?
There was a problem hiding this comment.
Addressed. Do we have any documentation indicating that the read must be by 4 bytes?
The gtest passed without this change on CX6 and BF3.
|
🤖 Starting review — findings will be posted here when done. |
|
🤖 CI Triage Agent — TL;DR: The Coverity Full analysisSummary: Azure Pipelines job "Coverity release on coverity_rh7" (build 138717) failed at the end of Root cause: A secondary real issue on the same lines: Implicated commit: File: Suggested fix: Make the stack-vs-heap decision explicit with a single local pointer, so void *dm_data_heap = NULL; /* add to declarations */
...
#if HAVE_IBV_DM
if (iface->dm.dm != NULL) {
dm_data_len = ucs_align_up(iface->dm.seg_len, sizeof(uint64_t));
if (dm_data_len > UCS_ALLOCA_MAX_SIZE) {
dm_data_heap = ucs_malloc(dm_data_len, "rc_purge_dm");
if (dm_data_heap == NULL) {
return UCS_ERR_NO_MEMORY;
}
callback_data.dm_data = dm_data_heap;
} else {
callback_data.dm_data = ucs_alloca(dm_data_len);
}
} else
#endif
{ dm_data_len = 0; callback_data.dm_data = NULL; }
...
ucs_free(dm_data_heap); /* replaces ucs_free_on_stack() at line 1819 */Given Related: PR #12058 (this change); prior commits in the same series:
|
|
🤖 Starting review — findings will be posted here when done. |
|
🤖 Starting review — findings will be posted here when done. |
|
🤖 Starting review — findings will be posted here when done. |
|
🤖 Starting review — findings will be posted here when done. |
|
🤖 CI Triage Agent — TL;DR: The GPU gtest job failed on a single UCP protocol-selection test, Full analysisSummary: Root cause: The test runs with Note: the Azure API only returns the tail of the job log, so the exact Implicated commit: Not the PR under test. The test and its gating come from the CUDA staging-force series by Nathan Bellalou — File: Suggested fix:
Related: PR #12058 (the build under test, unrelated), PR #11641 / commits |
|
🤖 Starting review — findings will be posted here when done. |
|
@jeynmann please follow up on CI, merge. |
Head branch was pushed to by a user without write access
|
🤖 Starting review — findings will be posted here when done. |
|
🤖 CI Triage Agent — TL;DR: Full analysisSummary: Azure job "roce on worker 1" failed with Root cause: Timestamp analysis of the stage log shows continuous activity up to That is the gtest watchdog ( The operation that hung is identifiable from the spec table order in This points squarely at the branch under test.
If an op created on the Implicated commit: File: Suggested fix:
Related: Prior hangs with the same test signature: #4975 (
|
|
🤖 Starting review — findings will be posted here when done. |
|
🤖 CI Triage Agent — TL;DR: The ASAN gtest run was aborted by the 900 s gtest watchdog (SIGTERM → signal 6) because Full analysisSummary: Root cause: Log timeline shows the problem clearly:
Line 188 is inside Suspect code is this branch's RC/mlx5 device-memory + outstanding-purge rework, which rewrites the short send and send-op bookkeeping used by exactly this path: Implicated commit: branch head File: Suggested fix:
Related: PR #12058 (this change), #12033 (transmit range in outstanding purge), #11873, #11866, #11836 (earlier purge-outstanding series on the same RC mlx5 path).
|
|
🤖 Starting review — findings will be posted here when done. |
|
🤖 CI Triage Agent — TL;DR: The job was killed after a ~58-minute hang (not a slow test): Full analysisSummary: "Tests roce on worker 1" was cancelled by the Azure agent shutdown after Root cause: The
Timeline proves a hang, not slowness: the largest inter-line gap is Two contributing CI-harness gaps amplified the node fault into a full job loss:
Implicated commit: unknown — no UCX commit is implicated. PR #12058 ("UCT/IB/MLX5: support short dm in purge outstanding") touches the RC/MLX5 device-memory purge path, which is not exercised by File: Suggested fix:
Related: PR #12058 (#12058) — the PR under test, not the cause. No existing issue tracks the
|
|
🤖 Starting review — findings will be posted here when done. |
What?
Add wqe parse support for am/get short dm.
Need rebase after PR#11873 is merged.
Why?
FT need extract info from wqe before replay uct am/get short dm.
How?
Add parser for uct am/get short dm.