Add: sub-classify scheduler no-progress timeout for host triage#1182
Merged
ChaoWao merged 1 commit intoJun 30, 2026
Merged
Conversation
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements a sub-classification mechanism for scheduler no-progress timeouts, enabling the host to diagnose device stalls using shared memory. The feedback identifies critical issues in the stall classification logic across both the a2a3 and a5 runtimes: scanning the ring buffer from index 0 instead of last_task_alive causes incorrect diagnostic counts due to slot reuse, and accessing the non-atomic slot_state.task pointer directly poses risks of null pointer dereferences and data races.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/a2a3/runtime/tensormap_and_ringbuffer/host/runtime_maker.cpp`:
- Around line 622-635: The timeout logging in runtime_maker.cpp is using a stale
snapshot from host_header, so sched_stall_detail and the locator fields can be
logged before the producer has finished publishing them. Refresh or re-read the
timeout snapshot inside the PTO2_ERROR_SCHEDULER_TIMEOUT branch before calling
LOG_ERROR, using host_header and the stall_detail_name/stall fields so the log
reflects the final published sub-class and locator values. Make sure the fix
keeps the self-diagnosing timeout line consistent with the host/device
publication order.
In
`@src/a2a3/runtime/tensormap_and_ringbuffer/runtime/scheduler/scheduler_cold_path.cpp`:
- Around line 442-455: The timeout publication order in scheduler_cold_path.cpp
is racy because latch_scheduler_error() exposes sched_error_code before the
sched_stall_* snapshot is fully visible, so the host in runtime_maker.cpp can
read code 100 and then see default stall fields. Fix this by making the stall
snapshot fully publish before sched_error_code becomes visible, or by changing
the host-side readiness check in runtime_maker.cpp to wait on sched_stall_detail
rather than the code alone. Use the existing latch_scheduler_error(),
sched_stall_detail, and sched_error_code paths to keep the host-visible timeout
data consistent.
In
`@src/a5/runtime/tensormap_and_ringbuffer/runtime/scheduler/scheduler_cold_path.cpp`:
- Around line 391-392: `classify_stall_reason()` is reading
`CoreExecState::running_slot_state` concurrently with scheduler
dispatch/completion updates, so the scan can race before `completed_` is
latched. Fix this by either making `running_slot_state` an atomic pointer in
`CoreExecState` and using atomic loads/stores in the relevant scheduler paths,
or by ensuring peers are fully quiesced before `classify_stall_reason()`
performs the core scan. Keep the change localized around
`classify_stall_reason()` and the code paths that update `running_slot_state`.
- Around line 441-452: Publish the scheduler stall snapshot before setting the
error code latch in the timeout path of scheduler_cold_path.cpp: the current
ordering in the latch_scheduler_error block can let read_runtime_status() and
the host observe PTO2_ERROR_SCHEDULER_TIMEOUT before sched_stall_* fields are
fully visible. Move the stall field stores in the timeout handling around
latch_scheduler_error so the snapshot fields on the header are written first,
and only then latch sched_error_code, keeping sched_stall_detail and the locator
fields consistent for readers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 958b0bea-ee8b-4703-bc77-640f2ab38afb
📒 Files selected for processing (14)
src/a2a3/runtime/tensormap_and_ringbuffer/common/pto_runtime_status.hsrc/a2a3/runtime/tensormap_and_ringbuffer/host/runtime_maker.cppsrc/a2a3/runtime/tensormap_and_ringbuffer/runtime/pto_shared_memory.hsrc/a2a3/runtime/tensormap_and_ringbuffer/runtime/scheduler/scheduler_cold_path.cppsrc/a2a3/runtime/tensormap_and_ringbuffer/runtime/scheduler/scheduler_context.hsrc/a2a3/runtime/tensormap_and_ringbuffer/runtime/shared/pto_shared_memory.cppsrc/a5/runtime/tensormap_and_ringbuffer/common/pto_runtime_status.hsrc/a5/runtime/tensormap_and_ringbuffer/host/runtime_maker.cppsrc/a5/runtime/tensormap_and_ringbuffer/runtime/pto_shared_memory.hsrc/a5/runtime/tensormap_and_ringbuffer/runtime/scheduler/scheduler_cold_path.cppsrc/a5/runtime/tensormap_and_ringbuffer/runtime/scheduler/scheduler_context.hsrc/a5/runtime/tensormap_and_ringbuffer/runtime/shared/pto_shared_memory.cpptests/ut/cpp/a2a3/test_shared_memory.cpptests/ut/cpp/a5/test_shared_memory.cpp
ChaoZheng109
force-pushed
the
fix/issue-1180-subclassify-scheduler-timeout
branch
4 times, most recently
from
c123453 to
9f26976
Compare
… device-error coverage Issue hw-native-sys#1180. Every device-side failure that surfaces as the AICPU scheduler no-progress timeout latches a single generic PTO2_ERROR_SCHEDULER_TIMEOUT (100) that the host cannot tell apart. Classify the stall into a small set of sub-reasons from state the scheduler already has, and propagate that sub-reason to host alongside the unchanged code 100 (backward compatible). Both arches. Feature: - classify_stall_detail() reduces the running/ready/waiting snapshot to one dominant sub-class (S1 running-stalled, S3 ready-but-idle, S4 dep-deadlock, S5 orch-starvation, UNKNOWN) -- a free-standing, unit-testable pure function. - The scheduler publishes the detail + a few locators into the SM header on the thread that wins the code-100 latch; validate_runtime_impl prints sub_class on the host failure line. Top-level sched_error_code stays 100. - classify_stall_reason() (and the pre-existing log_stall_diagnostics) scan only the live window [last_task_alive, current_task_index); scanning from 0 re-read each wrapped slot once per retired task_id, inflating the stall counts. The running-slot task pointer is snapshotted + null-guarded before deref. Platform fix: - device_runner_base finalize unloads the AICPU dispatcher binary before the device reset (not in ~LoadAicpuOp, which runs after the error-path force-reset + aclFinalize) -- the late unload dereferenced a torn-down RTS handle and segfaulted a5 at process exit. - Documented the teardown invariant in finalize_common (every RTS-using member is released there before the device reset; its destructor is an idempotent backstop). Audited the other RTS-owning members -- DeviceArena, MemoryAllocator -- which already follow it; LoadAicpuOp was the only gap. Closes hw-native-sys#1197. Tests (closes the issue's coverage-gap comment; the device-error -> host path was guarded for only 1 of 15 codes): - tests/st/runtime_fatal_codes/: e2e negative STs that drive each reachable code to its latch and assert the host surfaces it (sim: real -N; onboard: the watchdog may mask as 507xxx but the device class still reaches the host log). Covered: 1, 2, 3 (same-ring nesting), 4, 5, 7, 8, 9, 100 (S3 dep-pool starvation + S1 a real spinning AIC kernel), 101, 102. The pre-existing tests/st/ explicit_fatal (code 9, sim only) is folded in here (now sim + onboard). - Documented why the rest have no e2e trigger: 103 is pre-empted by ASYNC_WAIT_ OVERFLOW(102); SCOPE_TASKS_OVERFLOW(10) can never reach its cap (the rings physically fill first -> code 1 or 3); TENSORMAP_OVERFLOW(11) needs a 65536-entry flood; sub-classes S4/S5/UNKNOWN are design-unreachable defensive labels (classifier unit-tested). - cpput asserting new header init + classify_stall_detail priority table. Removed dead PTO2_ERROR_DEPENDENCY_OVERFLOW (6): zero references; the per-task "too many fanin" condition it named is now reported as DEP_POOL_OVERFLOW(4). Closes hw-native-sys#1180.
ChaoZheng109
force-pushed
the
fix/issue-1180-subclassify-scheduler-timeout
branch
from
9f26976 to
c297a68
Compare
ChaoWao
approved these changes
Jun 30, 2026
doraemonmj
pushed a commit
to doraemonmj/simpler_wc
that referenced
this pull request
Jul 1, 2026
… device-error coverage (hw-native-sys#1182) Issue hw-native-sys#1180. Every device-side failure that surfaces as the AICPU scheduler no-progress timeout latches a single generic PTO2_ERROR_SCHEDULER_TIMEOUT (100) that the host cannot tell apart. Classify the stall into a small set of sub-reasons from state the scheduler already has, and propagate that sub-reason to host alongside the unchanged code 100 (backward compatible). Both arches. Feature: - classify_stall_detail() reduces the running/ready/waiting snapshot to one dominant sub-class (S1 running-stalled, S3 ready-but-idle, S4 dep-deadlock, S5 orch-starvation, UNKNOWN) -- a free-standing, unit-testable pure function. - The scheduler publishes the detail + a few locators into the SM header on the thread that wins the code-100 latch; validate_runtime_impl prints sub_class on the host failure line. Top-level sched_error_code stays 100. - classify_stall_reason() (and the pre-existing log_stall_diagnostics) scan only the live window [last_task_alive, current_task_index); scanning from 0 re-read each wrapped slot once per retired task_id, inflating the stall counts. The running-slot task pointer is snapshotted + null-guarded before deref. Platform fix: - device_runner_base finalize unloads the AICPU dispatcher binary before the device reset (not in ~LoadAicpuOp, which runs after the error-path force-reset + aclFinalize) -- the late unload dereferenced a torn-down RTS handle and segfaulted a5 at process exit. - Documented the teardown invariant in finalize_common (every RTS-using member is released there before the device reset; its destructor is an idempotent backstop). Audited the other RTS-owning members -- DeviceArena, MemoryAllocator -- which already follow it; LoadAicpuOp was the only gap. Closes hw-native-sys#1197. Tests (closes the issue's coverage-gap comment; the device-error -> host path was guarded for only 1 of 15 codes): - tests/st/runtime_fatal_codes/: e2e negative STs that drive each reachable code to its latch and assert the host surfaces it (sim: real -N; onboard: the watchdog may mask as 507xxx but the device class still reaches the host log). Covered: 1, 2, 3 (same-ring nesting), 4, 5, 7, 8, 9, 100 (S3 dep-pool starvation + S1 a real spinning AIC kernel), 101, 102. The pre-existing tests/st/ explicit_fatal (code 9, sim only) is folded in here (now sim + onboard). - Documented why the rest have no e2e trigger: 103 is pre-empted by ASYNC_WAIT_ OVERFLOW(102); SCOPE_TASKS_OVERFLOW(10) can never reach its cap (the rings physically fill first -> code 1 or 3); TENSORMAP_OVERFLOW(11) needs a 65536-entry flood; sub-classes S4/S5/UNKNOWN are design-unreachable defensive labels (classifier unit-tested). - cpput asserting new header init + classify_stall_detail priority table. Removed dead PTO2_ERROR_DEPENDENCY_OVERFLOW (6): zero references; the per-task "too many fanin" condition it named is now reported as DEP_POOL_OVERFLOW(4). Closes hw-native-sys#1180.
3 tasks
ChaoWao
added a commit
that referenced
this pull request
Jul 11, 2026
…gbuffer (#1335) Follows PR #1327 (which synced hbg's scheduler to tmr's #1319 state). tmr then landed five more early-dispatch changes on the same subsystem; this ports them into host_build_graph's copies, preserving hbg's host-orchestration invariants (single ring, no execution-time reclaim, flat Runtime, no ACK-gate, host-side orchestrator). Absorbed from tmr: - #1304 Add early-dispatch for require_sync_start SPMD cohorts, incl. the drain_stage_cores parallel-drain refactor (handle_drain_mode two-stage). - #1326 Defer early dispatch until producer publication (correctness/race): published_block_count + record_published_blocks; propagate_dispatch_fanin is gated on published_block_count >= logical_block_num; the published_list ledger in dispatch and the stage_consumer_blocks two-sided publication/release handshake. - #1328 Shrink dispatch cold-write cost via a folded-gate src_payload (PTO2DispatchPayload: not_ready -> volatile uint64_t src_payload, 0 = ready, non-zero = gated source PTO2TaskPayload address; global_context to the tail; the PTO2_TASKPAYLOAD_*_OFFSET constants + static_asserts). build_payload writes args on the ready path and only src_payload on the gated path; the idle AICore fills args[] from src_payload during its doorbell wait; init-time prefill of async_ctx slab pointers/capacity + context-ptr args; deferred_slab reset moved off the dispatch path; software-pipelined prefetch of the next block. - #1329 Make early-dispatch doorbell ownership exclusive (correctness). - #1331 Serialize block range claims (correctness): every block-range claim now goes through slot_state->claim_block_range(...) instead of an inline next_block_idx CAS. Preserved host-orch divergences (not overwritten by tmr): PTO2_MAX_RING_DEPTH==1 with rings[]/ring_id removed; no advance_ring_pointers / reset_for_reuse / check_and_handle_consumed (completion via completed_tasks_, consumer wait keys on fanout_refcount); flat runtime->workers / runtime->func_id_to_addr_ (not runtime->dev.*); pto_runtime2_types.h keeps its dropped pto2_dispatch_payload.h include; hbg's AICore FIN-vs-swimlane-record ordering and its no-ACK-gate completion path (drains via completion-before-dispatch + backstop flushes); no device orchestrator (orchestrator_done_ absent, scheduler threads gate on runtime_init_ready_). Out of scope (unrelated tmr changes hbg still lacks, tracked for a follow-up): per-device scheduler timeout (get_scheduler_timeout_ms), PTO2_TENSOR_DATA_TIMEOUT MS-vs-cycles (#1189), stall sub-classification (#1182), runtime scope_tasks_cap (#1188). Verified: all 8 runtime targets build -Werror; a2a3sim host_build_graph scene suite 10/10; a2a3 onboard host_build_graph suite 10 passed / 1 skipped (paged_attention exercises the new claim_block_range serialization, drain_stage_cores drain, src_payload gated arg-fill, and published-block gating). tensormap_and_ringbuffer and the a5 runtimes are untouched.
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
2 participants
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.
Summary
PTO2_ERROR_SCHEDULER_TIMEOUT(code 100) is the catch-all the AICPUno-progress watchdog latches for every distinct device stall, so the
host can't tell a silently-hung AICore apart from a dependency deadlock, a
never-dispatched ready task, orchestrator starvation, or an
accounting-invariant violation. Triage means hand-reading the device log.
This classifies the stall at
handle_timeout_exitfrom state the scheduleralready holds and propagates a sub-reason to host. Top-level
sched_error_codestays 100 (backward compatible) — a newsched_stall_detailsub-class plus a few locator fields ride alongside itin the SM header. It also closes the issue's coverage gap (the
device-error → host path was guarded for only 1 of 15 codes).
Sub-classes (priority
RUNNING > READY > WAIT > orch-not-done > unknown):Changes
Feature — stall sub-classification
common/pto_runtime_status.h—PTO2_STALL_DETAIL_*constants, a pureclassify_stall_detail()decider, andstall_detail_name(). Free-standing→ unit-testable and identical on host and device.
sched_stall_detail+ locator atomics (completed/total, three raw counts, orch_done, S1 stuck
task_id/core), init'd to0/-1. Append-only keeps every existing
offsetofstable.scheduler/scheduler_cold_path.cpp—classify_stall_reason()scans thelive window
[last_task_alive, current_task_index)once (same ground truthas
log_stall_diagnostics);handle_timeout_exitpublishes the detail iffthis thread won the code-100 CAS and extends the
TIMEOUT_EXITlog line.latch_scheduler_errornow returns whether it won.host/runtime_maker.cpp— on code 100, print the sub-class + locators sothe failure line is self-diagnosing without a device-log dive.
Platform fix (a5 teardown segfault)
device_runner_base::finalize_commonunloads the AICPU dispatcher binarybefore the device reset (not in
~LoadAicpuOp, which runs after theerror-path force-reset + aclFinalize) — the late unload dereferenced a
torn-down RTS handle and segfaulted a5 at process exit. Documented the
teardown invariant (every RTS-using member released before the device reset;
idempotent destructors as backstop) after auditing the other RTS-owning
members (DeviceArena, MemoryAllocator already follow it). Closes [Code Health] Teardown ordering: release RTS resources before aclFinalize (load_aicpu_op_ stopgap is ad-hoc) #1197.
Cleanup
PTO2_ERROR_DEPENDENCY_OVERFLOW (6)(zero references; theper-task fanin overflow it named is now reported as
DEP_POOL_OVERFLOW(4)).Tests
tests/st/runtime_fatal_codes/— e2e negative STs that drive eachreachable code to its latch and assert the host surfaces it (sim: real
-N;onboard: the watchdog may mask as 507xxx but the device class still reaches
the host log). Covered: 1, 2, 3 (same-ring nesting), 4, 5, 7, 8, 9, 100
(S3 dep-pool starvation + S1 a real spinning AIC kernel), 101, 102. Folds in
the pre-existing
explicit_fatal(code 9, now sim + onboard).ASYNC_WAIT_OVERFLOW(102);SCOPE_TASKS_OVERFLOW(10)can never reach itscap (rings physically fill first → code 1 or 3);
TENSORMAP_OVERFLOW(11)needs a 65536-entry flood; sub-classes S4/S5/UNKNOWN are design-unreachable
defensive labels (classifier unit-tested).
classify_stall_detailpriority table.
Testing
runtime_fatal_codessim suite — 12/12 pass on a2a3sim + a5sim;scheduler_timeoutclassifies S3st-onboard-a2a3,st-onboard-a5)last_task_alive(was re-counting wrapped slots → inflated counts); running-slot task pointer snapshotted + null-guarded; publish-order + atomicity comments answered (post-run single-snapshot read, hot-path diagnostic)Fixes #1180
Closes #1197