fix(starry): align mount and umount2 semantics with Linux - #876
Conversation
There was a problem hiding this comment.
PR #876 Review: fix(starry): align mount and umount2 semantics with Linux
本 PR 对 StarryOS 的 mount(2) / umount2(2) 进行了全面的 Linux 兼容性补齐,涵盖了 propagation flags、bind mount、move mount、remount、readonly 写保护、umount2 高级 flags 等核心语义,并附带了系统性用户态测试和详细的技术文档。
总体评价
这是一个质量很高的 PR,实现思路清晰,测试覆盖系统全面,文档详实。主要的代码变更集中在两个层面:
- VFS 层 (
axfs-ng-vfs/mount.rs):扩展Mountpoint状态模型,增加 propagation/readonly/expire 属性,新增 bind/move/detach 等操作 - Syscall 层 (
StarryOS/syscall/fs/mount.rs):按 Linux 语义分派不同 flags 路径
主要优点
- 分派结构正确:
sys_mount按照 propagation → remount → move → bind → normal 的优先级分派,与 Linuxdo_mount()一致 - resolve_mountpoint 改造关键:从检查
DirEntry.mountpoint()改为检查Mountpoint.childrenmap,正确区分了普通 bind 和 recursive bind 的可见性 - propagation 机制实现扎实:shared peer group 的 join/leave、slave 的单向传播、unbindable 的 prune 都有正确的锁保护和数据结构操作
- MNT_EXPIRE 两阶段语义:利用
AtomicBool::swap优雅实现 - 测试覆盖面广:从 propagation flags 合法性到 bind/move/remount/readonly/umount2 各 flags 的正反向测试都很完整
发现的问题
详见 inline comments。主要关注点:
- readonly mount 上
open(O_WRONLY)(不带 O_CREAT)已有文件缺少写拒绝 - 文档中包含本地开发路径
bind()recursive 分支中的锁持有范围可以优化
总体建议:批准合并,readonly 写保护的小缺口可以作为后续改进项。
Powered by glm-5.1
| readonly: AtomicBool, | ||
| /// Expire mark for umount2(MNT_EXPIRE). | ||
| expired: AtomicBool, | ||
| /// Mount propagation type. |
There was a problem hiding this comment.
这里 source.mountpoint.children.lock() 在整个 for 循环期间持有,而循环体内 child.upgrade().is_none_or(|child| !child.is_unbindable()) 会调用 propagation() → self.propagation.lock()。
虽然当前锁序 children → propagation 在其他代码路径中没有反向使用,但建议在循环外先 clone 出 (key, child) 的 Vec,再在锁释放后处理。这样既能避免长锁持有,也能降低未来引入反向锁序的死锁风险:
let children_to_copy: Vec<_> = source.mountpoint.children.lock()
.iter()
.filter(|(_, child)| {
child.upgrade().is_none_or(|c| !c.is_unbindable())
})
.map(|(k, v)| (k.clone(), v.clone()))
.collect();
for (key, child) in children_to_copy {
result.children.lock().insert(key, child);
}| return Err(VfsError::ReadOnlyFilesystem); | ||
| } | ||
| self.entry | ||
| .as_dir()? |
There was a problem hiding this comment.
当前 readonly 检查只拦截了 options.create || options.create_new,但没有拦截以 O_WRONLY 或 O_RDWR 打开已有文件的情况。
Linux 在 do_open() → may_open() 中会检查 __mnt_is_readonly(),即使文件已存在、不带 O_CREAT,也不允许以写模式打开。
建议补充:
pub fn open_file(&self, name: &str, options: &OpenOptions) -> VfsResult<Location> {
if self.is_readonly() {
if options.create || options.create_new {
return Err(VfsError::ReadOnlyFilesystem);
}
// 也需要检查是否以写模式打开已有文件
// 这取决于 OpenOptions 是否暴露了 write/read 标志
}当前测试中的 write_file() 使用 O_CREAT | O_WRONLY | O_TRUNC,所以 create=true 被捕获了。但直接 open(path, O_WRONLY) 不带 O_CREAT 的场景没有被覆盖。
| if !target.is_root_of_mount() { | ||
| return Err(AxError::InvalidInput); | ||
| } | ||
| if (flags & MS_RDONLY) != 0 { |
There was a problem hiding this comment.
这里 target 解析和 mountpoint 操作都在 FS_CONTEXT 的锁外执行(resolve(target)? 之后没有持有 FS_CONTEXT lock)。这是正确的,因为 propagation 操作只修改已有 mountpoint 的 propagation 属性,不涉及 mount tree 结构变更。
| }; | ||
|
|
||
| // Linux umount2 returns EINVAL for paths that are not mount points. | ||
| if !target.is_root_of_mount() { |
There was a problem hiding this comment.
MNT_EXPIRE 的两阶段语义实现很精炼:mark_expired() 返回旧值,第一次调用旧值=false → !false=true → 返回 EAGAIN 并标记;第二次调用旧值=true → !true=false → 继续卸载。与 Linux do_expire_mount() 行为一致。👍
| - propagation 操作: | ||
| 带 `MS_SHARED` / `MS_PRIVATE` / `MS_SLAVE` / `MS_UNBINDABLE` 时,表示“修改已有 mount 的传播属性” | ||
| - bind 挂载: | ||
| 带 `MS_BIND` 时,表示“把已有路径对应的 mount tree 映射到另一个位置” |
There was a problem hiding this comment.
文档第 5.3 节和 5.1 节中包含本地开发路径,例如:
os/StarryOS/kernel/src/syscall/fs/mount.rs→/Users/ken/26s-rust/tgoskits/os/StarryOS/kernel/src/syscall/fs/mount.rscomponents/axfs-ng-vfs/src/mount.rs→/Users/ken/26s-rust/tgoskits/components/axfs-ng-vfs/src/mount.rs
建议改为仓库相对路径,方便其他开发者使用。
d0f9230 to
e62fe5d
Compare
There was a problem hiding this comment.
PR #876 Review:fix(starry): align mount and umount2 semantics with Linux
本 PR 对 StarryOS 的 mount(2) / umount2(2) 进行了全面的 Linux 兼容性补齐,涵盖 propagation flags、bind mount、move mount、remount、readonly 写保护、umount2 高级 flags 等核心语义,并附带了系统性用户态测试和详细的技术文档。
变更范围
- VFS 层 (
components/axfs-ng-vfs/src/mount.rs,+370/-10):扩展Mountpoint状态模型,新增readonly/expired/propagation/peers/slaves/masters属性,新增bind/move_to/detach/propagate_new_child/join_shared_group等操作,改造resolve_mountpoint从检查DirEntry.mountpoint()改为检查Mountpoint.childrenmap - Syscall 层 (
os/StarryOS/kernel/src/syscall/fs/mount.rs,~+200):按 Linuxdo_mount()优先级 propagation → remount → move → bind → normal 分派 - 高层文件操作 (
os/arceos/modules/axfs-ng/src/highlevel/file.rs,+11):在OpenOptions::open和File::access中拦截 readonly mount 上的写操作 - 测试 (
test-suit/starryos/normal/qemu-smp1/util-linux/c/src/main.c,~+1600):系统性 mount/umount2 兼容性用例 - 文档 (
os/StarryOS/docs/mount-umount2-linux-compat.md,+672):Linux 语义矩阵与实现思路
本地验证
cargo fmt --check -p axfs-ng-vfs:通过cargo check -p axfs-ng-vfs:通过cargo xtask clippy --package axfs-ng-vfs:全部检查通过- QEMU 用户态测试:PR 描述中声称通过
cargo xtask starry test qemu --target aarch64-unknown-none-softfloat -c util-linux,当前环境无法运行 QEMU,依赖作者验证
CI 状态
所有 CI check runs 状态为 skipped,无 PR 相关的 CI 失败。
重复/重叠分析
- 在 dev 分支上搜索
mount.rs、bind_mount、MS_BIND、MS_MOVE等,确认 dev 分支没有已有的 bind/move/remount/readonly 实现 - 搜索开放 PR 列表,未发现与 mount/umount2 语义补齐重叠的 PR(PR #707 涉及 pidfd,PR #881 涉及 self-compilation,均不重叠)
- 本 PR 无重复风险
之前的 review 评论
上一轮 review(commit d0f92305,bot APPROVED)提出了 5 条 inline comment:
bind()递归分支中的锁持有范围 — 建议优化,非阻塞open_filereadonly 检查缺少 O_WRONLY(不带 O_CREAT)— 通过file.rs的OpenOptions层已覆盖,主 syscall 路径已有完整保护- propagation 操作在 FS_CONTEXT 锁外执行 — 正确
- MNT_EXPIRE 两阶段语义 — 正确
- 文档包含本地开发路径 — 应改为仓库相对路径
实现质量评价
- 分派结构正确:
sys_mount按 propagation → remount → move → bind → normal 的优先级分派,与 Linuxdo_mount()一致 resolve_mountpoint改造关键:从检查DirEntry.mountpoint()改为检查Mountpoint.childrenmap,正确区分了普通 bind 和 recursive bind 的可见性- Propagation 机制扎实:shared peer group 的 join/leave、slave 的单向传播、unbindable 的 prune 都有正确的 Weak 引用管理和锁保护
- MNT_EXPIRE 两阶段语义:利用
AtomicBool::swap优雅实现 - Readonly 写保护分层合理:
file.rs在OpenOptions::open拦截 WRITE/APPEND/truncate,在File::access拦截已打开 fd 的后续写入;mount.rs拦截 create/unlink/rename/mkdir/chmod - 测试覆盖面广:从 propagation flags 合法性到 bind/move/remount/readonly/umount2 各 flags 的正反向测试
未实现/后续工作(PR 文档已明确声明)
- 真实 shared subtree propagation 的完整语义(当前只实现了基本的 peer/slave 关系维护和子挂载传播)
MS_NODEV/MS_NOSUID/MS_NOEXEC等其他 mount flagsMNT_FORCE的真实强制卸载语义
总结
这是一个高质量的 PR,实现思路清晰,与 Linux mount(2)/umount2(2) 语义对齐程度高,测试覆盖系统和全面。建议批准合并。
Powered by glm-5.1
| - propagation 操作: | ||
| 带 `MS_SHARED` / `MS_PRIVATE` / `MS_SLAVE` / `MS_UNBINDABLE` 时,表示“修改已有 mount 的传播属性” | ||
| - bind 挂载: | ||
| 带 `MS_BIND` 时,表示“把已有路径对应的 mount tree 映射到另一个位置” |
There was a problem hiding this comment.
文档第 5.1 节和 5.3 节中包含本地开发路径,例如 /Users/ken/26s-rust/tgoskits/os/StarryOS/...。建议改为仓库相对路径(如 os/StarryOS/kernel/src/syscall/fs/mount.rs),方便其他开发者阅读。
| Some(location_in_parent), | ||
| source.mountpoint.device(), | ||
| ); | ||
| result |
There was a problem hiding this comment.
递归 bind 分支中,source.mountpoint.children.lock() 在整个 for 循环期间持有,而循环体内的 is_unbindable() 会调用 self.propagation.lock()。虽然当前锁序 children → propagation 在其他路径中没有反向使用,但建议在循环外先 clone 出 (key, child) 的 Vec 再处理,降低长锁持有和未来引入反向锁序的风险。这是一个性能优化建议,不阻塞合并。
| target.mountpoint().set_readonly(true); | ||
| } | ||
| return Ok(0); | ||
| } |
There was a problem hiding this comment.
这里 propagation-only 分支的 resolve(target) 和 mountpoint 操作都在 FS_CONTEXT 锁外执行是正确的——propagation 操作只修改已有 mountpoint 的 propagation 属性,不涉及 mount tree 结构变更。但注意:set_shared()/set_slave() 等方法内部会获取 self.propagation.lock() 和 self.peers.lock(),如果将来有其他路径在持有 FS_CONTEXT 锁的同时调用这些方法,可能会产生锁序问题。当前是安全的。
e62fe5d to
2fb2283
Compare
There was a problem hiding this comment.
PR #876 Review:fix(starry): align mount and umount2 semantics with Linux
本 PR 对 StarryOS 的 mount(2) / umount2(2) 进行了全面的 Linux 兼容性补齐,涵盖 propagation flags、bind mount、move mount、remount、readonly 写保护、umount2 高级 flags 等核心语义,并附带了系统性用户态测试和详细的技术文档。
变更范围
- VFS 层 (
components/axfs-ng-vfs/src/mount.rs,+370/-10):扩展Mountpoint状态模型,新增readonly/expired/propagation/peers/slaves/masters属性,新增bind/move_to/detach/propagate_new_child/join_shared_group等操作,改造resolve_mountpoint从检查DirEntry.mountpoint()改为检查Mountpoint.childrenmap - Syscall 层 (
os/StarryOS/kernel/src/syscall/fs/mount.rs,+200/-5):按 Linuxdo_mount()优先级 propagation → remount → move → bind → normal 分派 - 高层文件操作 (
os/arceos/modules/axfs-ng/src/highlevel/file.rs,+11):在OpenOptions::_open和File::access中拦截 readonly mount 上的写操作 - 测试 (
test-suit/starryos/normal/qemu-smp1/util-linux/c/src/main.c,+1600):系统性 mount/umount2 兼容性用例 - 文档 (
os/StarryOS/docs/mount-umount2-linux-compat.md,+672):Linux 语义矩阵与实现思路
实现质量评价
- 分派结构正确:
sys_mount按 propagation → remount → move → bind → normal 的优先级分派,与 Linuxdo_mount()一致 resolve_mountpoint改造关键:从检查DirEntry.mountpoint()改为检查Mountpoint.childrenmap,正确区分了普通 bind 和 recursive bind 的可见性- Propagation 机制扎实:shared peer group 的 join/leave、slave 的单向传播、unbindable 的 prune 都有正确的 Weak 引用管理和锁保护
- MNT_EXPIRE 两阶段语义:利用
AtomicBool::swap优雅实现 - Readonly 写保护分层完整:
mount.rs拦截 create/unlink/rename/mkdir/chmod,file.rs在OpenOptions::_open拦截 WRITE/APPEND/truncate,在File::access拦截已打开 fd 的后续写入——经确认,open(path, O_WRONLY)不带 O_CREAT 的场景由file.rs的_open完整覆盖 - 测试覆盖面广:从 propagation flags 合法性到 bind/move/remount/readonly/umount2 各 flags 的正反向测试
本地验证
cargo fmt --check -p axfs-ng-vfs:通过cargo xtask clippy --package axfs-ng-vfs:全部检查通过- QEMU 用户态测试:PR 描述声称通过
cargo xtask starry test qemu --target aarch64-unknown-none-softfloat -c util-linux,当前环境无法运行 QEMU,依赖作者验证
CI 状态
所有 CI check runs 状态为 skipped,无 PR 相关的 CI 失败。
重复/重叠分析
- 在 dev 分支上搜索
mount.rs、bind_mount、MS_BIND、MS_MOVE等,确认 dev 分支没有已有的 bind/move/remount/readonly 实现 - 搜索开放 PR 列表,未发现与 mount/umount2 语义补齐重叠的 PR
- 本 PR 无重复风险
之前的 review 评论
前两轮 review 提出的关注点:
bind()递归分支中的锁持有范围 — 建议优化,非阻塞open_filereadonly 检查缺少 O_WRONLY(不带 O_CREAT)— 通过file.rs的OpenOptions层已覆盖,保护完整- propagation 操作在 FS_CONTEXT 锁外执行 — 正确
- MNT_EXPIRE 两阶段语义 — 正确
- 文档包含本地开发路径 — 仍需修改(见 inline comment)
未实现/后续工作(PR 文档已明确声明)
- 真实 shared subtree propagation 的完整语义
MS_NODEV/MS_NOSUID/MS_NOEXEC等其他 mount flagsMNT_FORCE的真实强制卸载语义
总结
这是一个高质量的 PR,实现思路清晰,与 Linux mount(2)/umount2(2) 语义对齐程度高,测试覆盖系统和全面。文档中包含本地路径(见 inline comment),建议后续清理。批准合并。
Powered by glm-5.1
|
|
||
| 对应代码位置: | ||
|
|
||
| - [`os/StarryOS/kernel/src/syscall/fs/mount.rs`](/Users/ken/26s-rust/tgoskits/os/StarryOS/kernel/src/syscall/fs/mount.rs) |
There was a problem hiding this comment.
文档第 5.1 节(第 278 行)、第 5.3 节(第 335 行)和第 7 节(第 639 行)中包含本地开发路径,例如 /Users/ken/26s-rust/tgoskits/os/StarryOS/kernel/src/syscall/fs/mount.rs。建议改为仓库相对路径(如 os/StarryOS/kernel/src/syscall/fs/mount.rs),方便其他开发者使用。
# Conflicts: # components/axfs-ng-vfs/src/mount.rs
There was a problem hiding this comment.
PR #876 Review: fix(starry): align mount and umount2 semantics with Linux
本 PR 在 StarryOS 中系统性地补齐了 mount(2) / umount2(2) 的 Linux 兼容语义,涵盖 propagation flags 合法性校验、bind/move/remount/readonly 挂载操作、umount2 高级 flags(MNT_EXPIRE / UMOUNT_NOFOLLOW / MNT_DETACH)以及只读挂载的写保护。附带了 ~1000 行 util-linux 用户态测试和 672 行技术文档。
变更范围
- VFS 层 (
components/axfs-ng-vfs/src/mount.rs, +378/-10):扩展Mountpoint状态模型,新增 readonly/expired/propagation/peers/slaves/masters 属性,新增 bind/move_to/detach/propagate_new_child/join_shared_group 等操作,改造resolve_mountpoint从检查DirEntry.mountpoint()改为检查Mountpoint.childrenmap - Syscall 层 (
os/StarryOS/kernel/src/syscall/fs/mount.rs, +75/-15):按 Linuxdo_mount()优先级 propagation → remount → move → bind → normal 分派 - 高层文件操作 (
os/arceos/modules/axfs-ng/src/highlevel/file.rs, +11):在OpenOptions::_open中拦截 readonly mount 上的 WRITE/APPEND/truncate,在File::access中拦截已打开 fd 的后续写入 - 测试 (
test-suit/starryos/normal/qemu-smp1/util-linux/c/src/main.c, +1038):系统性 mount/umount2 兼容性用例 - 文档 (
os/StarryOS/docs/mount-umount2-linux-compat.md, +672):Linux 语义矩阵与实现思路
前序 review 评论处理情况
本轮 PR 头部 (4fb7a78) 已处理前序 bot review 提出的所有关注点:
- ✅ bind() 递归分支锁持有范围:已改为先 snapshot children Vec,释放锁后在锁外过滤 unbindable child,最后再获取 result.children 锁插入,锁持有时间大幅缩短
- ✅ open_file readonly 检查缺少 O_WRONLY:经确认,
file.rs的OpenOptions::_open在loc.is_readonly()时拦截FileFlags::WRITE | FileFlags::APPEND及 truncate,涵盖 O_WRONLY(不带 O_CREAT)打开已有文件的场景;File::access覆盖已打开 fd 的后续写入。分层防护完整 - ✅ 文档本地路径:已全部改为仓库相对路径(
grep /Users/ken无匹配) - ✅ propagation 操作在 FS_CONTEXT 锁外执行:确认安全,propagation 操作只修改 mountpoint 属性不涉及 mount tree 结构变更
- ✅ MNT_EXPIRE 两阶段语义:实现正确,
AtomicBool::swap优雅
本地验证
cargo fmt --check -p axfs-ng-vfs → PASS
cargo xtask clippy --package axfs-ng-vfs → PASS (1/1)
cargo xtask clippy --package ax-fs-ng → PASS (7/7)
QEMU 用户态测试(PR 描述声明):cargo xtask starry test qemu --target aarch64-unknown-none-softfloat -c util-linux → 230 passed, 0 failed。当前环境无法运行 QEMU,依赖作者验证。
CI 状态
所有 CI check runs 的 conclusion 均为 skipped(因为 PR 来自 fork 仓库),仅有一个 self-hosted board 测试 (licheerv-nano-sg2002 / run_host) 返回 success。无 PR 相关的 CI 失败。
重复/重叠分析
- 在 dev 分支上搜索
bind_mount、move_mount、detach_mount、is_readonly、set_shared等新方法,确认 dev 分支没有已有实现 - 浏览当前开放 PR 列表(#926 axtask, #927 rsext4),均与 mount/umount2 语义无关
- 无重复或重叠风险
实现质量评价
- 分派结构正确:
sys_mount按 propagation → remount → move → bind → normal 优先级分派,与 Linuxdo_mount()逻辑一致 resolve_mountpoint改造:从检查共享的DirEntry.mountpoint改为检查 per-mount 的Mountpoint.childrenmap,正确区分了普通 bind(不递归)和 recursive bind 的可见性差异- Propagation 机制:shared peer group 的 join/leave、slave 的单向传播、unbindable 的 prune 都有正确的 Weak 引用管理和锁保护
- MNT_EXPIRE 两阶段语义:利用
AtomicBool::swap返回值区分第一次标记(返回 EAGAIN)和第二次真正卸载 - 只读写保护分层:VFS 层拦截 create/unlink/rename/mkdir/update_metadata,文件层拦截 WRITE/APPEND/truncate 及已打开 fd 后续写入,覆盖全面
- 测试覆盖面广:从 propagation flags 合法性校验到 bind/move/remount/readonly/umount2 各 flags 的正反向测试
未实现/后续工作(PR 文档已明确声明)
- 真实 shared subtree propagation 的完整传播语义
MS_NODEV/MS_NOSUID/MS_NOEXEC等其他 mount flagsMNT_FORCE的真实强制卸载语义
总结
这是一个高质量的 PR,实现思路清晰,与 Linux mount(2)/umount2(2) 语义对齐程度高,测试覆盖系统全面,文档详实。前序 review 评论已全部处理,无阻塞性问题。建议批准合并。
Powered by deepseek-v4-pro
问题
StarryOS 当前
mount(2)/umount2(2)与 Linux 语义存在多处差距。主要问题包括:
sys_mount仅支持基础tmpfs/ext4挂载,部分 mount flag 只做了校验,没有真实语义MS_BIND/MS_MOVE/MS_REMOUNT/MS_RDONLY等行为不完整umount2的MNT_EXPIRE/UMOUNT_NOFOLLOW/MNT_DETACH等真实行为缺失util-linux用例对这些兼容性缺口覆盖不足,缺少系统性回归保护修改内容
1. 补全 mount/umount2 核心语义
在 VFS 和 Starry 内核 syscall 层补齐了这批 Linux 兼容行为:
MS_PRIVATE/MS_SHARED/MS_SLAVE/MS_UNBINDABLEMS_BINDMS_BIND绑定子目录MS_BIND | MS_RECMS_MOVEMS_MOVE对 descendant target 返回ELOOPMS_REMOUNTMS_RDONLYMS_REMOUNT | MS_RDONLYMS_REMOUNT | MS_BIND | MS_RDONLYumount2invalid flags / invalid comboMNT_EXPIREUMOUNT_NOFOLLOWMNT_DETACH2. 补全只读挂载的写保护
补齐了只读 mount 上的写保护,确保以下操作返回
EROFS:chmodrenameunlinkmkdir3. 扩展 util-linux 用户态测试
在现有
util-linux聚合测试中系统补充了 mount/umount2 兼容性用例,覆盖:4. 增加技术文档
新增文档说明当前 Linux 兼容语义、实现思路与后续边界:
os/StarryOS/docs/mount-umount2-linux-compat.md5. 同步 dev 并处理 review 建议
本轮已将 PR 分支同步到
rcore-os/tgoskits:dev的768ac21e0,并处理合并冲突。冲突解决与 review 处理包括:
components/axfs-ng-vfs/src/mount.rs保留最新dev的children初始化方式,同时保留本 PR 的 readonly / expire / propagation 状态children,再在锁外过滤 unbindable child,降低锁持有范围OpenOptions::_open与File::access拦截,无需在Location::open_file重复处理实现思路
这次修改不是只做 flag 校验,而是把 mount tree 里的关键语义补到了可验证的程度。
具体思路包括:
axfs-ng-vfs中扩展 mountpoint 状态,维护 readonly / expire / propagation 等挂载属性mount(2)/umount2(2)语义分派不同 flags 路径util-linux用户态测试逐步暴露缺口,再针对每类缺口补实现,保证最终具备稳定回归保护验证
已完成验证:
cargo fmt cargo fmt --check cargo xtask clippy --package axfs-ng-vfs cargo xtask clippy --package ax-fs-ng cargo xtask clippy --package starry-kernel cargo xtask starry test qemu --target aarch64-unknown-none-softfloat -c util-linux其中
util-linuxStarry QEMU 用例通过:230 passed, 0 failed。