[ianmccul]

Nothing in uni20 enabled CUDA peer access. Every cross-device copy therefore went through cudaMemcpyPeerAsync staged by the driver through host memory, and no kernel could dereference another device's buffer.

Measured on polaron (2x Quadro GV100, NVLink NV4, CUDA 12.9), 256 MiB, cudaMallocAsync pool memory as CudaBuffer allocates it:

Access granted	cudaMemcpyPeerAsync	kernel on GPU 1 reading GPU 0
none (current main)	7.9 GB/s	not possible
cudaDeviceEnablePeerAccess only	7.9 GB/s	illegal memory access
plus cudaMemPoolSetAccess	96.9 GB/s	85.2 GB/s

Change. cuda::initialize() grants direct access for every ordered pair of enrolled devices that cudaDeviceCanAccessPeer supports. CUDA needs two grants: cudaDeviceEnablePeerAccess (from the accessing device) covers cudaMalloc allocations, and cudaMemPoolSetAccess on the owning device's default pool covers the stream-ordered allocations used by CudaBuffer. Each grant is recorded before state changes so a failure part-way can be rolled back, and runtime shutdown revokes exactly the access it added once its allocations are drained. Access that was already enabled is reported but left alone.

• Runtime::peer_access(accessor, owner) reports a pair's state, for the strided peer-copy lowering that follows.
• RuntimeConfig::enable_peer_access (default true) disables it to measure or diagnose staged transfers.
• Directly constructed DeviceResources are unaffected.

Existing contiguous peer copies benefit with no API change. The runtime, buffer and getting-started docs describe the grants, the measurement, and how to read nvidia-smi topo -m.

Tests (new, in uni20_cuda_runtime_tests; skipped with fewer than two devices):

• Access matches cudaDeviceCanAccessPeer for both directions, at both the device and pool level, and is fully revoked at shutdown.
• enable_peer_access = false leaves access unchanged.
• A kernel on device 1 reads a pool-allocated CudaBuffer on device 0 (a small .cu helper added to the test module). With only the pool grant disabled, this test aborts with cudaErrorIllegalAddress.

Validation (polaron, GCC 13 Debug, CUDA + cuSOLVER): uni20_cuda_runtime_tests 89/89. Full ctest: 2738/2739; the one failure was CudaTensorTest.StreamOrderedDescriptorAccessCanBeCoAwaitedAsMdspanLease, the flaky test fixed by #123. The full build also hits the pre-existing uni20_common_tests CUDA build failure, also fixed by #123. scripts/check-docs.py passes; clang-format clean. Not run: CPU-only build (this change is CUDA-only), Clang, Release, CUDA 13, or hardware without peer support (the grant path skips such pairs).

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary: ❌ Failed

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	❌ Failed 2026-11-12T18:18:06.947359Z We couldn’t complete this review. Comment @codex review to try again.	5f6bc1d	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

[gemini-code-assist]

Code Review

This pull request implements automatic peer access management for CUDA devices within the Uni20 runtime. It introduces a mechanism to grant and revoke peer access between enrolled devices, covering both standard cudaMalloc allocations and stream-ordered memory pool allocations. The changes include updates to the Runtime class, the addition of a configuration option enable_peer_access, updated documentation, and new test cases to verify peer access functionality and proper cleanup at shutdown. I have no feedback to provide as the implementation appears robust and well-tested.

[ian-review-bot]

Review Bot: review of exact head 5f6bc1d822dadf5acdfedebd32b03f37b5905804

Found 1 issue: the default combined test executable uni20_tests no longer links with CUDA enabled (inline comment on tests/backend/cuda/CMakeLists.txt:6). The runtime change itself behaved as described in every case I ran on two GPUs.

What the PR does. cuda::initialize() now grants peer access for every ordered pair of enrolled devices that cudaDeviceCanAccessPeer allows. It makes two grants: cudaDeviceEnablePeerAccess from the accessing device, and cudaMemPoolSetAccess on the owning device's default pool. It records each grant so that a failed initialization, or shutdown after device_resources_ drain, revokes only what it added (src/uni20/backend/cuda/runtime.{hpp,cpp}). It also adds Runtime::peer_access(accessor, owner) and RuntimeConfig::enable_peer_access, three two-GPU tests with a small .cu kernel helper, and a "Peer Access" section in the CUDA runtime, buffer and getting-started docs.

