Fix: kernel-mode capacity refusals no longer free what they protect (⑤K3) - #2193
Draft
sunkaixuan2018 wants to merge 2 commits into
Draft
Fix: kernel-mode capacity refusals no longer free what they protect (⑤K3)#2193sunkaixuan2018 wants to merge 2 commits into
sunkaixuan2018 wants to merge 2 commits into
Conversation
Kernel mode is simpler's second execution identity: instead of owning
the device, a context borrows the caller's already-current device and
stream to enqueue one bounded asynchronous operator per launch, so a
PyPTO program is capturable by ACLGraph as an ordinary node. This
change freezes the public surface that identity hangs off and gives the
context a write-once identity the guards can key on. It creates no
resources. The program path gains one call — simpler_init latches
PROGRAM — and no behavior: latching a fresh context always succeeds, is
idempotent, and nothing on that path reads the latch.
- runtime_c_api.h declares the lifecycle entries
simpler_kernel_mode_{supported,init,prepare_callable,launch} and adds
the host-band code PTO_RUNTIME_ERR_INVALID_STATE. The existing
finalize_device stays the fifth lifecycle entry, and a kernel context
now reaches it. Kernel-mode capacity is
a mode invariant rather than a gated state: config is context-static,
so each pooled arena region is committed at most once, and
setup_static_arena reports a grow or release request on a committed
region under kernel mode as an internal invariant break; capacity
intent travels in CallConfig.runtime_env like everywhere else.
- Execution identity is a write-once property of the context rather
than a state that evolves. ExecutionModeLatch (platform/include/host/
execution_mode_latch.h) replaces the four-state claim: the first init
entry to run latches the mode, and it never changes — not on finalize,
not on error. simpler_init latches PROGRAM before touching any
process or runner state, so the program/kernel mutual exclusion is
enforced on every program init instead of resting on a separate
declaration call. There is no unlatch, which the latch documents as a
consequence: a handle from a failed kernel init can never be recycled
into a program context. SimplerExecutionMode now has one definition
(task_interface/execution_mode.h) that both the wire header and the
latch consume, so the host-side identity and the value that travels to
the AICPU can no longer disagree.
- device_id_ records which device a context is on, not a claim on it —
ownership is what the latch carries. attach_current_thread splits
accordingly: bind_current_thread does the per-thread rtSetDevice and
nothing else; attach_current_thread is the program-mode adopt
(bind plus the one-shot op-execute watchdog and identity write) and
refuses on a kernel latch; adopt_borrowed_device records the device a
kernel context runs on without binding the thread and without
configure_aicore_op_timeout, whose aclrtSetOpExecuteTimeOutV2 would
rewrite the watchdog for every other user of a borrowed card. It does
resolve the timeout config, because the stream and scheduler timeouts
derived from it are read on both identities. DeviceRunner::finalize()
is the one caller that runs under both identities and skips the bind
on a kernel latch, so the kernel close path reaches its no-reset
branch instead of being turned away by a device bind it never needed.
- ensure_acl_ready(), force_reset_device(), and finalize()'s rt-layer
device reset refuse on a kernel-mode context (a2a3 + a5): the ACL
lifecycle belongs to the caller, and every call site of the five ACL
lifecycle APIs falls into three enumerable classes (below the
ensure_acl_ready guard, inside force_reset_device behind its own
guard, or gated on acl_ready_ which only the guarded path sets), with
finalize's rt-layer reset intercepted by its own kernel-mode branch —
so poison recovery can never reset the device out from under the
host process.
- kernel_invocation_header.h pins the envelope every kernel launch
ships to the AICPU (mode / callable / generation / payload length /
int32_t arg counts). Both sides of the wire come from one
build_runtimes.py build, so the struct carries no version or size
negotiation and the POD/standard-layout guards are its only
compile-time checks. generation is the occupancy counter of the
residency slot callable_id resolves to - a property of the slot, not
of the callable in it, so a generation carried by the callable could
not detect slot reuse - with zero reserved for "not recorded".
ChipCallable's sig_count includes the scalar entries and its
scalar_count reads 0 both for a scalar-free orchestration and for an
artifact built before the field existed, so a consumer derives the
effective scalar count - the field when nonzero, otherwise the
signature's SCALAR entries, the split count_callable_tensor_args
already computes - and compares tensor_count against sig_count minus
it. Subtracting the field directly would count an unrecorded
callable's scalars as tensors.
- Kernel-entry argument validation is shared by all eight host-runtime
components through kernel_entry_validation.h (one copy of the
null/range/image-size/alignment checks; a binary pointer and its size
must be present or absent together, and a callable image must be
aligned for ChipCallable so its CALLABLE_CHILD_ALIGN-relative storage_
lands aligned too), so a stub and a real implementation accept and
reject exactly the same arguments.
- KernelExecutionState and ExecutionModeClaimState carry the kernel
context phase machine (New/Collecting/ReadyEnqueued/Poisoned/
Closing/Closed with sticky, retriable Closing and separate
runtime-error and teardown-error slots) and the two restricted
operation vocabularies; synchronize, allocation, capture queries,
and model attachment stay unrepresentable in those tables, and a
launch implementation is obligated to route through them. Every
kernel-mode guard reads the identity through ExecutionModeLatch::
is_kernel() rather than comparing an enumerator at the call site, so
the test lives in one place instead of eight.
- ChipWorker dlsyms the four new symbols from every runtime, so a
component missing one fails at load, and clears them alongside the
other resolved pointers on all three teardown paths so none is left
dangling into the library DlHandleGuard dlcloses.
test_host_runtime_abi.py
asserts the export across all eight components, and table-driven UTs
cover the phase machine (including failed-rollback landing in
Closing with the create error reported and the cleanup error
latched), the shared argument validation, and the wire layout.
- ChipCallable additionally records scalar_count as a cached
derivation of the signature's SCALAR entries: make_callable rejects
a nonzero count that disagrees with the signature, while 0 also
means "not recorded" (legacy blobs read 0). The field occupies four
bytes of historical header tail padding, so every historical offset,
sizeof, and the kernel-cache ABI token are unchanged;
ChipCallable.build gains a trailing scalar_count=0 keyword and a
read-only property.
Two facts a reader should not have to re-derive. The latch refusal returns
PTO_RUNTIME_ERR_INVALID_STATE (-1003) rather than PTO_RUNTIME_ERR_INTERNAL
(-1000) on purpose: conftest.py scrapes "simpler_init failed with code <N>"
and treats -1000 as a poisoned card, so an identity conflict must not look
like one. And kernel_execution_state.cpp stays compiled into all four host
runtimes even though grepping KernelExecutionState now finds only its own
header and .cpp — it is the persistent-state change's foundation, not an
orphaned translation unit.
Every kernel-mode branch this adds is provably dead in this commit: no
production site latches KERNEL (`git grep 'latch(SIMPLER_MODE_KERNEL)' src`
is empty) because both simpler_kernel_mode_init stubs return before any latch
call, so is_kernel() is false on every context and the program path takes the
same branch it took before.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks 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 |
sunkaixuan2018
force-pushed
the
skx/kernel-capacity-freeze-k3
branch
from
September 11, 2026 01:52
eb69687 to
4297a35
Compare
A kernel-mode context's device buffers are sized once and keep their addresses, because a captured graph replays the addresses of the run it captured; a re-base there produces silently wrong data rather than a failure. Two refusals that enforce this were themselves destructive. setup_static_arena's guard returned an error the caller collapsed into a rollback that released all three regions, so a fired guard dropped the base addresses it had just declined to move. The bank's commit rule now lives in host/static_arena_bank.h, shared by the onboard and simulation runners rather than duplicated in both, and it separates the two failures: an allocation failure still rolls the whole bank back, while a capacity refusal leaves every region committed and every cached size intact. RetainedTempBump::begin freed the retained buffer before asking for a larger one, so a refusal could only land after the address was already gone. It now refuses ahead of the free. The refusal is scoped to a slot that already holds a buffer: an empty slot has no address for a captured graph to hold, so a context's first allocation is not a re-base and is taken normally, which is what lets a kernel context reach its frozen size at all. Nothing in the tree restricts a kernel context's tensors to device memory, so that first allocation is reachable. The context's identity reaches the runtime through a new HostApiOps entry, is_kernel_mode. It is a query of the existing latch, not a new gate, and a table that does not supply it reports program mode. Program mode keeps growth, release and rollback unchanged. Both arena call sites keep their prebuilt-arena cache invalidation, now keyed on whether the commit moved a base. The contracts that described the behavior these guards make conditional move with them: the setup_static_arena and retained-temp-buffer entries in host_api.h, DeviceRunnerBase::setup_static_arena's rollback promise and its return code, the kernel-mode capacity paragraph in runtime_c_api.h, both runners' latch comments, task-flow.md, and RUNTIME_LOGIC.md in both architecture trees. tests/ut/cpp/common/test_static_arena_bank.cpp covers the bank rule with no device: base and capacity constant across repeated calls, zero allocator calls, exact capacity accepted, one byte over refused with PTO_RUNTIME_ERR_INTERNAL, and the held layout still served afterwards. The TRB suite gains the same coverage on the retained temporary buffer for both architectures, including a context that is in kernel mode from its first bind, which is the only shape the write-once latch permits. Both suites pin the other exit too: an allocation failure rolls the whole bank back, in kernel mode as in program mode, since a setup that never completed published no address to protect. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
sunkaixuan2018
force-pushed
the
skx/kernel-capacity-freeze-k3
branch
from
September 11, 2026 02:09
4297a35 to
b8e739d
Compare
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 is
A kernel-mode context's device buffers must keep their addresses for the life of
the context, because an ACLGraph replays the addresses that were captured. Every
runtime-time growth is
release + reserveorfree + malloc, which re-bases; thefailure mode is silently wrong data on replay, not a crash. That makes this a
correctness change, not a sizing one.
K1 landed the first guard on that invariant, in
setup_static_arena. Itspredicate is right and is unchanged here. What this PR fixes is that its
refusal, and the refusal on the second growth point, were both destructive.
Two refusals that destroyed what they protected
1.
setup_static_arena— the guard freed the bases it had just declined tomove. The refusal returned
PTO_RUNTIME_ERR_INTERNAL, the caller collapsed thatto
ok = false, and the!okblock released all three regions unconditionally.DeviceArena::release()frees the backing buffer, so a fired guard droppedexactly the base addresses it names in its own error message. K1's comment above
the guard said so; nothing acted on it.
2.
RetainedTempBump::begin— the free came before anything could refuse. Thegrow path is
device_free(old)thendevice_malloc(bigger). A refusal placedanywhere downstream of that — including at the platform's
device_malloc—arrives after the address is already gone, and the existing failure path then
clears the slot to
{nullptr, 0}. The guard has to sit ahead of the free.The invariant both fixes share, and the one every later growth-point guard will
need: a refusal must have no side effects.
What changed
host/static_arena_bank.h(new)setup_static_arenabodiesbases_changedRetainedTempBump::begin(a2a3 + a5)device_free, leaving the slot as the previous run left itHostApiOps::is_kernel_mode(new)is_kernel_modeis a query of the existingExecutionModeLatch, not a newswitch — no environment variable and no macro, per
env-macro-gating.md§1. Its producer(
c_api_shared.cpp) and its consumer (runtime_maker.cpp) are compiled into thesame
libhost_runtime.so, so appending to the ops table creates no ABI skew.The retained-buffer guard refuses a re-base, not an allocation
This is the part worth a reviewer's attention, because I got it wrong first.
The guard fires on
required > size && addr != nullptr. Theaddr != nullptrhalf matters: an empty slot holds no address a captured graph could reference, so
a context's first allocation moves nothing and is taken normally. Without that
clause the guard refuses the first bind too, and since the grow path is the only
writer of the slot, the slot would stay
{nullptr, 0}forever — a kernel contextcould never reach its frozen size at all.
My first version omitted it, on the premise that a kernel run never stages host
tensors anyway. That premise is not enforced anywhere in the tree, and I could
not support it on a re-read:
validate_kernel_launch_argschecks null-ness and the callable-id range. Itnever inspects tensors.
ChipTensor::address_spacedefaults toHOST, so the default goes theopposite way.
SimplerKernelInvocationHeader::host_copy_tensor_countexists precisely tocarry host-memory args in kernel mode, documented as "zero until the host-only
copy contract lands". Host tensors there are a planned contract, not an excluded
one.
So the narrow guard is the correct one: it still refuses every real re-base, and
it refuses nothing else.
Doc updates in the same commit
The behavior these guards make conditional was described in six places, all of
which now read false for a kernel context and move with the code:
setup_static_arenaand the retained-temp-buffer entries inhost_api.h,DeviceRunnerBase::setup_static_arena's rollback promise (and its@return,which named
-1for a function that returnsPTO_RUNTIME_ERR_INTERNAL), thekernel-mode capacity paragraph in
runtime_c_api.h, both runners' latch comments,docs/task-flow.md, andRUNTIME_LOGIC.mdin both architecture trees.The extraction is a flagged deviation from "program path byte-identical"
Behavior in program mode is unchanged —
kernel_modeis false there, so the newbranch is unreachable and growth, release and rollback all take the paths they
took before. But the code moved, so this is not literally untouched, and it is
deliberate for two reasons. The rule was two hand-maintained copies that
codestyle.md§10 warns about, and onboard/simsymmetry is now structural rather than a review obligation. And it is what makes
the fix testable without a device:
DeviceRunnerBaseneeds CANN, the rule needsnothing.
The
allocate_tensordesign question, answeredThe handover asked whether kernel mode should refuse
allocate_tensorunconditionally, or only outside a prepare window borrowed from ④K2. The
unconditional form holds: #2176's prepare-once allocator reaches
mem_alloc_directly through its own context ops, never through
HostApi::device_malloc, sorefusing the runtime-facing surface would not block a legitimate kernel-time
allocation. This PR does not install that refusal, because the growth point it is
in scope for needs its guard higher up anyway — but the question is settled and the
next guard does not have to re-derive it.
Out of scope, and one question for a reviewer
Growth points #1
acquire_graph_definition_blockand #2 HBG host-tensor stagingare not closed here. Both are HBG-only and both live in files #2173 rewrites;
#2173 already carries a disposition for the definition block. @TaoZQY — do you want
to keep #1 (and #2) on your side, or should a follow-up here close both against the
is_kernel_modequery this PR adds? Splitting one growth point across two PRs isthe outcome worth avoiding. Note that
acquire_graph_definition_blockisgrow-by-replacement of a device block, so it is the same silent-replay hazard,
not a lesser one.
DFX is outside the frozen capacity, but it is not address-stable. This is the
question the handover asked, so here is the full answer rather than half of it. The
DFX device buffers — the device-wall buffer and the collectors' workspaces — are
separate
mem_alloc_allocations, not regions of an arena bank, so nothing this PRfreezes covers them and it correctly writes no DFX teardown logic. The device-wall
buffer is allocated lazily once and never grown, so it does not re-base. The
collector pools do:
prepare_executioncallsfinalize_collectors()whenevercollector_shape_is_stale(...), so a run whose core or AICPU-thread counts differfrom the previous one tears their device memory down and rebuilds it at a new
address. A kernel context's shape is context-static, so that should not fire — but
nothing enforces it, and if the DFX buffers are ever reachable from a captured
graph this is a growth point in its own right. Flagging it rather than guarding it
here: it belongs with whoever owns DFX under the execution claim (#2163).
The sim-side guard stays. Simulation never latches KERNEL, so that branch is
never true, but removing it would break the onboard/sim symmetry the stub-parity
argument rests on.
Verification
All on myserver (aarch64, CANN 9.0.0). No device is needed for any of it.
86dd62b4is 143/143, built and run the same waytest_static_arena_bankno_hardwarelabeltest_trb_runtime_temp_buffertest_a5_trb_runtime_temp_bufferlibhost_runtime.sobuilt (a2a3 + a5 × onboard + sim × hbg + trb); no warning names a changed filetests/ut/py/test_host_runtime_abi.pyThe five acceptance criteria are covered twice, once per slot kind:
RepeatedCommitsMoveNoBaseAndCallNoAllocatorKernelModeHoldsOneAddressAcrossRepeatedRunsDeviceArena::alloc_count()ExactAndSmallerRequestsAreServedInPlaceINTERNALOneByteOverCapacityIsRefusedAndChangesNothingKernelModeRefusesToGrowTheRetainedBufferKernelModeRefusalLeavesTheRetainedBufferUsableKernelModeAllocatesOnceThenRefusesToGrowis the production shape: the latch iswrite-once, so a real context is in kernel mode from its very first bind. It
asserts one allocation, then a refusal, then the sized run still binding from the
same address.
The other exit is pinned too.
AllocationFailureRollsTheWholeBankBackandAllocationFailureOnTheFirstCommitStillRollsBackdrive a failing backingallocator through
DeviceArena's injectable alloc/free, so the rollback path isreachable with no device: an allocation failure releases every region and zeroes
every remembered size, in kernel mode exactly as in program mode, because a setup
that never completed published no address to protect.
Negative controls
A test that passes against the fix is not evidence it would have caught the
defect, so each defect was re-introduced and the suites re-run.
capacity_refusedbranch): 3 kernel-mode arena tests fail, and bothprogram-mode tests still pass.
a5; the rest pass.
addr != nullptrclause: exactlyKernelModeAllocatesOnceThenRefusesToGrowfails, on both architectures — thefirst-bind regression has a precise barrier.
fail; the nine capacity tests still pass, so the two failure classes are
pinned independently of each other.
How the findings above were found
The commit was put through a self-review before this update: eight independent
reviewers over separate dimensions of the diff, then three adversarial refutation
lenses per finding. It raised 22 findings and the skeptic panel refuted all 22 —
a verdict I do not report as a clean bill of health, because the refutation lens
set included reachability, and since nothing latches KERNEL yet, every
kernel-mode finding is trivially unreachable today. The panel's value was in
surfacing candidates, not in adjudicating them.
Four of those candidates were textually verifiable and are fixed here regardless
of the vote: the first-allocation refusal, the
is_kernel_modecomment claimingtable-wide address stability that
acquire_graph_definition_blockcontradicts,the stale contracts listed earlier, and the two test defects above — the
"smaller" run that packed to exactly the retained capacity, and the missing
rollback coverage.
🤖 Generated with Claude Code