Repository navigation
perftest: add data validation for ROCm device memory - #419
Open
sbates130272 wants to merge 1 commit into
Open
sbates130272 wants to merge 1 commit into
sbates130272 wants to merge 1 commit into
Conversation
perftest validates host and CUDA memory but not ROCm: --data_validation is refused for --use_rocm, and the ROCm backend's copy ops could not have supported it anyway. Add a ROCm validation backend, wired through the existing memory_ctx hooks. src/rocm_kernels.hip is a port of src/cuda_kernels.cu and is the default: a resident kernel in which block 0 monitors the tail markers and blocks 1..N drain a lock-free queue, so each cycle is checked as it lands rather than inspecting the final state afterwards. Where it cannot run -- a build without hipcc, a device the kernel will not launch on, or PERFTEST_ROCM_VALIDATION=staged -- the fallback compares on the host after the run. The fallback is strictly weaker: it sees only each chunk's final state, so transient corruption a later cycle overwrote is missed and bytes_validated is correspondingly lower. Which path ran is printed at stop, and a live validator that observed no completed cycle falls back rather than reporting an empty result. Both paths reuse upstream's classify_validation_mismatch(), so a run stopping mid-cycle is scored as an overwrite race rather than corruption. The ROCm copy ops were plain memcpy into and out of device memory. They are now hipMemcpy with the right direction. The port stays deliberately close to the original: same monitor/validator split, same lock-free queue, same structures from cuda_validation.h, same exported entry points with nothing missing. The rocm_kernel_ prefix is not cosmetic -- the CUDA plugin is dlopen'd from libperftest_kernels.so, while these link into libperftest.a and share a namespace with the memory_ctx hooks. Divergences are marked PORT: at the site. Warp width is the one that matters. The CUDA reduction assumes a 32-lane warp (tid & 31, shuffle offset starting at 16). An AMD wavefront is 64 lanes on CDNA and 32 on RDNA, so the literal reduces only within half a wave and lets two lanes observe lane == 0, double-counting the atomics that follow. The port uses warpSize, which is a built-in rather than a constant, so the unrolls go with it. HIP's warp intrinsics take no mask operand, so the *_sync forms map onto the HIP ones. The synchronisation is expressed with scoped atomics rather than the CUDA original's volatile idiom. volatile is not a synchronisation primitive: it constrains the compiler for one access and says nothing about which agents the access must be coherent with. It works on NVIDIA because the emitted loads happen to bypass cache. On AMD the memory scope is what selects which cache levels are bypassed, so __hip_atomic_load/store carry it explicitly -- system scope for the tail markers and receive slots the NIC DMAs into, and for the flags shared with the host; agent scope for the work queue only this kernel's blocks touch. The queue handoff is a release/acquire pair on the slot's valid flag. A uniform __threadfence_system() over-synchronised the queue while under-specifying the markers. Two __ldg uses go with them, both on data the NIC is actively writing. __ldg is a read-only-cache hint, asserting the data will not change for the life of the kernel, which is false for a DMA target. The tail retry re-read through __ldg was the load least likely to observe the write it was waiting for; and the forward-byte snapshot feeds classify_validation_mismatch(), where a stale read biases towards reporting corruption that did not occur. The bulk compare loop keeps __ldg, ordered by the acquire on the marker. Poll loops back off with s_sleep rather than spinning. A device that runs a wave until it blocks never retires a tight poll loop, so the kernel cannot observe should_stop and the host waits in hipDeviceSynchronize() forever. That includes the slot-valid wait in the work queue, which a consumer reaches holding its block at a __syncthreads(), so stalling there strands every thread in the block rather than just itself. validation_start() also honours num_blocks and threads_per_block, which the CUDA original accepts and ignores: 33 blocks x 1024 spinning threads is not a geometry every device retires. rocm_kernels_available() is the one addition with no CUDA counterpart. The CUDA plugin answers that question by whether libperftest_kernels.so dlopen'd at all; these kernels link in unconditionally and need a runtime probe. Error codes are not discarded. hipcc marks the HIP entry points nodiscard where nvcc does not, which exposed three cases the CUDA style hides: the teardown frees genuinely do not care and say so with a cast; a hipMemset that quietly failed would leave a previous run's markers, stats or error record in place for the validator to report as this run's, and now returns VALIDATION_ERR_MEMSET, which cuda_validation.h had defined and nothing returned; and a discarded hipMemcpy in check_status would have reported "not failed", asserting the data is good on the strength of a read that did not happen. Three supporting changes. VALIDATION_INLINE only annotated the shared helpers __host__ __device__ for __CUDACC__, so they were not callable from hipcc device code. hipcc compiles .hip as nvcc compiles .cu; both need the annotation. configure learns --with-rocm-arch, defaulting to rocm_agent_enumerator. hipcc otherwise targets whatever GPU the build host reports, which is wrong whenever perftest is cross-built or built in a VM, and the symptom is "device kernel image is invalid" at launch rather than anything at build time. The ROCm dmabuf path no longer greps the kernel config before running. That check required CONFIG_DMABUF_MOVE_NOTIFY, which no longer exists upstream -- dma-buf always builds with it, and .move_notify became .invalidate_mappings -- so it began refusing to run precisely because support had become universal. What it gated was whether amdgpu_dma_buf_pin() kept AMDGPU_GEM_DOMAIN_VRAM or fell back to GTT: driver policy, versioned with the driver rather than the running kernel, which on a DKMS install moves independently of it. CONFIG_PCI_P2PDMA fared no better, being blind to whether the NIC's driver implements .reg_user_mr_dmabuf. Both questions are already answered accurately by hsa_amd_portable_export_dmabuf() and ibv_reg_dmabuf_mr(); their diagnostics are improved instead of being predicted. data_validation_stop_and_report() is reordered: errors, then nothing-validated, then pass. This affects every backend, not just ROCm. Testing "nothing validated" first reports a run whose chunks all mismatched -- errors > 0 with bytes_validated == 0 -- as a warning about missing data, hiding a real failure. And backends set passed = (errors == 0 && chunks > 0), so a run that observed no completed cycle has passed == 0 and would otherwise be reported as a failure with every error field still zero: "FAILED - 0 error(s) ... (expected 0x00, got 0x00)". bytes_validated == 0 is that same condition counted differently, since a chunk mismatching at its first byte contributes no compared bytes but still increments the chunk count. Two debugging hooks. PERFTEST_ROCM_VALIDATION_FAULT=n corrupts n bytes of the staged copy before the compare, because a validator that only ever passes is indistinguishable from one that does not look. PERFTEST_ROCM_HOST_ALLOC=1 allocates host-mapped rather than VRAM, for devices whose MR registers cleanly and then fails to gather; it measures host memory, so it diagnoses reachability rather than benchmarking peer-to-peer. Tested on an emulated gfx1250 behind an emulated RDMA NIC: the live kernel retires and the host-staged fallback passes with real byte counts, and injected faults are caught on both. Builds warning-free with --enable-rocm and hipcc, with --enable-rocm without it, and with neither. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Stephen Bates <sbates@raithlin.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this does
Adds
--data_validationsupport for ROCm device memory, which today is refused for--use_rocm.One commit, 10 files, +1837/−97. Four new files (
rocm_validation.{c,h},rocm_kernels.{h,hip}); the rest is build glue plus the changes described under Behaviour changes below.Depends on nothing unmerged. Builds clean with
--enable-rocmand hipcc, with--enable-rocmwithout hipcc, and with neither.Why
perftest validates host memory and CUDA memory. ROCm has neither a validation backend nor working copy ops, so
--data_validationis rejected at parameter-parse time for--use_rocm. That gap matters most exactly where a bandwidth number is least trustworthy: a wrong address translation moves the wrong bytes and the throughput figure looks fine.The ROCm memory backend also set all three copy ops to plain
memcpyinto and out of device memory. Those are nowhipMemcpywith the right direction.How it works
Two paths, mirroring the CUDA backend's shape.
Live (default).
src/rocm_kernels.hipis a port ofsrc/cuda_kernels.cu: a resident kernel where block 0 monitors the tail markers and blocks 1..N drain a lock-free queue, so each cycle is checked as it lands.Host-staged (fallback). Where the live kernel cannot run — a build without hipcc, a device the kernel will not launch on, or
PERFTEST_ROCM_VALIDATION=staged— the buffer is copied back and compared on the host. This is strictly weaker: it sees only each chunk's final state, so transient corruption that a later cycle overwrote is missed andbytes_validatedis correspondingly lower. Which path ran is printed at stop, and a live validator that observed no completed cycle falls back rather than reporting an empty result.Both reuse upstream's
classify_validation_mismatch(), so a run stopping mid-cycle is scored as an overwrite race rather than as corruption, consistently with the other backends.The port, and where it diverges from CUDA
Deliberately close to the original: same monitor/validator split, same queue, same structures from
cuda_validation.h, same exported entry points. Divergences are markedPORT:at the site. The ones worth a reviewer's time:Warp width — a correctness fix, not a tuning choice. The CUDA reduction hardcodes a 32-lane warp (
tid & 31, shuffle offset from 16). An AMD wavefront is 64 lanes on CDNA and 32 on RDNA, so the literal reduces only within half a wave and lets two lanes observelane == 0, double-counting the atomics that follow. The port useswarpSize, which is a built-in rather than a constant, so the#pragma unrolls go with it.Scoped atomics instead of
volatile. The CUDA code polls through*(volatile T *)pand orders with__threadfence_system().volatileis not a synchronisation primitive — it constrains the compiler for one access and says nothing about which agents it must be coherent with; it works on NVIDIA because the emitted loads happen to bypass cache. On AMD the memory scope selects which cache levels are bypassed, so this uses__hip_atomic_load/storewith it stated: system scope for the markers and slots the NIC DMAs into and for flags shared with the host, agent scope for the queue only this kernel touches. A uniform__threadfence_system()over-synchronised the queue while under-specifying the markers.Two
__ldgreads replaced, both on data the NIC is actively writing.__ldgis a read-only-cache hint asserting the data will not change for the kernel's life, which is false for a DMA target. The tail retry re-read through__ldgwas the load least likely to observe the write it was waiting for; and the forward-byte snapshot feedsclassify_validation_mismatch(), where a stale read biases toward reporting corruption that did not occur. The bulk compare loop keeps__ldg, ordered by the acquire on the marker.Poll loops back off with
s_sleep. A device that runs a wave until it blocks never retires a tight poll loop, so the kernel cannot observeshould_stop.validation_start()also honoursnum_blocks/threads_per_block, which the CUDA original accepts and(void)s — 33 blocks × 1024 spinning threads is not a geometry every device retires.rocm_kernels_available()is the one addition with no CUDA counterpart: the CUDA plugin answers that question by whetherlibperftest_kernels.sodlopen'd, while these kernels link intolibperftest.aand need a runtime probe.Behaviour changes outside the ROCm backend — please look here
data_validation_stop_and_report()is reordered: errors, then nothing-validated, then pass. This affects every backend.Testing "nothing validated" first reports a run whose chunks all mismatched (
errors > 0withbytes_validated == 0) as a warning about missing data, hiding a real failure. And backends setpassed = (errors == 0 && chunks > 0), so a run that observed no completed cycle also haspassed == 0and would otherwise print a failure with every error field still at its initialised zero:bytes_validated == 0is that same condition counted differently, since a chunk mismatching at its first byte contributes no compared bytes but still increments the chunk count.The ROCm dma-buf path no longer greps the kernel config before running. The old check required
CONFIG_DMABUF_MOVE_NOTIFY, which no longer exists upstream — dma-buf always builds with it now, and.move_notifybecame.invalidate_mappings— so the probe began refusing to run because support had become universal. What it gated was whetheramdgpu_dma_buf_pin()keptAMDGPU_GEM_DOMAIN_VRAMor fell back to GTT: driver policy, versioned with the driver rather than the running kernel, which on a DKMS install moves independently of it.CONFIG_PCI_P2PDMAfared no better, being blind to whether the NIC's driver implements.reg_user_mr_dmabuf. Both questions are already answered accurately byhsa_amd_portable_export_dmabuf()andibv_reg_dmabuf_mr(); their diagnostics are improved instead of being predicted. This also removesPERFTEST_ASSUME_DMABUF, which existed only to defeat that check.VALIDATION_INLINEnow also annotates__host__ __device__for__HIPCC__. hipcc compiles.hipas nvcc compiles.cu; both need it for the shared helpers to be callable from device code. No change to the CUDA path.configurelearns--with-rocm-arch, defaulting torocm_agent_enumerator. hipcc otherwise targets whatever GPU the build host reports, which is wrong whenever perftest is cross-built or built in a VM — and the symptom is "device kernel image is invalid" at launch rather than anything at build time.The
--data_validationrejection message now mentions--use_rocm.Environment variables
All default off; none changes behaviour unless set.
PERFTEST_ROCM_VALIDATION=stagedPERFTEST_ROCM_VALIDATION_STAGED=1PERFTEST_ROCM_VALIDATION_BLOCKS/_THREADSPERFTEST_ROCM_VALIDATION_FAULT=nPERFTEST_ROCM_HOST_ALLOC=1Testing, and its limits
Tested on an emulated gfx1250 behind an emulated RDMA NIC, two guests with real RDMA traffic between them. The live kernel retires, the host-staged fallback passes with real byte counts, and injected faults are caught on both paths — so a
PASSEDhere means the checker was looking, not merely silent.What that does not cover: no discrete AMD GPU, and no performance figures (the device has no real compute, so throughput numbers from it would be meaningless). Reviewers with Instinct hardware would be very welcome to run it.
Build-tested in containers across
--enable-rocmwith hipcc,--enable-rocmwithout hipcc, and a plain build — all warning-free.