What I reviewed. Runtime constructor, destructor, grant_peer_access, revoke_peer_access and peer_access, compared against how CudaAllocation picks pool or cudaMalloc (buffer.hpp:121-125) and how DeviceResources drains at destruction. The three new tests and the test CMake wiring, including add_test_module and the combined-executable path. The claims in the description and docs: the bandwidth table, the illegal-address fault without the pool grant, exact revocation, and leaving pre-existing access alone.

Verified (polaron, 2× Quadro GV100 linked NV4, GCC 13.3, CUDA 12.9, -DCMAKE_BUILD_TYPE=Debug -DUNI20_ENABLE_CUDA=ON -DUNI20_BACKEND_CUSOLVER=ON -DCMAKE_CUDA_ARCHITECTURES=70, other options as in the CI Release job):

• uni20_cuda_runtime_tests: 89/89 pass, including the three new *Peer* tests.
• A standalone probe linked against the built libuni20_backend_cuda.a and your kernel helper. Each case checks device-level and pool-level access before, during and after the runtime's lifetime:
  • enrolment {1, 0}: both directions granted and both kernel reads correct; everything revoked afterwards.
  • enrolment {1}: peer_access(1,0) and peer_access(0,0) are false, peer_access(1,1) is true; nothing granted.
  • pre-existing device and pool access for 1→0: still present after shutdown, and the 0→1 grants this runtime added are removed.
  • pre-existing pool access only for 1→0: the runtime adds and then removes only the device-level grant.
  • enable_peer_access = false: the kernel read 1←0 fails with cudaErrorIllegalAddress.
  • cudaDeviceEnablePeerAccess alone: also cudaErrorIllegalAddress. This confirms that the pool grant is the one the new kernel test depends on.
  • three init/shutdown rounds: each round's peer kernel reads are correct, and the state is clean at the end.
• 256 MiB cudaMemcpyPeerAsync between pool-allocated CudaBuffers: 7.7–8.6 GB/s with no grant or the device-level grant only, and 96.6–96.8 GB/s with the runtime's grants. This matches your 7.9 and 96.9 GB/s.
• ctest over the CUDA runtime, cuBLAS, cuSOLVER, linalg-CUDA, TBB-CUDA-scheduler and krylov-CUDA tests (217 tests, -j4), plus five further -j8 runs of the ^Cuda|^DebugCuda subset, on head and on base 697e49e7:
  • Intermittent CudaTensorTest failures (StreamOrderedDescriptorAccessCanBeCoAwaitedAsMdspanLease, StreamOrderedAccessCanBeCoAwaitedAsTensorView, OwningRvalueStreamReadPublishesCompletion) happen on base as well. These tests use a directly constructed DeviceResources, so the new code does not run in them.
  • One -j8 run on head timed out in CudaCopyTest.SameDeviceCopyOrdersPendingSourceAndDestinationReuse. It enrols only device 0, so no peer call is made. I could not reproduce it: 100/100 passes with --gtest_repeat on head and on base, and the next ctest repeat passed.
• scripts/check-docs.py passes. The runtime.md#peer-access anchor resolves.
• Combined uni20_tests (-DUNI20_BUILD_COMBINED_TESTS=ON, the standalone default): the full build fails at link with undefined reference to uni20_test_launch_copy_doubles from test_cuda_runtime.cpp:292. With a two-part CMake change, tested in a scratch worktree, it links and its *Peer* tests pass. Details are in the inline comment. In this configuration uni20_common_tests builds without error, so I did not see the separate failure you mention.

Needs a human. grant_peer_access treats any cudaDeviceEnablePeerAccess result other than success or cudaErrorPeerAccessAlreadyEnabled as fatal, and the constructor rethrows it (runtime.cpp:776-784, 821-825). With the default "enrol every device", initialize() now fails on any machine where a peer grant fails, where it used to succeed with staged copies. One known way this can happen: CUDA documents a limit of eight peer connections per device on systems without NVSwitch (cudaErrorTooManyPeers). That would hit a node with more than eight mutually peer-capable GPUs. I could not test this with two GPUs, and whether a failed grant should be fatal or just leave that pair staged is a design decision for you.

