Skip to content

BUILD: Fix compilation with GCC 16 - #11908

Open
Alexey-Rivkin wants to merge 2 commits into
openucx:masterfrom
Alexey-Rivkin:perftest-omp-masked
Open

BUILD: Fix compilation with GCC 16#11908
Alexey-Rivkin wants to merge 2 commits into
openucx:masterfrom
Alexey-Rivkin:perftest-omp-masked

Conversation

@Alexey-Rivkin

@Alexey-Rivkin Alexey-Rivkin commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

What?

Make UCX build with GCC 16 (Fedora 44): two source fixes, no behaviour change.

Why?

Fedora 44's GCC 16 turns two things into errors under -Werror:

  • OpenMP 5.1 renamed master to masked; -Werror=deprecated-openmp rejects the old spelling in ucx_perftest.
  • -Werror=unused-but-set-variable now fires on three gtest counters that are only ever incremented.

Seen on both arches in #11906, which adds the Fedora 44 CI rows and depends on this.

How?

  • src/tools/perf/perftest.c: both #pragma omp master sites switch on defined(_OPENMP) && (_OPENMP >= 202011).
  • test/gtest/ucs/test_pgtable.cc, uct/ib/test_ud_pending.cc, uct/test_mem.cc: drop the dead counters; each test asserts on other values.
  • Build: full make -k of configure-devel and configure-release trees in the Fedora 44 builder image, gcc (GCC) 16.2.1 20260819, clean.
  • Tests: this PR's Azure run (134621) ran the edited tests green on IB hardware - test_pgtable.multi_search, test_mem.mmap_fixed/0-3, test_ud_pending.tx_wqe on ud_mlx5 and ud_verbs - and drove ucx_perftest through run_ucx_perftest*, run_ucx_perftest_fault_tolerance and run_ucx_perftest_with_daemon. Those compilers take the master path. The masked path was run under GCC 16 in the Fedora 44 image: ucx_perftest client/server, single-threaded and -T 2, clean; the fedora44 rows in CI: Add Fedora 44, Ubuntu 26.10, Azure Linux 4.0, UOS and openEuler 22.03 SP4 / 24.03 SP3 #11906 build it in CI.

@svc-nvidia-pr-review

Copy link
Copy Markdown

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

Comment thread src/tools/perf/perftest.c
Comment thread src/tools/perf/perftest.c
Alexey-Rivkin added a commit to Alexey-Rivkin/ucx that referenced this pull request Sep 7, 2026
…efore merge)

Temporary copy of openucx#11908 so the fedora44 rows in this PR build with GCC
16. Revert or drop this commit once openucx#11908 lands.

(cherry picked from commit 493b319557b5f0a1d05b0d15e5dc7ce4f2ddc2fb)
Alexey-Rivkin added a commit to Alexey-Rivkin/ucx that referenced this pull request Sep 7, 2026
…efore merge)

Temporary copy of openucx#11908 so the fedora44 rows in this PR build with GCC
16. Revert or drop this commit once openucx#11908 lands.

(cherry picked from commit 087470a)
@svc-nvidia-pr-review

Copy link
Copy Markdown

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

Comment thread src/tools/perf/perftest.c Outdated
Comment thread src/tools/perf/perftest.c Outdated
@svc-nvidia-pr-review

Copy link
Copy Markdown

Coverage note: This is a build-portability change with no runtime behavior change; it is validated by the existing perftest build/CI as long as CI includes an OpenMP-enabled compiler that emits deprecation warnings for omp master (OpenMP 5.1+). If no CI job builds perftest with a 5.1+ compiler with warnings-as-errors, the deprecation-warning motivation isn't actually exercised — worth confirming with the author which build job surfaced the warning.

Alexey-Rivkin added a commit to Alexey-Rivkin/ucx that referenced this pull request Sep 7, 2026
…re merge)

Temporary copy of openucx#11908 so the fedora44 rows in this PR build with GCC
16. Revert or drop this commit once openucx#11908 lands.

(cherry picked from commits 087470a and 4859353)

