feat(starry-mm): file-backed mmap readahead (batched page-fault fill)#1217
Conversation
There was a problem hiding this comment.
审查总结
本 PR 为 StarryOS 的 file-backed mmap 添加了 Linux 风格的 readahead(批量缺页填充),将原本每个 4 KiB 页一次 read_at 的逐页 I/O 改为在 handle_page_fault 中向前填充一个 32 页窗口,由 CowBackend::populate 将连续未映射的 file-backed 页合并为一次批量 read_at。改动范围集中在 mm/aspace 通用模块(无架构相关路径),并附带回归测例 syscall-test-mmap-readahead。
验证结果
| 检查项 | 结果 |
|---|---|
cargo fmt --check |
失败 — cow.rs:192-193 的 unsafe 块换行格式不符合 rustfmt 要求 |
cargo clippy --manifest-path os/StarryOS/kernel/Cargo.toml --all-features -- -D warnings |
通过 |
| GitHub Actions CI — "Check formatting / run_host" | 失败(与本地 cargo fmt 结果一致) |
| GitHub Actions CI — "Detect changed paths" | 通过 |
| GitHub Actions CI — 其余 StarryOS/QEMU 测试 job | 全部 skipped(fork PR 未触发) |
阻塞问题
cargo fmt --check失败(os/StarryOS/kernel/src/mm/aspace/backend/cow.rs第 192-193 行):请运行// 当前代码(不符合 rustfmt): let dst = unsafe { slice::from_raw_parts_mut(phys_to_virt(frame).as_mut_ptr(), ps) }; // rustfmt 期望: let dst = unsafe { slice::from_raw_parts_mut(phys_to_virt(frame).as_mut_ptr(), ps) };
cargo fmt修复后重新推送。CI 的 "Check formatting / run_host" job 同样报告了此问题。
非阻塞观察
- 测试用例
syscall-test-mmap-readahead的设计很好:使用绝对偏移哈希模式验证逐字节正确性,覆盖了「大于一个窗口的映射」「末页越过 EOF 的 demand-zero」「中部缺页」三个关键场景。 - 新增的
test_framework.h沿用了项目中多个测试用例各自拷贝一份的现有模式,不是新问题。 alloc_file_run中alloc_new_frame(false)+ 从零初始化buf拷贝的方式,正确地保证了末页越过 EOF 部分的 demand-zero 语义。file.read_at(&mut &mut buf[..max_read], ...)的双重&mut与alloc_new_at中的现有写法一致,clippy 未报告问题。- 未引入
[patch.crates-io]覆盖。
重复/重叠分析
- 在
origin/dev上搜索readahead、alloc_file_run、READAHEAD,确认 dev 分支的mm/aspace中不存在 readahead 相关代码,本 PR 是全新功能。 - 未发现其他触及
mm/aspace/backend/cow.rs或mm/aspace/mod.rs的开放 PR 与本 PR 冲突。 - 本 PR 的 readahead 与 #1007(page reclaim for file-backed memory pressure)不重叠:前者是缺页时的批量填充,后者是内存压力下的页面回收,属于互补功能。
审查决定
REQUEST_CHANGES:请修复 cargo fmt 问题后重新推送,其余代码质量良好。
Powered by deepseek-v4-pro
814c411 to
39f4c67
Compare
|
已采纳 |
There was a problem hiding this comment.
审查总结
本 PR 为 StarryOS 的 file-backed mmap 添加了 Linux 风格的 readahead(批量缺页填充),将原本每个 4 KiB 页一次 read_at 的逐页 I/O 改为在 handle_page_fault 中向前填充一个 32 页窗口,由 CowBackend::populate / alloc_file_run 将连续未映射的 file-backed 页合并为一次批量 read_at。改动范围集中在 mm/aspace 通用模块,并附带回归测例 syscall-test-mmap-readahead。
验证结果
| 检查项 | 结果 |
|---|---|
cargo fmt --check |
通过(上一轮审查的格式问题已修复) |
cargo clippy --manifest-path os/StarryOS/kernel/Cargo.toml --all-features -- -D warnings |
通过 |
| 本地编译(starry-kernel) | 通过 |
[patch.crates-io] 检查 |
未发现 |
| GitHub Actions CI | 全部 skipped(fork PR 标准行为,非本 PR 问题) |
上一轮审查回顾
上一轮审查(mai-team-app[bot])因 cargo fmt 格式问题请求修改。作者已修复(let dst = unsafe { ... } 合并为单行),cargo fmt --check 现通过。无其他未解决的审查线程。
代码质量评估
alloc_file_run正确处理了:非页对齐映射头(回退逐页路径)、EOF 越界(零填充缓冲区末尾)、空文件 / 零长度读取、run 最小长度为 1(不会对空切片 panic)。populate的批量检测逻辑正确:对连续NotMapped的 file-backed 页聚合为一次 run 调用alloc_file_run,匿名页和 COW 缺页保持原逐页路径。handle_page_fault的窗口计算:仅对 4K 页使用 32 页 readahead 窗口,大页仅填充一页;窗口正确 clamp 到area.end()。unsafe代码块(slice::from_raw_parts_mut)与alloc_new_at中的现有写法一致,语义正确(新分配的物理帧已映射为有效虚拟地址)。- 测试用例
test-mmap-readahead设计良好:使用绝对偏移哈希模式验证逐字节正确性,覆盖了「大于一个窗口的映射」「末页越过 EOF 的 demand-zero」「中部缺页」三个关键场景。 - 错误传播路径匹配现有模式,无新引入的 panic 风险、死锁风险或资源泄漏。
重复/重叠分析
origin/dev的mm/aspace中不存在 readahead 相关代码,本 PR 是全新功能。- 未发现其他触及
mm/aspace/backend/cow.rs或mm/aspace/mod.rs的开放 PR(PR #1203 是网络栈重构,不重叠)。
审查决定
APPROVE — 上一轮的阻塞问题已解决,代码质量良好,测试覆盖充分,无安全或设计问题。
Powered by deepseek-v4-pro
ZR233
left a comment
There was a problem hiding this comment.
已复查当前 head 39f4c67b1e8a4aa4e312566e1c1ef97bce00f02e。
合并要求逐项检查:
- 变更范围集中在 Starry
mm/aspace的 file-backed mmap 缺页填充,以及对应qemu-smp1/system/syscall-test-mmap-readahead回归用例。 CowBackend::populate只在 file-backed 且连续NotMapped页时走批量填充;匿名页、已映射页上的 COW fault 仍走原路径。- readahead 窗口只对 4K 页启用,并 clamp 到当前 area 结束;测试覆盖了大于窗口的映射、EOF 后 demand-zero、从映射中部先触发缺页等关键偏移场景。
- 已检查 open PR 重叠情况,未发现另一个同向 mmap readahead PR;搜索到的 #1173/#1017/#1062 虽触及
cow.rs或mm/aspace,但主题分别是 RSS 统计、apk-cmake 支持、ptrace 支持,不是重复实现。 - 旧的格式化 review thread 已过时,当前
cargo fmt --check通过,已 resolve。
本地验证:
cargo fmt --check
cargo xtask clippy --package starry-kernel
cargo xtask starry test qemu --arch x86_64 -c qemu-smp1/system/syscall-test-mmap-readahead
其中 starry-kernel clippy 14/14 feature checks 全部通过;新增 QEMU 用例 9/9 pass,STARRY_GROUPED_TESTS_PASSED。
CI 中 Test axvisor self-hosted board roc-rk3568-pc-linux / run_host 的失败发生在 U-Boot 控制命令 setenv autoload yes 超时阶段,未进入本 PR 的 Starry mmap 代码路径,已单独跟踪为 #1227。其余被取消的任务是该失败后的级联取消。
结论:当前变更没有发现阻塞问题,可以合入。
Demand-paging a file-backed mapping faulted ONE 4 KiB page per `read_at`, each a full axfs-ng→virtio-blk path traversal. Loading a large `.so` (e.g. a 186 MB libLLVM ≈ 46K pages) thus issued ~46K separate reads. This adds Linux-style readahead: - `aspace::handle_page_fault` now populates a 32-page window forward from the fault (clamped to the area; base 4K pages only), instead of a single page. - `CowBackend::populate` batches runs of consecutive not-mapped FILE-backed pages into ONE `read_at(N*page_size)` via new `alloc_file_run` (zero-init buffer → partial EOF pages stay demand-zero; non-page-aligned head falls back per-page; anonymous and COW-fault pages keep the existing per-page path). Regression (test-suit/starryos/qemu-smp1/system/syscall-test-mmap-readahead): mmap a file LARGER than the 32-page window with a partial last page; verify every in-file byte equals an offset-hash pattern (any wrong per-page run offset → mismatch), the bytes past EOF in the last page are demand-zero (readahead clamp), and a mid-mapping-first fault (window starting mid-file) still yields correct bytes everywhere. Verified (single-core qemu10): x86_64 / aarch64 / riscv64 system group all `STARRY_GROUPED_TESTS_PASSED` with test-mmap-readahead 9/9 and no regression; loongarch64 — kernel builds and test-mmap-readahead 9/9 (run in isolation, since the full loongarch system group currently hangs in an UNRELATED pre-existing F_SETLKW blocking-lock test, not this change). The readahead code is in generic mm/aspace (no arch-specific path). Conflict-free on current dev (no readahead in mm/aspace; no open PR touches it). Note: this does NOT by itself unblock pocl/opencl — its dlopen of the 186 MB libpocl→libLLVM closure is dominated by libLLVM's C++ static-ctor execution under qemu-TCG, not the per-page I/O this addresses. It remains a correct, general improvement for all large file-backed loads (java/node startup, big `.so`, etc). > 把困困投入生产后更名 `智慧集群`(Smart Cluster) Signed-off-by: Smart Cluster <haxxorbunny@qq.com> Signed-off-by: 林晨 (Leo Cheng) <chengkelfan@qq.com>
39f4c67 to
c3677bf
Compare
…#1217) Demand-paging a file-backed mapping faulted ONE 4 KiB page per `read_at`, each a full axfs-ng→virtio-blk path traversal. Loading a large `.so` (e.g. a 186 MB libLLVM ≈ 46K pages) thus issued ~46K separate reads. This adds Linux-style readahead: - `aspace::handle_page_fault` now populates a 32-page window forward from the fault (clamped to the area; base 4K pages only), instead of a single page. - `CowBackend::populate` batches runs of consecutive not-mapped FILE-backed pages into ONE `read_at(N*page_size)` via new `alloc_file_run` (zero-init buffer → partial EOF pages stay demand-zero; non-page-aligned head falls back per-page; anonymous and COW-fault pages keep the existing per-page path). Regression (test-suit/starryos/qemu-smp1/system/syscall-test-mmap-readahead): mmap a file LARGER than the 32-page window with a partial last page; verify every in-file byte equals an offset-hash pattern (any wrong per-page run offset → mismatch), the bytes past EOF in the last page are demand-zero (readahead clamp), and a mid-mapping-first fault (window starting mid-file) still yields correct bytes everywhere. Verified (single-core qemu10): x86_64 / aarch64 / riscv64 system group all `STARRY_GROUPED_TESTS_PASSED` with test-mmap-readahead 9/9 and no regression; loongarch64 — kernel builds and test-mmap-readahead 9/9 (run in isolation, since the full loongarch system group currently hangs in an UNRELATED pre-existing F_SETLKW blocking-lock test, not this change). The readahead code is in generic mm/aspace (no arch-specific path). Conflict-free on current dev (no readahead in mm/aspace; no open PR touches it). Note: this does NOT by itself unblock pocl/opencl — its dlopen of the 186 MB libpocl→libLLVM closure is dominated by libLLVM's C++ static-ctor execution under qemu-TCG, not the per-page I/O this addresses. It remains a correct, general improvement for all large file-backed loads (java/node startup, big `.so`, etc). > 把困困投入生产后更名 `智慧集群`(Smart Cluster) Signed-off-by: Smart Cluster <haxxorbunny@qq.com> Signed-off-by: 林晨 (Leo Cheng) <chengkelfan@qq.com> Co-authored-by: Smart Cluster <haxxorbunny@qq.com>
背景
file-backed mmap 的按需缺页此前每个 4 KiB 页一次
read_at,每次都是一整条 axfs-ng→virtio-blk 路径。加载一个大.so(例如 186 MB 的 libLLVM ≈ 46K 页)因此要发 ~46K 次独立读,慢到分钟级。改动
Linux 风格 readahead:
aspace::handle_page_fault从缺页起向前填一个 32 页窗口(clamp 到该 area;仅基础 4K 页),而非单页。CowBackend::populate把连续未映射的 file-backed 页成段(run)合并为一次read_at(N*page_size)(新增alloc_file_run):零初始化缓冲 → 越过 EOF 的部分末页保持 demand-zero;非页对齐的映射头回退逐页路径;匿名页与 COW 缺页保持原逐页路径不变。回归测例
test-suit/starryos/qemu-smp1/system/syscall-test-mmap-readahead:mmap 一个大于 32 页窗口且带部分末页的文件,验证 ① 每个文件内字节等于"绝对偏移哈希"模式(任一页拿错 run 偏移即失配)② 末页越过 EOF 的字节为 demand-zero(readahead 的 EOF clamp)③ 先在映射中部缺页(窗口从中部起)各字节仍正确。验证
单核 qemu10:x86_64 / aarch64 / riscv64 的 system 组均
STARRY_GROUPED_TESTS_PASSED,test-mmap-readahead9/9 且全组无回归;loongarch64 内核可编译且test-mmap-readahead9/9(单独隔离运行——当前 loongarch system 组卡在一个与本改动无关的、既有的 F_SETLKW 阻塞锁测试上,非本 PR 引入)。readahead 代码在通用mm/aspace(无架构相关路径)。conflict-free(当前 dev 的 mm/aspace 无 readahead,无其它 open PR 触及)。备注(据实)
这不单独解开 pocl/opencl:其 dlopen 186 MB 的 libpocl→libLLVM 闭包瓶颈在 libLLVM 的 C++ static-ctor 在 qemu-TCG 下的执行,而非本 PR 处理的逐页 I/O。但它对所有大 file-backed 加载(java/node 启动、大
.so等)是通用且正确的改进。