Not checked.

• Hardware without peer support, PCIe-only peer pairs, more than two GPUs, and the memory_pools_supported == false path (both GV100s support pools).
• CUDA 12.5, 12.6 and 13.0 toolkits; Release, Clang 19 and CPU-only builds. This change is CUDA-only, and CI does not build CUDA.
• A full unfiltered ctest of the whole tree; I ran only the CUDA test targets listed above.
• MPS, MIG or multi-process sharing of the default memory pools.

---

_{Review tier: full (code or new pull request) · model: claude-opus-5-5 · effort: high}

[ian-review-bot]

Review Bot: review of exact head 5f6bc1d822dadf5acdfedebd32b03f37b5905804

The new .cu helper is added only to uni20_cuda_runtime_tests, not to the combined uni20_tests executable. add_test_module (cmake/Uni20TestHelpers.cmake:29-31) collects only its SOURCES into UNI20_COMBINED_TEST_SRCS. So uni20_tests compiles test_cuda_runtime.cpp, which now calls uni20_test_launch_copy_doubles (test_cuda_runtime.cpp:292), but does not compile the file that defines it. UNI20_BUILD_COMBINED_TESTS defaults to ON for standalone builds (CMakeLists.txt:127, docs/development/testing.md:259), so a default CUDA build of all will fail to link. CI does not catch this because every job passes -DUNI20_BUILD_COMBINED_TESTS=OFF. The existing test_cuda_descriptor_compile.cu in tests/linalg avoids the problem only because nothing calls into it.

How I checked (polaron, GCC 13.3, CUDA 12.9, Debug, -DUNI20_ENABLE_CUDA=ON -DUNI20_BACKEND_CUSOLVER=ON -DUNI20_BUILD_COMBINED_TESTS=ON, other options as in the CI Release job):

• ninja -t commands uni20_tests | tail -1: the link line has 228 objects, including tests/CMakeFiles/uni20_tests.dir/backend/cuda/test_cuda_runtime.cpp.o, and no .cu.o at all.

• nm -C -u on that object lists U uni20_test_launch_copy_doubles(double*, double const*, unsigned long, CUstream_st*). Only tests/backend/cuda/test_cuda_peer_kernel.cu defines it.

• Linking that object with the combined uni20_tests_main.cpp.o and the same libraries fails with exactly one error: test_cuda_runtime.cpp:292: undefined reference to 'uni20_test_launch_copy_doubles(...)'.

• A full cmake --build build-comb --target uni20_tests -- -k 0 compiles every object. The only failure is the final link of tests/uni20_tests, with that same undefined reference. In this configuration uni20_common_tests also builds without error.

A fix I tested in a scratch worktree of this head:

1. Move test_cuda_peer_kernel.cu into the SOURCES of add_test_module(cuda_runtime ...) and drop the target_sources line.
2. In tests/CMakeLists.txt, after uni20_tests is created, add if(CMAKE_CUDA_COMPILER) set_target_properties(uni20_tests PROPERTIES CUDA_STANDARD 20 CUDA_STANDARD_REQUIRED ON) endif().

Step 2 is needed: with step 1 alone, CMake configure fails with Target "uni20_tests" requires the language dialect "CUDA23". With both steps, uni20_tests and uni20_cuda_runtime_tests build and link, and uni20_tests --gtest_filter='*Peer*' passes 7/7.

Not checked. Clang 19 or Release builds of the combined target; a combined build with UNI20_ENABLE_CUDA=OFF (not affected, since tests/backend/cuda is CUDA-only); running all of uni20_tests.

[ianmccul]

Confirmed, thanks. Fixed in a29f570 with your two-part change: test_cuda_peer_kernel.cu is now in the module's SOURCES, and uni20_tests gets CUDA_STANDARD 20 when a CUDA compiler is enabled. Checked in a fresh -DUNI20_BUILD_COMBINED_TESTS=ON CUDA tree on polaron: uni20_tests configures, builds and links, and its *Peer* tests pass (8/8 at the current head).

[ian-review-bot]

Review Bot: recheck of exact head 66a304ff358bb4f95586822dd9a7c45bc6cfe0d8