Signed-off-by: Alexey Rivkin <arivkin@nvidia.com>
@svc-nvidia-pr-review

Copy link
Copy Markdown

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

@Alexey-Rivkin Alexey-Rivkin changed the title TOOLS/PERF: Use omp masked on OpenMP 5.1 and newer BUILD: Fix compilation with GCC 16 Sep 7, 2026
Comment thread src/tools/perf/perftest.c Outdated
@svc-nvidia-pr-review

Copy link
Copy Markdown

Scope (no-prefix, non-blocking): this PR bundles an unrelated OpenMP portability fix (perftest.c) with dead-variable cleanup across three gtest files. Per REVIEW.md, refactoring/cleanup should be separate from functional changes. Both parts are trivial, so this is only worth noting, not a blocker — but if the author wants a clean history, splitting is preferable.

@svc-ucx

svc-ucx commented Sep 7, 2026

Copy link
Copy Markdown

🤖 CI Triage AgentUCX PR (Tests new on worker 0) · commit 4859353b

TL;DR: shm_ib/test_ucp_peer_failure_keepalive.kill_receiver/0 <shm,ib/am> hung for 15 minutes inside the unbounded AM-receive poll loop in smoke_test() until the gtest watchdog aborted the process; the PR (#11908 is a GCC-16 build fix) cannot cause this, so it is an unrelated hang/flake — re-run CI and bound the poll loop with a deadline so it fails with a diagnostic instead of a watchdog abort.

Full analysis

Summary: Azure job "new on worker 0" failed with make: *** [Makefile:4713: test] Aborted (core dumped) after gtest's watchdog killed a hung test_ucp_peer_failure_keepalive.kill_receiver/0 <shm,ib/am>.

Root cause: Timestamp analysis shows a single ~15-minute silence: the last application line is 09:26:47 [ RUN ] shm_ib/test_ucp_peer_failure_keepalive.kill_receiver/0 <shm,ib/am> and the next line is 09:41:48 test_helpers.cc:57: Failure / Connection timed out - abort testing, i.e. the gtest watchdog (watchdog_func, pthread_cond_timedwaitETIMEDOUTpthread_kill) fired — a hang, not a slow test. The backtrace pinpoints the hang at test_ucp_peer_failure::smoke_test() test_ucp_peer_failure.cc:267, called from test_body() line 678 (smoke_test(false) on the failing pair). That is the loop:

while (m_am_rx_count == prev_am_rx_count) {
    progress();
}

The 1 MB AM (rendezvous) sent on failing_sender() over the shm,ib transport was never delivered, and the loop has no deadline and never checks the send request's status, so the thread spun in progress()/sched_yield() forever. Note the preceding shm_ib test on the same node was also anomalously slow (stress_connect_force_disconnect/0 took 29 s vs ~12 s for its siblings), consistent with the IB/shm path on swx-rdmz-ucx-new-01 stalling. Nothing in PR #11908 ("BUILD: Fix compilation with GCC 16") touches UCP AM, keepalive, IB or shm datapaths, and the test file has no recent changes (last touch dad9eb55, 2026-05-27), so the failure is not attributable to this PR.

Implicated commit: unknown — not caused by commit [REDACTED:Hex High Entropy String] (PR #11908 is build-only); latest change to the test is dad9eb5 (nbellalou, 2026-05-27), which predates and is unrelated.

File: test/gtest/ucp/test_ucp_peer_failure.cc:266-268 (hang site); watchdog abort reported at test/gtest/common/test_helpers.cc:57

Suggested fix:

  1. Re-run the Azure job — this PR only changes build flags/compilation, so the failure should be treated as an infrastructure/flaky hang; check the IB link and leftover gtest processes on agent swx-rdmz-ucx-new-01 (agent-03), since the neighbouring shm_ib test was also abnormally slow.
  2. Harden the test so a stalled AM produces a real diagnosis instead of a 15-minute watchdog abort: replace the unbounded loop with a deadline-bounded wait, e.g.
    ucs_time_t deadline = ucs::get_deadline();
    while ((m_am_rx_count == prev_am_rx_count) && (ucs_get_time() < deadline)) {
        progress();
    }
    ASSERT_NE(prev_am_rx_count, m_am_rx_count) << "AM was not received";
    and also check m_sreq's status inside the loop (ucp_request_check_status) so a failed/erroring send breaks out instead of spinning.

Related: PR #11908 (the triggering PR, unrelated to the failure); no existing issue found for this hang — worth opening one for flaky test_ucp_peer_failure_keepalive.kill_receiver <shm,ib/am>.

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id 0722b96b-9836-4101-b9ac-4cce2aeafaf3 in the triage console for the audit trail.

OpenMP 5.1 renamed the master construct to masked, and GCC 16 (Fedora
44) now rejects the old spelling under -Werror=deprecated-openmp, so
ucx_perftest no longer compiles there. Pick the construct from _OPENMP
so older compilers keep the spelling they understand.

Signed-off-by: Alexey Rivkin <arivkin@nvidia.com>
Alexey-Rivkin added a commit to Alexey-Rivkin/ucx that referenced this pull request Sep 8, 2026
…re merge)

Temporary copy of openucx#11908 so the fedora44 rows in this PR build with GCC
16. Revert or drop this commit once openucx#11908 lands.

(cherry picked from commits 29aa3c1 and 29aa3c1)

Signed-off-by: Alexey Rivkin <arivkin@nvidia.com>
@svc-nvidia-pr-review

Copy link
Copy Markdown

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

GCC 16 promotes these to errors under -Werror=unused-but-set-variable
and the Fedora 44 devel build stops on them. Each counter is only ever
incremented; the tests assert on other values.

Signed-off-by: Alexey Rivkin <arivkin@nvidia.com>
Alexey-Rivkin added a commit to Alexey-Rivkin/ucx that referenced this pull request Sep 8, 2026
…re merge)

Temporary copy of openucx#11908 so the fedora44 rows in this PR build with GCC
16. Revert or drop this commit once openucx#11908 lands.

(cherry picked from commits 29aa3c1 and f1bea8d)

Signed-off-by: Alexey Rivkin <arivkin@nvidia.com>
@svc-nvidia-pr-review

Copy link
Copy Markdown

🤖 Review complete — no issues found.

@svc-nvidia-pr-review

Copy link
Copy Markdown

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

@svc-nvidia-pr-review

Copy link
Copy Markdown

🤖 Review complete — no issues found.

Comment thread src/tools/perf/perftest.c
#if _OPENMP
# pragma omp barrier
# pragma omp master
# if _OPENMP >= 202011

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can we always use pragma omp masked ?

@Alexey-Rivkin Alexey-Rivkin Sep 9, 2026

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.

Tried and nope - masked is a new keyword. Older GCC versions don't support it.

Alexey-Rivkin added a commit to Alexey-Rivkin/ucx that referenced this pull request Sep 8, 2026
…re merge)

Temporary copy of openucx#11908 so the fedora44 rows in this PR build with GCC
16. Revert or drop this commit once openucx#11908 lands.

(cherry picked from commits 29aa3c1 and f1bea8d)

Signed-off-by: Alexey Rivkin <arivkin@nvidia.com>
@svc-ucx

svc-ucx commented Sep 8, 2026

Copy link
Copy Markdown

🤖 CI Triage AgentUCX PR (Tests roce on worker 2) · commit f1bea8d2

