guard+cleanup(cuda): DtoD-via-pinned pre-commit guard + delete orphan HER
Two related changes installing the structural guard against the SP6 Pearl 5 IQN τ failure mode (root cause fixed atfacbf76ebfor that one site) and removing the only remaining orphan callers of the broken pattern. The bug class. The mapped_pinned::{upload,clone_to_device}_{f32,i32}_via_pinned helpers are named to suggest "no HtoD per feedback_no_htod_htoh_only_mapped_pinned" but their bodies do MappedXBuffer::new() + memcpy_dtod_async() + stream.synchronize(). The DtoD copy and synchronize are both forbidden inside CUDA Graph capture (CUDA_ERROR_STREAM_CAPTURE_INVALIDATED) and add a host stall otherwise. The canonical pattern is MappedXBuffer stored directly + write_from_slice + kernel reads via .dev_ptr, used by SP4 portfolio_state and SP6 IQN τ atfacbf76eb. Guard. New check_no_dtod_via_pinned in pre-commit-hook.sh rejects any staged .rs file calling upload_(f32|i32)_via_pinned or clone_to_device_(f32|i32)_via_pinned, except mapped_pinned.rs itself. Per feedback_no_hiding: no suppression marker. Also fixes a pre-existing silent-skip bug: the gpu-hotpath-guard.sh invocation used $(cd "$(dirname "$0")" && pwd) which resolved to .git/hooks/ (the symlink's directory) instead of scripts/, so the guard never ran. Replaced with readlink -f "$0" + an explicit "guard missing" error branch — silent skip is worse than no guard. Orphan deletion. gpu_her.rs carried legacy relabel_batch, generate_random_donors (CPU), HerBatch, slice_clone_f32, slice_clone_i32 — zero production callers (verified via grep). Production uses relabel_batch_with_strategy + generate_random_donors_gpu. The orphan held the only upload_i32_via_pinned callers in the codebase; per feedback_no_hiding the right fix is delete. Scope. Eliminates 2 of 47 production *_via_pinned call sites. Remaining 45 across 14 files are cold-path init — graph-capture-fragile and host-stalling but not breaking operationally. Guard enforces no new calls; existing 45 migrate in subsequent atomic per-buffer commits. After all 45 are converted, the four helpers themselves get deleted from mapped_pinned.rs. Validation. Smoke smoke-test-82fjk atfacbf76ebsucceeded — magnitude differentiation restored (q_full=0.462 > q_half=0.409 > q_quarter=0.350 vs baseline frozen Pascal-triangle 0.225/0.280/0.495), eval distribution unfrozen (eq=0.596, eh=0.404, ef=0.000 vs baseline single-action collapse). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -57,9 +57,15 @@ if [ -n "$STAGED_FILES" ]; then
|
||||
fi
|
||||
fi
|
||||
|
||||
# Resolve script directory by following the symlink to the real location.
|
||||
# When invoked via .git/hooks/pre-commit symlink, $0 points at the symlink
|
||||
# (.git/hooks/) — `dirname $0` would silently miss the sibling guard scripts
|
||||
# in scripts/. `readlink -f` resolves through the symlink chain to the real path.
|
||||
REAL_SCRIPT="$(readlink -f "$0")"
|
||||
SCRIPT_DIR="$(cd "$(dirname "$REAL_SCRIPT")" && pwd)"
|
||||
|
||||
# GPU hot-path leak detection
|
||||
echo "🔎 Checking for GPU→CPU leaks in hot paths..."
|
||||
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
|
||||
if [ -x "$SCRIPT_DIR/gpu-hotpath-guard.sh" ]; then
|
||||
if ! "$SCRIPT_DIR/gpu-hotpath-guard.sh" --staged; then
|
||||
echo "⛔ GPU hot-path leak detected — fix or suppress with // gpu-ok: <reason>"
|
||||
@@ -67,8 +73,59 @@ if [ -x "$SCRIPT_DIR/gpu-hotpath-guard.sh" ]; then
|
||||
fi
|
||||
echo " No GPU→CPU leaks in hot paths"
|
||||
echo ""
|
||||
else
|
||||
echo "❌ Pre-commit guard missing: $SCRIPT_DIR/gpu-hotpath-guard.sh not found or not executable"
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# DtoD-via-pinned guard: zero use of upload_*_via_pinned or
|
||||
# clone_to_device_*_via_pinned anywhere in production code.
|
||||
#
|
||||
# These helpers internally do memcpy_dtod_async + stream.synchronize() — the
|
||||
# "via_pinned" name is a lie. They cause CUDA_ERROR_STREAM_CAPTURE_INVALIDATED
|
||||
# inside any captured graph and force a host-side stall otherwise. There is no
|
||||
# legitimate use case: every CPU↔GPU communication path must use
|
||||
# MappedF32Buffer / MappedI32Buffer directly (cuMemHostAlloc DEVICEMAP — host
|
||||
# writes via host_ptr, kernel reads dev_ptr, zero copy).
|
||||
#
|
||||
# Per feedback_no_hiding: no suppression marker. If you find yourself wanting
|
||||
# to suppress, rewrite the call site instead.
|
||||
#
|
||||
# Reference: gpu_iqn_head.rs Pearl 5 τ buffers (commit facbf76eb) — converted
|
||||
# from CudaSlice + upload helper to MappedF32Buffer per-branch arrays.
|
||||
check_no_dtod_via_pinned() {
|
||||
local staged
|
||||
staged=$(git diff --cached --name-only --diff-filter=ACM | grep '\.rs$' || true)
|
||||
if [ -z "$staged" ]; then return 0; fi
|
||||
|
||||
local bad=""
|
||||
while IFS= read -r f; do
|
||||
[ -z "$f" ] && continue
|
||||
# Only skip the helpers' definitions inside mapped_pinned.rs itself
|
||||
# (where they currently live; the structural fix removes them entirely).
|
||||
[[ "$f" == *"cuda_pipeline/mapped_pinned.rs" ]] && continue
|
||||
|
||||
local hits
|
||||
hits=$(grep -nE 'upload_(f32|i32)_via_pinned|clone_to_device_(f32|i32)_via_pinned' "$f" 2>/dev/null \
|
||||
| grep -vE '^[0-9]+:[[:space:]]*//' \
|
||||
|| true)
|
||||
if [ -n "$hits" ]; then
|
||||
bad+="${bad:+$'\n'}$f"$'\n'"$hits"
|
||||
fi
|
||||
done <<< "$staged"
|
||||
|
||||
if [ -n "$bad" ]; then
|
||||
echo "❌ DtoD-via-pinned guard violation: upload_*_via_pinned / clone_to_device_*_via_pinned"
|
||||
echo " These helpers do memcpy_dtod_async + stream.synchronize() —"
|
||||
echo " graph-capture-incompatible AND host-stalling. There is no"
|
||||
echo " legitimate use. Rewrite to MappedF32Buffer / MappedI32Buffer"
|
||||
echo " stored directly (host_ptr writes; kernel reads dev_ptr; no copy)."
|
||||
echo " No suppression marker — fix the structure."
|
||||
echo "$bad" | sed 's/^/ /'
|
||||
return 1
|
||||
fi
|
||||
}
|
||||
|
||||
# DQN v2 Invariant 7 enforcement: component-adding commits must update audit docs.
|
||||
check_audit_doc_updates() {
|
||||
local staged=$(git diff --cached --name-only)
|
||||
@@ -122,6 +179,7 @@ check_no_isv_migrations() {
|
||||
check_audit_doc_updates || exit 1
|
||||
check_no_todo_fixme || exit 1
|
||||
check_no_isv_migrations || exit 1
|
||||
check_no_dtod_via_pinned || exit 1
|
||||
|
||||
echo "✅ All pre-commit checks passed!"
|
||||
echo ""
|
||||
|
||||
Reference in New Issue
Block a user