Confirmed fixed. I configured a fresh tree at this head with -DUNI20_BUILD_COMBINED_TESTS=ON (GCC 13.3, CUDA 12.9, Debug, -DUNI20_ENABLE_CUDA=ON -DUNI20_BACKEND_CUSOLVER=ON -DCMAKE_CUDA_ARCHITECTURES=70, other options as in the CI Release job) and ran cmake --build build --target uni20_tests. All 278 steps finished, including the link of tests/uni20_tests. uni20_tests --gtest_filter='*Peer*' then passed 8/8 on the two GV100s.

Not checked. The combined target under Clang 19 or Release.

[ianmccul]

On the Needs a human point: decided that a failed grant is not fatal by default. 66a304f replaces RuntimeConfig::enable_peer_access with peer_access_policy:

• best_effort (default): grant every capable pair. A pair whose grant fails is rolled back (including a device-level grant made before a failed pool grant), reported with display::warning, and left to staged copies.
• required: a failed grant is an error.
• disabled: grant nothing.

Policies apply only to pairs that cudaDeviceCanAccessPeer reports as capable; other pairs are never failures. Each pair's grant now returns the failing CUDA status rather than raising, because cuda::check aborts by default instead of throwing.

Tests: a new RequiredPeerAccessGrantsEveryCapablePair. uni20_cuda_runtime_tests passes 90/90, and the full CUDA ctest passes 2740/2740. The only build failure is the pre-existing uni20_common_tests one fixed by #123. The best-effort failure path itself is not exercised: two always-peer-capable GPUs cannot make a grant fail without a fault-injection hook, which I did not add.

[ian-review-bot]

Review Bot: review of exact head 66a304ff358bb4f95586822dd9a7c45bc6cfe0d8

