Skip to content

debug(hip-tests): targeted workflow to reproduce prefetch flake (issue 8271) - #11750

Closed
anugodavar wants to merge 4 commits into
developfrom
users/agodavar/debug-prefetch-flake
Closed

anugodavar wants to merge 4 commits into
developfrom
users/agodavar/debug-prefetch-flake

Conversation

@anugodavar

Copy link
Copy Markdown
Contributor

Summary

Adds a manually-triggered workflow and a HIP_TESTS_REGEX env var to reproduce the intermittent hipMemPrefetchAsync failures reported in ROCm/TheRock#8271.

Failing tests:

  • Unit_hipMemPrefetchAsync_Basic_AllDevices — 42 == 0 (value mismatch at index 0)
  • Unit_hipMemPrefetchBatchAsync_SingleOperationSingleLocation — false == true

Root cause hypothesis: hipMemPrefetchAsync dispatches hsaKmtSVMSetAttr(HSA_SVM_ATTR_PREFETCH_LOC) on a CPU signal-handler thread. The kernel's migrate_pages() can silently fail to migrate some pages (due to concurrent access or memory pressure during parallel CTest execution). With XNACK disabled, a CPU read of an unmapped GPU-resident page returns stale/zero data instead of faulting and retrying. The failure is intermittent because migration only sometimes succeeds.

Changes:

  • .github/workflows/debug-prefetch-flake.yml — new workflow_dispatch workflow that runs only the two failing tests on gfx942 CSP runners, with a configurable repeat count (5–30 parallel matrix jobs) to stress the flake
  • projects/hip-tests/catch/packaging/run_hiptests.py — adds HIP_TESTS_REGEX env var support so ctest --tests-regex can target a specific subset without modifying test source

How to use:

  1. Get a TheRock artifact run ID from a recent gfx94X-dcgpu CI run
  2. Go to Actions → "Debug hipMemPrefetchAsync flake (issue 8271)" → Run workflow
  3. Set the artifact run ID and repeat count (10–30 recommended)
  4. Check per-matrix-job logs for 42 == 0 or false == true failures

🤖 Generated with Claude Code

Add a manually-triggered workflow that runs only the two intermittently
failing prefetch tests on gfx94X-dcgpu in parallel to increase the
probability of catching the failure:
  - Unit_hipMemPrefetchAsync_Basic_AllDevices
  - Unit_hipMemPrefetchBatchAsync_SingleOperationSingleLocation

Also add HIP_TESTS_REGEX env var support to run_hiptests.py so ctest
can be filtered to a targeted subset of tests for focused debugging.

Issue: ROCm/TheRock#8271

Co-Authored-By: Claude <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings September 17, 2026 08:44
@anugodavar
anugodavar requested review from a team as code owners September 17, 2026 08:44
@github-actions github-actions Bot added github actions Pull requests that update GitHub Actions code project: hip-tests labels Sep 17, 2026
@therock-pr-bot

Copy link
Copy Markdown

✅ All Policy Checks Passed

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ⚠️ Warning Error: Source/code files changed without an accompanying unit test.
Expected: add at least one test file named like test_<name>.py / test_<name>.cpp (or <name>_test.*).
Current: code file(s) changed: projects/hip-tests/catch/packaging/run_hiptests.py; no test file found
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —

🎉 All policy checks passed!

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

…tigation

Enable AMD_LOG_LEVEL=3 (info) with AMD_LOG_MASK=0x123 to capture:
  - LOG_API  (0x001): hipMemPrefetchAsync/hipStreamSynchronize call traces
  - LOG_CMD  (0x002): stream command dispatch and barrier packets
  - LOG_SIG  (0x020): signal creation and completion lifecycle
  - LOG_COPY (0x100): hsa_amd_svm_prefetch_async dispatch and completion

This will show whether the prefetch signal is being waited on correctly
and whether the migration actually completes before the CPU reads.

Co-Authored-By: Claude <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The workflow setup is broken and lacks required GPU-isolation configuration.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds a manual, repeatable workflow to reproduce the hipMemPrefetchAsync flake and supports targeted HIP test selection.

Changes:

  • Adds configurable parallel gfx942 debugging workflow.
  • Adds HIP_TESTS_REGEX support to the HIP test runner.
