Repository navigation
test(cuda): pin the allocating state on host-pinned pictures - #1649
Merged
Merged
Conversation
lusoris
force-pushed
the
fix/cuda-pic-prealloc-check
branch
from
October 1, 2026 00:43
cb11e87 to
48e3811
Compare
Upstream master's test_cuda_pic_preallocation dies with SIGSEGV in its host-pinned case. Checked on the RTX 4090: the fault is in default_release_pinned_picture() at `mov 0x18(%rdx),%rbp` with rdx = 0, a load of state->f through a NULL priv->cuda.state on the first unref. Upstream's vmaf_cuda_picture_alloc_pinned() sets only priv->cuda.ctx; the fix is the still-open Netflix/vmaf#1573, hunk (a). The fork is not affected. picture_cuda.c:260 sets the state on the pinned path. On the 4090 the fork's test_cuda_pic_preallocation passes 5/5, and so does upstream's own test source built against the fork once its handles start as NULL. The device test proves this only on a GPU host, so test_cuda_runtime_unwind gains test_pinned_picture_release_uses_the_allocating_state. It allocates a pinned picture through the fake driver table and unrefs it. It fails, without crashing, when the assignment is removed, and runs on any host with the nv-codec headers. The touched file is now clean under clang-tidy (CUDA lane: 8 findings on master, 0 here) and cppcheck: - the allocation checks free what they already allocated before returning; - the lifecycle-close test asserts through a helper after its frees; - the legacy runner is split in three. docs/state.md refreshes the Netflix/vmaf#1573 hunk (a) row with this evidence and the current line numbers. core/src/cuda/AGENTS.md and docs/rebase-notes.md record the invariant for upstream syncs.
lusoris
force-pushed
the
fix/cuda-pic-prealloc-check
branch
from
October 1, 2026 07:04
48e3811 to
1f0abd5
Compare
This was referenced Oct 1, 2026
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
1 participant
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
Upstream Netflix/vmaf master's
test_cuda_pic_preallocationdies with SIGSEGV in its host-pinned case due to a NULLpriv->cuda.statedereference indefault_release_pinned_picture()(Netflix/vmaf#1573 hunk a). The fork'score/src/cuda/picture_cuda.c:260already sets the state on the pinned path. This PR adds a device-free regression testtest_pinned_picture_release_uses_the_allocating_stateincore/test/test_cuda_runtime_unwind.cthat runs on any host with nv-codec headers, updates docs/state.md and AGENTS.md, and ships the changelog fragment.Type
feat— new featurefix— bug fixperf— performance improvementrefactor— no behavior changedocs— documentation onlytest— test-onlybuild/ci— tooling / infraport— cherry-pick from upstream Netflix/vmafsycl/cuda/simd— backend-specificChecklist
make format && make lintis green locally.python3 scripts/ci/run_meson_test.py -- -C build./cross-backend-diffand the worst ULP is ≤ 2..c/.cpp/.cu/.h/.hpp, it has the appropriate license header (seeCONTRIBUTING.md).!orBREAKING CHANGE:and the migration path is documented below.docs/adr/_index_fragments/<NNNN-slug>.mdand the slug is appended todocs/adr/_index_fragments/_order.txt— do not editdocs/adr/README.mddirectly (regenerated byscripts/docs/concat-adr-index.sh; see ADR-0221).Bug-status hygiene (ADR-0165)
docs/state.mdupdated in this PR with a row in the appropriate section (Open / Recently closed / Confirmed not-affected / Deferred), ORno state delta: REASON.Netflix golden-data gate (ADR-0024)
assertAlmostEqual(...)score in the Netflix golden Python tests.Cross-backend numerical results
Performance (if
perforfeat)Deep-dive deliverables (ADR-0108)
AGENTS.mdinvariant note — added to the relevant package'sAGENTS.md, OR "no rebase-sensitive invariants".changelog.d/<section>/<topic>.md(added/changed/deprecated/removed/fixed/security). Do not editCHANGELOG.mddirectly —scripts/release/concat-changelog-fragments.shrenders the Unreleased block from the fragment tree (see ADR-0221).docs/rebase-notes.mdunder a new ID, ORno rebase impact: REASON.Reproducer
Known follow-ups
None.
Breaking changes / migration
None.