model-loader : reclaim mmap source pages after CPU_REPACK

[markkobo]

TL;DR

After CPU_REPACK copies a tensor out of the mmap, drop the dormant file-backed source pages with madvise(MADV_DONTNEED) — saves 13.0 GiB (−42%) RSS on Qwen3-30B-A3B, 5.4 GiB (−34%) on DeepSeek-V2-Lite, 1.4 GiB (−38%) on Llama-3.2-3B.
No flag: always on for --load-mode auto (default) / mmap on Linux; a no-op for none/dio, under mlock, and off Linux. Byte-identical output, identical perplexity, zero major page faults. Fixes #16761.

The code fits to the existing coding convention much better now. It's a bug fix so I changed the default behavior with tests covered all types of memory strategies - CPU/CUDA on 3 different models.

Let me know if more tests are needed or a flag for this bugfix is needed. I can add it back with default ON or OFF.

RSS after load	master	this PR	saving
Llama-3.2-3B Q4_0	3.75 GiB	2.31 GiB	−1.44 GiB (−38%)
DeepSeek-V2-Lite Q4_K_M	16.20 GiB	10.77 GiB	−5.44 GiB (−34%)
Qwen3-30B-A3B Q4_K_M	30.75 GiB	17.73 GiB	−13.0 GiB (−42%)

Overview

With --load-mode auto (default) / mmap, tensors handled by CPU_REPACK are copied out of the mmap into their own buffers at load time, but the file-backed source pages stay resident, every repacked tensor is effectively stored twice (see #16761). This PR drops the dormant source pages with madvise(MADV_DONTNEED) once loading finishes. The VMA stays valid and re-faults identical bytes from the page cache if needed, so it is safe by construction (unlike munmap, which would break later readers such as --check-tensors).

Default behavior change: no flag — always on for mmap loads without mlock (Linux), a no-op for --load-mode none/dio, under mlock, and on other platforms. This is a bug fix for the RSS bloat in #16761; an opt-in flag would leave the bug in place for default users. (Earlier revisions had a --reclaim-mmap-source flag; dropped on the last rebase.)

Implementation notes:

• Linux-only (madvise(MADV_DONTNEED)); the code lives next to the existing unmap_fragment() and reuses its align_range() to round ranges inward to whole pages, so pages shared with read-in-place neighbors are kept.
• Tensors still read in place from the mmap are tracked per weight and skipped — this covers tied/duplicated weights such as token_embd also used as output (see the tied-embedding test below).
• Runs after the validation futures are joined, so --check-tensors cannot race the reclaim.
• Stops at the first madvise failure with a warning — on systems where it fails once (e.g. seccomp) it would fail for every tensor.
• Logs the reclaimed amount at the end of load, e.g. load_all_data: reclaimed 13323.00 MiB of mmap source pages.

Tests: master vs this commit, identical output in every run. CPU = 2x EPYC 9R14, CUDA = L40S.

RSS after load	master	this commit
CPU Qwen3-30B-A3B Q4_K_M	31483 MiB	18160 MiB
CPU DeepSeek-V2-Lite Q4_K_M	16592 MiB	11024 MiB
CPU Llama-3.2-3B Q4_0	3838 MiB	2368 MiB
CUDA Qwen3 -ngl 99 -ot exps=CPU	17861 MiB	17322 MiB
CUDA Llama-3.2-3B -ngl 14	2082 MiB	1772 MiB
CUDA full offload -ngl 99	same	same

i.e. −13.0 GiB (−42%) on Qwen3-30B-A3B, −5.4 GiB (−34%) on DeepSeek-V2-Lite, −1.4 GiB (−38%) on Llama-3.2-3B; the saving tracks the CPU_REPACK buffer size and vanishes when nothing is repacked (full GPU offload).

Unchanged: perplexity (Qwen3, 9.0652 on wikitext-2), --load-mode none/dio/mmap+mlock, pp512/tg128 speed (CPU and CUDA), peak RSS during load (the drop happens at the end of load). No major page faults observed.

Tied embeddings — token_embd RSS after load -> after a 2000-token prompt (an earlier revision dropped these pages and they faulted straight back in; this revision keeps them while a tied model still reads them):

Llama-3.2-3B	earlier revision	this commit
output on GPU (-ngl 99, RPC device)	0 -> 71 MiB	308 -> 308 MiB
output in CPU_REPACK (Q4_0 embd)	2 -> 152 MiB	212 -> 212 MiB

Rebased after #28334 (load flags consolidated into --load-mode) and #27837 (TENSOR_READ_LAZY).

Fixes #16761

Requirements

• I have read and agree with the contributing guidelines: YES
• AI usage disclosure: YES — AI assisted with code tracing, benchmark runs, and drafting. I reviewed and edited all of the code and take full responsibility for it per AGENTS.md and CONTRIBUTING.md.

[ggml-gh-bot]

Hi @markkobo, thanks for your contribution!

Per our contribution guidelines, the automated PR checker found the following issue(s) that need your attention:

• AI-generated content: This project does not accept PRs, descriptions or commit messages that are fully or predominantly AI-generated. If you have used AI to assist you in writing code, please make sure to disclose that explicitly.

---

Please note that maintainers reserve the right to make final decisions on PRs. If you believe there is a mistake, please comment below.

[markkobo]

Hi @markkobo, thanks for your contribution!

Per our contribution guidelines, the automated PR checker found the following issue(s) that need your attention:

• AI-generated content: This project does not accept PRs, descriptions or commit messages that are fully or predominantly AI-generated. If you have used AI to assist you in writing code, please make sure to disclose that explicitly.

Please note that maintainers reserve the right to make final decisions on PRs. If you believe there is a mistake, please comment below.

AI usage disclosure:
YES, AI helped trace the code, generate the flag plumbing from the existing use_mmap pattern, run the benchmarks, draft docs.

[rhjdvsgsgks]

im curious about why use madvise(MADV_DONTNEED) rather than munmap

[markkobo]

im curious about why use madvise(MADV_DONTNEED) rather than munmap

For safety,
madvise(MADV_DONTNEED) only discards the cache but keep the virtual mapping alive, so any later access can still page-fault to read the data from the file. And still giving us the same benefit of memory saving.

munmap also destroy the mapping so any later access of the data (like the --check-tensors validation) will instantly cause an access bug(Undefined Behavior).

Also the range of the madvise is rounded within the page boundaries so that the pages shared with neighborhood tensors can be safely accessed(won't be evicted). It's safe with madvise but risky to implement correctly with munmap.

[pwilkin]

/bot review

[ggml-gh-bot]

Automated code review

Review: PR #24156 - reclaim mmap source pages after load

I checked the diff (src/llama-mmap.{cpp,h}, src/llama-model-loader.{cpp,h}) and traced the surrounding load paths (load_all_data, init_mappings, unmap_weight, load_data_range, llama-quant, tied-weights handling). The core mechanism is sound: madvise(MADV_DONTNEED) on a clean file-backed MAP_SHARED mapping keeps the VMA valid and re-faults identical bytes, the inward page rounding via the existing align_range protects pages shared with in-place neighbors, and the read_from_mmap guard correctly protects tied/duplicated weights that are copied in one context and read in place in another (same llama_tensor_weight*). Validation futures are joined before the discard loop, so --check-tensors cannot race with the reclaim. The lmlocks == nullptr guard correctly covers --load-mode mlock (load_all_data receives the same mlock_mmaps vector used by init_mappings).

Blocking

(point 1) The advertised --reclaim-mmap-source flag does not exist anywhere in this diff or the tree (grep -rn reclaim common/ is empty). The title says "add --reclaim-mmap-source", the description says "Default-off flag", and every validation command in the description passes --reclaim-mmap-source - which would be rejected as an unknown option at this commit. What the code actually does is reclaim unconditionally for every mmap load without mlock, i.e. default ON for all Linux users (src/llama-model-loader.cpp:1670, guard is only use_mmap && lmlocks == nullptr). This needs to be reconciled before review can proceed: either restore the flag plumbing (default off, per the description, with the arg documented like --load-mode), or drop all flag references from the title/description and get explicit maintainer sign-off for changing default RSS behavior for everyone. As it stands, the PR's own test plan cannot be executed.

Will slow the review

(point 2) On non-Linux POSIX (macOS/BSD) discard_fragment is a silent no-op (src/llama-mmap.cpp:579-593), so the loader still accumulates the full copied_from_mmap vector and read_from_mmap set per tensor for zero effect. Minor, but consider a compile-time capability constant next to llama_mmap::SUPPORTED (e.g. CAN_DISCARD) so the bookkeeping is skipped where it cannot work, and so future readers can see at a glance which platforms this affects.

(point 3) The feature is completely invisible at runtime - no log line reports how much was reclaimed. Since the PR's whole value proposition is an RSS number users cannot easily reproduce, an INFO line after the discard loop (e.g. total bytes dropped, matching the existing load_tensors: style) would let anyone verify it fired without running /usr/bin/time -v or smaps. Strongly recommended if the behavior ends up default ON.

(point 4) src/llama-mmap.h:61 documents "returns false on failure", but the mmap-unsupported impl (src/llama-mmap.cpp:694-700) throws instead, unlike the two no-op impls. It is unreachable in practice (guarded by use_mmap), but returning false like its siblings would keep the contract uniform; the throw also makes the "no-op elsewhere" claim in the description slightly inaccurate.

Nits

(point 5) break on the first madvise failure (src/llama-model-loader.cpp:1797) means one transient ENOMEM on a single tensor silently skips reclaim for every remaining tensor. The seccomp rationale in the comment is fair, but continue would reclaim the rest while still emitting at most... well, one warning per tensor. Either is defensible; if you keep break, the comment is fine as is.

(point 6) The header comment in llama-model-loader.h:143 is accurate and the new comments follow the project style (concise, explain why, ASCII-only, no fixed wrapping) - no changes needed there; alignment of the two new members matches clang-format's AlignConsecutiveDeclarations.

The design itself is the right shape: per-tensor tracking is genuinely needed here because copied ranges sit interleaved inside the live read-in-place span, so the existing end-of-load unmap_fragment of the outer ranges cannot do this job. Reusing align_range rather than duplicating rounding logic is good.

This review was generated automatically by pi coding agent using zai-org/GLM-5.3. It may contain mistakes. Maintainers make the final call.

[markkobo]

Automated code review

Review: PR #24156 - reclaim mmap source pages after load

I checked the diff (src/llama-mmap.{cpp,h}, src/llama-model-loader.{cpp,h}) and traced the surrounding load paths (load_all_data, init_mappings, unmap_weight, load_data_range, llama-quant, tied-weights handling). The core mechanism is sound: madvise(MADV_DONTNEED) on a clean file-backed MAP_SHARED mapping keeps the VMA valid and re-faults identical bytes, the inward page rounding via the existing align_range protects pages shared with in-place neighbors, and the read_from_mmap guard correctly protects tied/duplicated weights that are copied in one context and read in place in another (same llama_tensor_weight*). Validation futures are joined before the discard loop, so --check-tensors cannot race with the reclaim. The lmlocks == nullptr guard correctly covers --load-mode mlock (load_all_data receives the same mlock_mmaps vector used by init_mappings).

Blocking

(point 1) The advertised --reclaim-mmap-source flag does not exist anywhere in this diff or the tree (grep -rn reclaim common/ is empty). The title says "add --reclaim-mmap-source", the description says "Default-off flag", and every validation command in the description passes --reclaim-mmap-source - which would be rejected as an unknown option at this commit. What the code actually does is reclaim unconditionally for every mmap load without mlock, i.e. default ON for all Linux users (src/llama-model-loader.cpp:1670, guard is only use_mmap && lmlocks == nullptr). This needs to be reconciled before review can proceed: either restore the flag plumbing (default off, per the description, with the arg documented like --load-mode), or drop all flag references from the title/description and get explicit maintainer sign-off for changing default RSS behavior for everyone. As it stands, the PR's own test plan cannot be executed.

Will slow the review

(point 2) On non-Linux POSIX (macOS/BSD) discard_fragment is a silent no-op (src/llama-mmap.cpp:579-593), so the loader still accumulates the full copied_from_mmap vector and read_from_mmap set per tensor for zero effect. Minor, but consider a compile-time capability constant next to llama_mmap::SUPPORTED (e.g. CAN_DISCARD) so the bookkeeping is skipped where it cannot work, and so future readers can see at a glance which platforms this affects.

(point 3) The feature is completely invisible at runtime - no log line reports how much was reclaimed. Since the PR's whole value proposition is an RSS number users cannot easily reproduce, an INFO line after the discard loop (e.g. total bytes dropped, matching the existing load_tensors: style) would let anyone verify it fired without running /usr/bin/time -v or smaps. Strongly recommended if the behavior ends up default ON.

(point 4) src/llama-mmap.h:61 documents "returns false on failure", but the mmap-unsupported impl (src/llama-mmap.cpp:694-700) throws instead, unlike the two no-op impls. It is unreachable in practice (guarded by use_mmap), but returning false like its siblings would keep the contract uniform; the throw also makes the "no-op elsewhere" claim in the description slightly inaccurate.

Nits

(point 5) break on the first madvise failure (src/llama-model-loader.cpp:1797) means one transient ENOMEM on a single tensor silently skips reclaim for every remaining tensor. The seccomp rationale in the comment is fair, but continue would reclaim the rest while still emitting at most... well, one warning per tensor. Either is defensible; if you keep break, the comment is fine as is.

(point 6) The header comment in llama-model-loader.h:143 is accurate and the new comments follow the project style (concise, explain why, ASCII-only, no fixed wrapping) - no changes needed there; alignment of the two new members matches clang-format's AlignConsecutiveDeclarations.

The design itself is the right shape: per-tensor tracking is genuinely needed here because copied ranges sit interleaved inside the live read-in-place span, so the existing end-of-load unmap_fragment of the outer ranges cannot do this job. Reusing align_range rather than duplicating rounding logic is good.

This review was generated automatically by pi coding agent using zai-org/GLM-5.3. It may contain mistakes. Maintainers make the final call.

addressed points:

• point 1: updated the PR desc/title to reflect the change.
• point 2: this is only a few pointers per tensor, and live only during loading time. I prefer to keep the #if out of the loader for consistent current coding style( no #if/else in loader ).
• point 3: add a log line that fits the coding convention.
• point 4: use throw is actually better matched to the coding convention - all the not-expected-logics use throw, same as existing unmap_fragment().
• point 5: kept the break;

Also re-ran all the tests. the results(using Claude Code):

Summary

Output identical to master in every run.

Where	RSS after load, master -> PR
CPU, Qwen3-30B-A3B Q4_K_M	31483 -> 18160 MiB (-42%), same as --load-mode none (18162)
CPU, DeepSeek-V2-Lite Q4_K_M	16592 -> 11024 MiB (-34%)
CPU, Llama-3.2-3B Q4_0	3838 -> 2368 MiB (-38%)
CPU, Qwen3 offloaded to an RPC device	-ngl 24: 16371 -> 9429 MiB; -ngl 99 -ot exps=CPU: 30277 -> 16994 MiB
CUDA (L40S)	Qwen3 -ngl 99 -ot exps=CPU: 17861 -> 17322 MiB; Llama-3.2-3B -ngl 14: 2082 -> 1772 MiB; Qwen3 full/partial offload: no change
Tied embeddings	token_embd stays resident; the previous commit dropped it (0 -> 71 MiB with the output offloaded, 2 -> 152 MiB with it repacked)

• Correct: same md5 and perplexity (9.0652), --check-tensors clean, --load-mode none/dio/mmap+mlock unchanged, model-file RSS flat during generation, no extra major faults.
• Pages leave the process, not the page cache: Mapped -13 GiB, Cached flat.
• No measurable cost: pp512/tg128 equal (CPU NUMA-pinned and CUDA), load time 16.7 s for both, 290 madvise calls taking 34 ms.
• Peak RSS during load is unchanged; the drop happens at the end of load.
• The logged size matches the measured saving (see final commit).
• Final commit: editorconfig-checker v3.0.3 (the CI job) clean, git diff --check clean, no build warnings (CPU, CPU + RPC).