File summaries
File Summary
projects/hip-tests/catch/packaging/run_hiptests.py Adds regex-based CTest filtering.
.github/workflows/debug-prefetch-flake.yml Adds repeated prefetch-flake test jobs. Critical (3 votes): checkout/setup paths prevent execution. Moderate (1 vote): missing GPU-isolation environment configuration.
Review details

Suppressed comments (5)

.github/workflows/debug-prefetch-flake.yml:113

  • Only prejob/build_tools is fetched above, and the subsequent checkout is rocm-systems, so ./build_tools/github_actions/cleanup_processes.sh does not exist at the workspace root. Because this step runs with always(), every job will fail during cleanup even after a successful test run. Use the fetched tools path (or explicitly check out build_tools at the root).
        run: |

.github/workflows/debug-prefetch-flake.yml:77

  • The setup action's FETCH_ARTIFACT_ARGS input defaults to empty, while the hip-tests configuration requires --tests to download the catch test package. Without that argument, build/share/hip/catch_tests is not provisioned and the following test command cannot run the requested tests.
    .github/workflows/debug-prefetch-flake.yml:108
  • This invokes run_hiptests.py from the source tree, but derive_rocm_path() only accepts an installed .../share/hip/catch_tests layout. Since ROCM_PATH is not set here, the script raises before CTest starts; point it at the unpacked ROCm root (or invoke the installed copy).
          print("(query skipped - check rocminfo output above)")

.github/workflows/debug-prefetch-flake.yml:41

  • This runner is an OSSCI MIXXX GPU runner, for which the repository's other containerized GPU workflows pass /etc/podinfo/gha-gpu-isolation-settings (for example, rocprofiler-systems-continuous-integration.yml:160-168). Omitting that env file can run the matrix jobs without the required GPU-isolation configuration. Add the env-file option to this container.
      options: "--ipc host --user 0:0 --ulimit memlock=-1:-1 --ulimit nofile=1048576:1048576 --security-opt seccomp=unconfined --group-add video --device /dev/kfd --device /dev/dri -e ROCR_VISIBLE_DEVICES -e KUBE_CPU_REQUEST"

.github/workflows/debug-prefetch-flake.yml:102

  • The diagnostic step claims the SVM query is skipped, but it still loads libhsa-runtime64.so.1. The no-ROCm image only receives the ROCm libraries under the unpacked build tree, and no library path is configured for this standalone Python process, so this can raise OSError and abort the workflow before the tests. Remove the unused library load.
          echo ""
          echo "=== Large BAR (BAR0 size per GPU) ==="
          for f in /sys/class/drm/card*/device/resource; do
            awk 'NR==1{s=strtonum($1);e=strtonum($2);if(e>s)printf "  %s BAR0: %d MB\n",FILENAME,(e-s+1)/1024/1024}' "$f"
          done
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +68 to +69
- name: Checkout repository
uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0
anugodavar and others added 2 commits September 17, 2026 08:55
Two fixes:
1. Rewrite workflow to reuse therock-test-component.yml properly,
   matching the exact container/runner setup as failing CI runs.
2. Lower AMD_LOG_LEVEL from 7 to 3 (LOG_INFO): the installed release
   binary compiles out LOG_DEBUG (level 4) paths via compile-time
   AMD_LOG_LEVEL=0 macro — level 3 is the highest that produces output.

Co-Authored-By: Claude <noreply@anthropic.com>
AMD_LOG_LEVEL env var only enables LOG_INFO (level 3) in the installed
release binary because the ClPrint check for LOG_DEBUG paths is compiled
against a separate internal variable instance that the env-var path does
not reach. hipExtEnableLogging() writes to the correct internal variable.

Build a tiny LD_PRELOAD shim (enable_hip_log.so) that calls
hipExtSetLoggingParams(4, ...) + hipExtEnableLogging() via a constructor,
injecting it automatically when HIP_TESTS_REGEX is set. This surfaces:
  - HSA prefetch async dev_ptr / completion_signal values
  - Host wait on completion_signal
  - Full API call traces for hipMemPrefetchAsync/hipStreamSynchronize

Verified locally: LD_PRELOAD shim produces full level-4 log output
including the prefetch dispatch and signal-wait entries.

Co-Authored-By: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

github actions Pull requests that update GitHub Actions code organization: ROCm project: hip-tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants