Skip to content

Conversation

@xboHodx
Copy link
Contributor

@xboHodx xboHodx commented Feb 2, 2026

但是因为another_ext4库不支持创建块设备和字符设备节点,所以暂时没办法完全实现mknod


2026/2/5

Summary

  • 在 ext4 文件系统中实现完整的 mknod 系统调用支持,包括字符设备、块设备、FIFO 等特殊文件类型
  • 重构 sys_stat 系统调用,修复设备节点元数据返回不正确的问题
  • 添加全面的用户态测试套件验证功能正确性

Changes

1. ext4 设备节点支持 (another_ext4 crate)

kernel/crates/another_ext4/src/ext4_defs/inode.rs

  • 新增 device 模块,实现 Linux 兼容的设备号编解码:
    • Old format (16-bit): major < 256 && minor < 256
    • New format (32-bit): 支持 12-bit major, 20-bit minor
  • 新增 Inode::is_device(), Inode::device(), Inode::set_device() 方法
  • FileAttr 新增 rdev 字段存储设备号
  • 包含完整的单元测试覆盖编解码正确性和 Linux 兼容性

kernel/crates/another_ext4/src/ext4/alloc.rs

  • 新增 create_device_inode() 方法:
    • 关键区别:设备节点初始化 extent tree
    • 设备号存储在 i_block[0..1](遵循 Linux ext4 标准)

kernel/crates/another_ext4/src/ext4/low_level.rs

  • 新增 mknod() 方法创建设备节点
  • getattr() 现在正确返回设备节点的 rdev

2. VFS 层集成

kernel/src/filesystem/ext4/inode.rs

  • 实现 IndexNode::mknod() trait 方法
  • 支持文件类型:
    • 字符设备 (S_IFCHR):调用 ext4 mknod 存储设备号
    • 块设备 (S_IFBLK):调用 ext4 mknod 存储设备号
    • FIFO (S_IFIFO):创建普通 inode 并关联 pipe
    • 普通文件 (S_IFREG):委托给 create()
  • 新增 special_node 字段和 special_node() 方法,支持 FIFO 的 pipe inode
  • 修复 metadata() 返回:
    • dev_id: 正确返回文件系统设备号 (st_dev)
    • raw_dev: 仅对设备节点返回 st_rdev

3. 系统调用重构

kernel/src/filesystem/vfs/syscall/sys_mknod.rs & sys_mknodat.rs

  • 正确解析 mode 参数:分离文件类型位 (S_IFMT) 和权限位
  • 遵循 POSIX 规范:
    • "Zero file type is equivalent to type S_IFREG"
    • "permissions of the created node are (mode & ~umask)"
  • 使用 from_linux_dev_t() 正确解码用户空间 dev_t

kernel/src/filesystem/vfs/syscall/sys_stat.rs

  • 重构为使用 do_newfstatat() 而非 open/fstat/close
  • 消除不必要的文件描述符分配开销

kernel/src/driver/base/device/device_number.rs

  • 新增 from_linux_dev_t() 方法:
    • 兼容 glibc makedev()/major()/minor() 编码格式
    • 支持 old format 和 new format

4. 测试

user/apps/c_unitest/test_mknod.c (923 lines)

  • 全面的测试套件,覆盖:
    • 字符设备创建和验证
    • 块设备创建和验证
    • FIFO 创建和读写测试
    • 权限和 umask 测试
    • 错误处理测试 (EEXIST, ENOENT, EINVAL 等)
    • 设备号编码测试(包括大设备号)

Technical Notes

Linux ext4 设备号存储格式

设备号存储在 i_block[0..1]:

  • i_block[0] != 0: old format, decode from i_block[0]
  • i_block[0] == 0: new format, decode from i_block[1]

这与常规文件使用 extent tree 是互斥的——设备节点绝不能调用 extent_init()

FIFO 实现

FIFO 在 ext4 中作为普通 inode 存储(无 extent 数据),运行时关联一个 PipeInode 实现进程间通信。

Test plan

  • 在 DragonOS 中运行 test_mknod 测试套件
  • 验证 /dev/null, /dev/zero 等标准设备节点的 stat 输出
  • 验证 FIFO 的进程间通信功能
  • 验证与 Linux 用户态程序的二进制兼容性(设备号编码)

Signed-off-by: xboHodx <xbohodx2024@gmail.com>
Signed-off-by: xboHodx <xbohodx2024@gmail.com>
Signed-off-by: xboHodx <xbohodx2024@gmail.com>
Signed-off-by: xboHodx <xbohodx2024@gmail.com>
Comment on lines 521 to 540
// 字符设备/块设备需要存储设备号,但 another_ext4 crate 暂不支持
// TODO: 修改 another_ext4 使用 i_block[0:1] 存储 dev_t(Linux ext4 标准做法)
if mode.contains(InodeMode::S_IFCHR) || mode.contains(InodeMode::S_IFBLK) {
log::error!(
"ext4::mknod: device nodes not supported (filename='{}', dev={}:{})",
filename,
dev_t.major().data(),
dev_t.minor()
);
return Err(SystemError::ENOSYS);
}

let mut guard = self.0.lock();
let ext4 = &guard.concret_fs().fs;
let inode_num = guard.inner_inode_num;

// 确保当前 inode 是目录
if ext4.getattr(inode_num)?.ftype != FileType::Directory {
return Err(SystemError::ENOTDIR);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

参考下fat?

@fslongjin
Copy link
Member

@claude review 下,思考这个更改是否合理。有没有更好的

@dragonos-community-ai-pr-reviewer
Copy link

dragonos-community-ai-pr-reviewer bot commented Feb 2, 2026

Claude finished @fslongjin's task in 11m 11s —— View job


Claude 代码审查

正在分析 PR #1759 的更改,该 PR 添加了 ext4 文件系统的 mknod 函数。

当前任务列表:

  • 创建 todo 列表并更新 GitHub 评论
  • 查看 PR 更改的文件内容
  • 分析 ext4 mknod 实现
  • 审查代码合理性并提供反馈
  • 思考是否有更好的实现方式

最终审查总结

经过全面分析,以下是我的完整审查意见:

1. 当前实现评价

优点:

  1. ✅ 基础架构正确,符合 VFS 接口规范
  2. ✅ 系统调用层改进合理(模式解析、umask 处理、错误检查)
  3. ✅ 对于 another_ext4 库的限制处理得当(返回 ENOSYS 而非 panic)
  4. ✅ 代码风格与项目一致

主要问题:

  1. 缺少 FIFO 和 socket 支持 - 当前只处理了普通文件和设备文件
  2. 设备节点支持不完整 - 由于 another_ext4 库限制(已注明)
  3. 未参考其他文件系统的最佳实践 - 如 tmpfsramfs 的实现

2. 与其他文件系统实现的比较

参考了 DragonOS 中多个文件系统的 mknod 实现:

文件系统 FIFO 支持 字符设备 块设备 设备号存储 实现完整性
tmpfs ✅ 完整 ✅ 标记类型 ✅ 标记类型 raw_dev 字段
ramfs ✅ 完整 ⚠️ unimplemented!() ⚠️ unimplemented!() ❌ 未存储
devfs ✅ 完整 ✅ 完整 ✅ 完整 ✅ 通过 create_with_data
fat ✅ 完整 ⚠️ unimplemented!() ⚠️ unimplemented!() ❌ 未存储
ext4 (当前) ❌ 缺失 ENOSYS ENOSYS ❌ 库不支持

3. 具体改进建议

a. 添加 FIFO 支持(关键缺失)

参考 tmpfsramfs 的实现:

if mode.contains(InodeMode::S_IFIFO) {
    // 创建管道 inode
    let pipe_inode = LockedPipeInode::new();
    pipe_inode.set_fifo(); // 标记为命名管道
    
    // 创建 ext4 inode(底层存储)
    let id = ext4.create(inode_num, filename, file_mode)?;
    let vfs_inode = LockedExt4Inode::new(...);
    
    // 设置 special_node 存储管道数据
    vfs_inode.0.lock().special_node = Some(SpecialNodeData::Pipe(pipe_inode));
    return Ok(vfs_inode);
}
b. 改进模式转换安全性
// 确保只传递 another_ext4 支持的位
let ext4_mode_bits = mode.bits() & 0x1FF; // 标准 Unix 权限位(9位)
let file_mode = another_ext4::InodeMode::from_bits_truncate(ext4_mode_bits as u16);
c. 统一错误处理
  • self_ref.upgrade() 失败返回 EIO 而非 ENOENT
  • 添加更详细的错误日志
d. 未来设备节点支持

保持当前 ENOSYS 返回,但完善 TODO 注释:

// TODO: 需要修改 another_ext4 库以支持设备号存储
// Linux ext4 标准做法:使用 inode->i_block[0:1] 存储 dev_t
// 参见:https://www.kernel.org/doc/html/latest/filesystems/ext4/dynamic.html#special-inodes

4. 更好的实现方式

建议重构方案:

  1. 提取公共逻辑到辅助函数

    • 模式验证和转换
    • 权限位处理
    • 特殊文件类型创建(FIFO、socket)
  2. 分阶段实现

    • 阶段1:当前 PR + FIFO 支持(最小可行)
    • 阶段2:等待 another_ext4 库支持设备节点
    • 阶段3:添加 socket 支持
  3. 参考 tmpfs 实现结构

    match file_type {
        S_IFREG => self.create(...),
        S_IFIFO => self.create_fifo(...),
        S_IFCHR | S_IFBLK => self.create_device(...),
        S_IFSOCK => self.create_socket(...),
        _ => Err(EINVAL),
    }

5. 结论

当前 PR 的合理性:基础合理,但不完整。作为 mknod 函数的基础实现可以接受,但需要后续完善。

建议操作

  1. 接受当前 PR 作为基础实现
  2. 立即添加 FIFO 支持(关键缺失功能)
  3. 创建 issue 跟踪设备节点支持,依赖 another_ext4 库改进
  4. 参考 tmpfs 实现完善特殊文件处理

如果我是代码维护者:我会要求作者至少添加 FIFO 支持后再合并,因为这是 mknod 的基本功能之一。


行动建议

  1. Fix this → - 添加 FIFO 支持
  2. Fix this → - 改进模式转换安全性
  3. Fix this → - 改进错误处理

最终建议:要求作者补充 FIFO 支持后合并此 PR。设备节点支持可后续实现。

- 实现设备节点创建(mknod)和属性获取
- 添加设备号编码/解码工具函数
- 扩展Inode结构支持设备号存储
- 添加相关单元测试

Signed-off-by: xboHodx <xbohodx2024@gmail.com>
Signed-off-by: xboHodx <xbohodx2024@gmail.com>
Signed-off-by: xboHodx <xbohodx2024@gmail.com>
- 添加DeviceNumber::from_linux_dev_t方法解析Linux dev_t格式
- 在ext4 inode中增加special_node字段支持FIFO管道
- 修复设备节点元数据中的dev_id和raw_dev处理
- 更新mknod/mknodat系统调用使用新的dev_t解析方法

Signed-off-by: xboHodx <xbohodx2024@gmail.com>
- 移除不必要的文件打开/关闭操作
- 使用do_newfstatat替代原有实现

Signed-off-by: xboHodx <xbohodx2024@gmail.com>
@xboHodx
Copy link
Contributor Author

xboHodx commented Feb 4, 2026

单元测试和集成测试加起来有1300行hhh

@fslongjin
Copy link
Member

@codex review下功能设计是否合理、并且有无并发问题

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f170d43847

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines 206 to 210
// Create device inode (uses create_device_inode which sets device number)
let mut child = self.create_device_inode(mode, major, minor)?;

// Link to parent directory
self.link_inode(&mut parent_ref, &mut child, name)?;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Cleanup device inode on link failure to avoid leaks

If another thread creates the same name between the caller’s existence check and this link, link_inode can fail (e.g., EEXIST). In that case the freshly allocated device inode is left on disk with no directory entry, leaking an inode and potentially exhausting the inode table over time. This is a concurrency window introduced by the new mknod path; consider checking for name existence inside Ext4::mknod and/or freeing the inode on error (similar to rollback logic) so failed links don’t orphan device inodes.

Useful? React with 👍 / 👎.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mknod的父目录在这个过程是被加锁了的,应该不存在并发窗口。见kernel/src/filesystem/ext4/inode.rs:542

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@codex 看看这个呢?

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f170d43847

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines 206 to 210
// Create device inode (uses create_device_inode which sets device number)
let mut child = self.create_device_inode(mode, major, minor)?;

// Link to parent directory
self.link_inode(&mut parent_ref, &mut child, name)?;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Free the allocated inode when linking fails

If link_inode fails (e.g., dir_add_entry hits ENOSPC when it has to append a directory block), this path returns early with an error but the device inode allocated by create_device_inode has already been written to disk and the inode bitmap updated. That leaves an unlinked inode leak and inconsistent free-inode counts for the filesystem. Consider cleaning up (free_inode) on error from link_inode so that mknod behaves transactionally when the directory insert cannot be completed.

Useful? React with 👍 / 👎.

- 新增link_new_inode_or_free方法,在链接失败时自动清理新分配的inode
- 在create_file和mknod中使用新方法替代直接调用link_inode

Signed-off-by: xboHodx <xbohodx2024@gmail.com>
@xboHodx xboHodx requested a review from fslongjin February 10, 2026 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants