[rocprofiler-sdk] Fix queue interposition completion waiter ordering - #7924
Conversation
Clamp the async signal handler pool size when GPU_MAX_HW_QUEUES resolves to zero, and defer completion-signal waiter enqueue until after process_doorbell_impl publishes the final doorbell. This prevents PTL async enqueue or inline execution from waiting on packets that the GPU cannot observe yet.
❌ PR Check — Action Required
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🚫 Please fix the failed policies before requesting reviews. The following policy checks failed:
The |
There was a problem hiding this comment.
Pull request overview
This PR addresses a potential hang in rocprofiler-sdk’s inline HSA queue interposition path by ensuring completion-signal waiters are not enqueued until after the real write index has been published and the final doorbell has been rung.
Changes:
- Defers async completion-waiter enqueue from
write_interceptor()toprocess_doorbell_impl()(afterpublish_submitted_packets()). - Refactors async-signal-handler thread-count selection into
get_async_signal_handler_thread_count(). - Adds unit tests covering env-driven thread-count selection and clamping behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/tests/queue_interposition.cpp | Adds unit tests for async-signal-handler thread-count selection via env vars. |
| projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/hsa/queue_interposition.hpp | Exposes get_async_signal_handler_thread_count() (and includes <cstddef> for size_t). |
| projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/hsa/queue_interposition.cpp | Implements deferred waiter scheduling and centralizes thread-count env logic. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
While i don't have opposition to this change.
This shouldn't matter. The async just wont trigger until the completion signal is set. Its not clear why this change fixes anything other than changing the ordering and holding the lock for potentially less time. |
rahulc-gh
left a comment
There was a problem hiding this comment.
LGTM - wait for CI to Pass
Motivation
transpose-samplingtest from rocprofiler-systems can hang because queue interposition may start the async completion-signal waiter before the final real doorbell is published, so the waiter can block waiting for GPU completion while the GPU has not yet been told about the packet.Technical Details
write_interceptor()toprocess_doorbell_impl(), afterpublish_submitted_packets()publishes the real write index and rings the doorbell.JIRA ID
Test Plan
Test Result
Submission Checklist