This line directly follows the policy table with no blank line between them. In GitHub-flavored Markdown a table continues until a blank line, so on GitHub this sentence and the next two lines (the NV# / PIX/PXB / SYS explanation) become three extra rows of the policy table, each a one-cell row with an empty "Behavior" column. The leading space does not stop the table.

How I checked: I took the ## Peer Access section of this file at this head and rendered it with cmarkgfm 2025.10.22 (github_flavored_markdown_to_html, the cmark-gfm library GitHub uses). After the disabled row the output has <tr><td>Use <code>nvidia-smi topo -m</code> to see whether a pair is connected by</td><td></td></tr> and two more rows like it. markdown-it-py 3.0 with tables enabled gives the same result. pandoc 3.1.3 -f gfm ends the table there instead, which is why some renderers will look fine. scripts/check-docs.py passes because it does not check table structure.

Fix: put a blank line before this line and drop its leading space.

Not checked. The Doxygen rendering of this page.

[ianmccul]

Fixed in 700c363: blank line added and the leading space dropped.

[ian-review-bot]

Review Bot: review of exact head 66a304ff358bb4f95586822dd9a7c45bc6cfe0d8

Found 1 new issue, a docs rendering problem (inline on docs/backends/cuda/runtime.md:79). Both earlier points are settled: the combined uni20_tests link failure is fixed (confirmed in the thread), and the fatal-grant question is answered by PeerAccessPolicy. I also measured one pre-existing intermittent test deadlock that this PR's default makes more frequent; see Needs a human.

What changed since 5f6bc1d8.

• a29f570a moves test_cuda_peer_kernel.cu into the module's SOURCES and gives uni20_tests CUDA_STANDARD 20.
• 66a304ff replaces bool enable_peer_access with PeerAccessPolicy {disabled, best_effort, required}, defaulting to best_effort. It splits each pair's grant into try_grant_peer_access, which returns the failing status instead of raising. Under best_effort it rolls back a failed pair with revoke_peer_grant, pops it from peer_grants_ and calls display::warning. Under required it calls check and leaves the rollback to the constructor. The commit also adds RequiredPeerAccessGrantsEveryCapablePair and the policy table in the docs.

What I reviewed. In runtime.cpp:776-905, grant_peer_access, try_grant_peer_access, revoke_peer_grant and peer_access. I compared them against cuda::check → trace::raise, which aborts or throws depending on errors_abort(). I also checked the CMake changes and the new test. In the docs I checked the policy table and the getting_started.md field name. git grep finds no remaining use of enable_peer_access.

Verified (polaron, 2× Quadro GV100 NV4, GCC 13.3, CUDA 12.9, Debug, -DUNI20_ENABLE_CUDA=ON -DUNI20_BACKEND_CUSOLVER=ON -DCMAKE_CUDA_ARCHITECTURES=70 -DUNI20_BUILD_COMBINED_TESTS=ON, other options as in the CI Release job):

• uni20_cuda_runtime_tests: 90/90. Combined uni20_tests links, and its *Peer* tests pass 8/8.
• Failure paths, by fault injection. The two GPUs never fail a grant, so I linked a probe against libuni20_backend_cuda.a. CUDA::cudart is the shared library, so the probe can interpose cudaDeviceEnablePeerAccess and cudaMemPoolSetAccess and return cudaErrorTooManyPeers for one chosen pair. In every case I checked device-level and pool-level access before, during and after the runtime, and then started a second, clean runtime:
  • best_effort, pool grant 1→0 fails: the device grant 1→0 that had already been made is rolled back, 0→1 stays granted, peer_access(1,0) is false, the warning names cudaMemPoolSetAccess, and no error is left pending.
  • best_effort, device grant 0→1 fails: 1→0 stays granted and 0→1 is not.
  • required in throw mode, failing the pool grant on the first pair, the pool grant on the second pair, or the device grant: CudaRuntimeError carries the right operation, status and device. Every grant is revoked, including the pair granted before the failure.
  • required in default abort mode: aborts with the diagnostic at runtime.cpp:827.
  • After each of these runs, the next initialize() grants everything normally and revokes it again.
• scripts/check-docs.py passes (it does not check table structure; see the inline comment).

Needs a human. CudaCopyTest.AsyncPeerMaterializationCarriesCompletionIntoOwnedResult (tests/linalg/test_cuda_copy.cpp:823) intermittently aborts with PANIC … DEADLOCK get_wait object is not available but there are no runnable tasks from DebugScheduler::help_while_waiting (debug_scheduler.hpp:114). It first appeared in one of 10 runs of uni20_tests --gtest_filter='Cuda*:DebugCuda*'. To measure it I ran uni20_linalg_cuda_tests --gtest_filter='CudaCopyTest.*Peer*' --gtest_repeat=100 in a loop, restarting after each abort, and interleaved three builds in the second round:

Build	Runs of that test (approx.)	Aborts	Rate
base 697e49e7	1548	9	0.6%
this head, peer grants on (default)	1493	45	3.0%
this head, peer_access_policy = disabled in that test file only	824	4	0.5%

All 58 aborts are this same panic in this same test. The bug is therefore pre-existing, but the default grant makes it about five times more frequent on a two-GPU host. The task snapshot shows to_device (task 66) suspended at copy.hpp:148, and the copy-submission chain (dispatch.hpp:136, copy_task.hpp:41) also suspended. I have not found the root cause. One hypothesis: CUDA-side wakeups, such as the stream-pool return via cudaLaunchHostFunc at runtime.cpp:476, can still arrive after the debug scheduler sees an empty queue. Before merging, you may want to check whether #123's flaky-test fix also covers this test.

Not checked.

• Real (non-injected) grant failures, more than two GPUs, devices without memory pools.
• Clang 19, Release, and CUDA 12.5/12.6/13 builds.
• An unfiltered ctest of the whole tree.
• The Doxygen rendering of the new docs table.
• The root cause of the deadlock above.

---

_{Review tier: full (code or new pull request) · model: claude-opus-5-5 · effort: high}

[ianmccul]

On the Needs a human deadlock in CudaCopyTest.AsyncPeerMaterializationCarriesCompletionIntoOwnedResult: it is the false DEADLOCK … no runnable tasks panic that #123 removes. The get_wait finds the DebugScheduler queue empty while a stream is about to return through a CUDA host callback. Direct peer copies plausibly shift the timing so the wait lands in that window more often.

Measured on polaron (2× GV100), using your loop: uni20_linalg_cuda_tests --gtest_filter='CudaCopyTest.*Peer*' --gtest_repeat=100, 20 invocations, restarting after an abort:

Build	Runs of the test	Aborts
this PR alone (700c3636)	804	18, all the DEADLOCK panic
this PR merged locally with #123 (6f0bebba)	2000	0

So #123 should merge before, or together with, this PR.