TL;DR: The gtest run on the roce worker aborted because a UMR mkey registration on device mlx5_1 never completed — ibv_poll_cq(UMR CQ, registration...) hit its hard-coded 30 s timeout, making ucp_rkey_pack() fail with UCS_ERR_IO_ERROR, and the failed registration left a UCP rcache region with refcount == 1, so ucs_rcache_purge() asserted and core-dumped at teardown. This is a device/UMR-QP stall on the CI machine (unrelated to PR #11908, a GCC-16 build fix), amplified by a missing error-path ref release in the UMR registration failure handling.

Full analysis

Summary: all/test_ucp_ep_based_fence.test_ep_based_fence_before_put/0 failed (ucp_test.cc:1281 ASSERT_UCS_OK → "Input/output error") after a 30 s UMR CQ timeout on mlx5_1, then the process aborted on region->refcount == 0 assertion in rcache.c:394 during ucp_cleanup().

Root cause: Evidence chain in the log (12:22:25 test start → 12:22:55, exactly the 30 s poll budget, one silent gap):

  1. ib_mlx5dv_md.c:1443 mlx5_1: ibv_poll_cq(UMR CQ, registration, UMR mkey ... index 0x3c82) timed out — the UMR QP on mlx5_1 posted the registration WR but never produced a completion, i.e. the UMR QP/loopback path on that HCA stalled. This is the primary hang; uct_ib_mlx5_devx_umr_post_sync() gives up after ucs_time_from_sec(30) (src/uct/ib/mlx5/dv/ib_mlx5dv_md.c:1441-1447).
  2. The subsequent cleanup ibv_post_send(UMR QP, invalidation, ...) returned 12: Bad address confirms the UMR QP was already in an error state.
  3. ucp_rkey_pack() therefore returned UCS_ERR_IO_ERROR, tripping ASSERT_UCS_OK in ucp_test::mapped_buffer (test/gtest/ucp/ucp_test.cc:1281). Because gtest ASSERT_* only returns from the constructor, the half-registered memh/rcache region was never properly released — hence rcache.c:670 ucp_rcache: destroying inuse region ... ref 1 md[5]=mlx5_1 and the fatal assert at rcache.c:394, which turned a recoverable I/O error into make: *** [test] Aborted (core dumped).

The tested change (PR #11908, "BUILD: Fix compilation with GCC 16") touches no UCT/IB or rcache code, and test/gtest/ucp/test_ucp_fence.cc hasn't been modified since 2025 — so the PR is not the cause. The failing HCA mlx5_1 on swx-rain04 was already suspect earlier in the same log (all ud_mlx5/mlx5_1:1 atomics tests skipped via !check_atomics(...)).

Implicated commit: unknown (no code change in the PR touches this path; last relevant UMR-path commits are a956862 "UCT/IB: Support relaxed-only memory keys" by Roie Danino and b788849 "UCP/DEVICE: Add multi-lane support" by Artemy Kovalyov, which introduced/extended the synchronous UMR post path)

File: src/uct/ib/mlx5/dv/ib_mlx5dv_md.c:1441 (UMR poll timeout) and src/ucs/memory/rcache.c:394 (fatal assert during purge); test trigger at test/gtest/ucp/ucp_test.cc:1281

Suggested fix:

  1. Treat this run as infrastructure failure and re-run the PR — check/reset mlx5_1 on swx-rain04 (roce worker 2); a stuck UMR QP usually means the port/firmware needs attention. If mlx5_1 is not usable for UMR, exclude it from the roce worker's device list.
  2. Product-side hardening worth a separate PR: in the UMR registration error path (uct_ib_mlx5_devx_umr_post_sync() callers in ib_mlx5dv_md.c), release the memh/rcache reference taken before the post so a failed registration cannot leave an in-use region behind; that would make this manifest as a clean test failure instead of an abort/core dump in ucs_rcache_purge(). Also consider marking the md as broken after a UMR timeout instead of continuing to post to a dead QP (the "Bad address" invalidation).
  3. Optionally make the 30 s UMR poll timeout configurable and log the QP state on timeout to aid future triage.

Related: PR #11908 (the unrelated build-fix PR under test); no existing issue found for the ibv_poll_cq(UMR CQ ...) timed out + rcache: destroying inuse region signature — worth filing one.

@Alexey-Rivkin
Alexey-Rivkin marked this pull request as ready for review September 8, 2026 14:10
Alexey-Rivkin added a commit to Alexey-Rivkin/ucx that referenced this pull request Sep 8, 2026
…re merge)

Temporary copy of openucx#11908 so the fedora44 rows in this PR build with GCC
16. Revert or drop this commit once openucx#11908 lands.

(cherry picked from commits 29aa3c1 and f1bea8d)

Signed-off-by: Alexey Rivkin <arivkin@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants