Skip to content

cuda: backport upstream revert of "reset cuda context after reading memory size" (#24715) - #238

Merged
TheTom merged 2 commits into
TheTom:feature/turboquant-kv-cachefrom
min23asdw:fix/cuda-device-reset-after-model-reload
Jul 30, 2026
Merged

cuda: backport upstream revert of "reset cuda context after reading memory size" (#24715)#238
TheTom merged 2 commits into
TheTom:feature/turboquant-kv-cachefrom
min23asdw:fix/cuda-device-reset-after-model-reload

Conversation

@min23asdw

@min23asdw min23asdw commented Jul 27, 2026

Copy link
Copy Markdown

Close #237

### Suggested fix

Only reset a device that has never been used. Add an ever_active flag to
ggml_backend_cuda_device_context , set it at the three sites that do active_count++
(buffer alloc, host buffer alloc, backend init), and gate the reset on it:

if (ctx->active_count == 0 && !ctx->ever_active) {
CUDA_CHECK(cudaDeviceReset());
}

This preserves the original intent -- avoiding a lazily created context that permanently
holds VRAM when memory is queried before anything has been allocated -- while never
resetting a context whose kernel state other code still assumes is live.

- [x] Before the fix, llama-bench --n-cpu-moe 32,34,36,38,40,42,44 (6 model reloads)
aborts with CUDA error: invalid argument right after the first configuration
- [x] With the fix, the same sweep completes all 14 rows with no abort

Why a revert rather than a targeted fix

The first commit in this PR (f3c29b7) gated the reset behind an ever_active flag so a
device that had already been used would never be reset. It works, but upstream has
already settled this differently:

This branch has diverged from upstream at this point: it carries 0f7fada56 but not the
revert. The second commit (af8ac64) cherry-picks ea21e0395, which supersedes the
first commit and brings this file back in line with upstream.

  • Before: llama-bench --n-cpu-moe 32,34,36,38,40,42,44 -ngl 99 -ctk turbo4 -ctv turbo3 -b 4096 -ub 2048 aborts with CUDA error: invalid argument
    immediately after the first configuration
  • After: the same sweep (7 configurations, 6 model reloads) completes all 14 rows
    with no abort
  • Release build with -DGGML_CUDA=ON, no errors

@min23asdw
min23asdw marked this pull request as draft July 27, 2026 19:45
@min23asdw

min23asdw commented Jul 27, 2026

Copy link
Copy Markdown
Author

I double check in ggml-org it already know this issue
https://github.com/ggml-org/llama.cpp/pull/24682 -> CLOSED
the fix ggml-org#24328 <--- same like i do.
ref
ggml-org#24374 (crash during model load)
ggml-org#24608 (slow 20x)

the ggml-org pull/24682 they not fix they revert
ggml-org#23935
ggml-org#24715

@min23asdw

Copy link
Copy Markdown
Author

test [OK] - model reload (7 configs, 6 reloads) for single GPU CUDA sm_89

not test - multi-GPU

@min23asdw min23asdw changed the title cuda: prevent device reset after model reload if previously active cuda: backport upstream revert of "reset cuda context after reading memory size" (#24715) Jul 27, 2026
@min23asdw
min23asdw marked this pull request as ready for review July 27, 2026 20:21
@TheTom
TheTom merged commit ad16ac3 into TheTom:feature/turboquant-kv-cache Jul 30, 2026
1 check passed
@TheTom

TheTom commented Jul 30, 2026

Copy link
Copy Markdown
Owner

Confirmed the second commit is byte-identical to upstream's revert ea21e03 (ggml-org#24715), and agree the revert beats the ever_active gate given upstream closed ggml-org#24682 in favor of reverting. Post-merge test on GB10: llama-bench --n-cpu-moe 0,8,16 -ctk turbo4 -ctv turbo3 (2 model reloads) completes with no 'CUDA error: invalid argument'. Nice writeup of the upstream history — made this an easy review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Eval bug: llama-bench CUDA "invalid argument" on multiple configs

3 participants