Skip to content

Conversation

@xboHodx
Copy link
Contributor

@xboHodx xboHodx commented Nov 20, 2025

  • 新增log2和ranges工具模块
  • 添加文件预读功能支持

- 添加log2模块实现2的幂次相关计算函数
- 添加ranges模块实现连续整数范围合并功能
- 在mod.rs中导出新增模块
- 新增readahead模块实现文件预读算法
- 修改page_cache模块支持预读标记
- 在File结构体中添加预读状态管理
- 为PageFlags添加PG_READAHEAD标志位
Copilot AI review requested due to automatic review settings November 20, 2025 08:54
@github-actions github-actions bot added the ambiguous The title of PR/issue doesn't match the format label Nov 20, 2025
Copilot finished reviewing on behalf of xboHodx November 20, 2025 08:57
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements file readahead functionality for the DragonOS kernel to improve I/O performance by prefetching pages before they are explicitly requested. The implementation includes utility modules for power-of-two operations and range merging, along with an on-demand readahead algorithm.

Key changes:

  • Adds log2 and ranges utility modules for supporting readahead calculations and optimization
  • Implements on-demand readahead algorithm in readahead.rs with sync/async readahead entry points
  • Integrates readahead into file read operations with automatic triggering based on access patterns

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 13 comments.

Show a summary per file
File Description
kernel/src/libs/log2.rs New utility module providing power-of-two rounding functions for readahead window size calculations
kernel/src/libs/ranges.rs New utility module to merge consecutive page indices into ranges for efficient batch I/O operations
kernel/src/libs/mod.rs Module declarations for the new log2 and ranges utilities
kernel/src/mm/readahead.rs Core readahead implementation with state tracking, control logic, and on-demand algorithm
kernel/src/mm/page.rs Adds PG_READAHEAD flag to mark pages that trigger asynchronous readahead
kernel/src/mm/mod.rs Declares readahead module and adds Sized trait bounds to read/write methods
kernel/src/filesystem/vfs/file.rs Integrates readahead into file operations, adding state tracking and triggering logic in do_read
kernel/src/filesystem/page_cache.rs Refactors create_pages to support partial pages and makes it public for readahead use

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +65 to 68
pub fn create_pages(&mut self, start_page_index: usize, buf: &[u8]) -> Result<(), SystemError> {
if buf.is_empty() {
return Ok(());
}
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

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

[nitpick] The old implementation had an assertion assert!(buf.len().is_multiple_of(MMArch::PAGE_SIZE)) which enforced that the buffer length must be a multiple of page size. The new implementation removes this assertion and handles partial pages. While this is more flexible, if callers were relying on this assertion to catch bugs, they may now silently get unexpected behavior. Consider documenting this behavior change or adding a debug assertion to help catch potential misuse during development.

Copilot uses AI. Check for mistakes.
- 重构页缓存分配逻辑,改进错误处理时资源释放
- 修正file_readahead函数返回值为Result<(), SystemError>
- 更新文档注释中的函数引用名称
- 优化预读控制逻辑,移除不必要的unwrap调用
@xboHodx xboHodx closed this Nov 20, 2025
@xboHodx xboHodx reopened this Nov 20, 2025
@sparkzky sparkzky changed the title Feat/readahead feat: readahead Nov 20, 2025
@github-actions github-actions bot added enhancement New feature or request and removed ambiguous The title of PR/issue doesn't match the format labels Nov 20, 2025

let len = buf.len();
if len == 0 {
pub fn create_pages(&mut self, start_page_index: usize, buf: &[u8]) -> Result<(), SystemError> {
Copy link
Member

Choose a reason for hiding this comment

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

为什么不再需要 assert!(buf.len().is_multiple_of(MMArch::PAGE_SIZE));

Copy link
Contributor Author

Choose a reason for hiding this comment

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

文件大小不一定是页面大小的整数倍,
let page_len = core::cmp::min(MMArch::PAGE_SIZE, buf.len() - buf_offset);
后面
dst[..page_len].copy_from_slice(&buf[buf_offset..buf_offset + page_len]);
这样写就能保证比如最后一页这种不满一页的数据也能写入到页面中

self.do_write(offset, len, buf, false)
}

fn file_readahead(&self, offset: usize, len: usize) -> Result<(), SystemError> {
Copy link
Member

Choose a reason for hiding this comment

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

如果该inode的page cache已经存在(比如当前文件已经预读过,或者当前inode对应的其他file已经预读过),那还会预读,创建新的Page cache吗?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

page_cache是inode的,预读前先检查页面是否已经存在。逻辑上都是从第一个缺页的地方开始实际的读

Comment on lines 7 to 31
#[inline]
pub const fn round_up_pow_of_two(n: usize) -> usize {
if n < 2 {
return 1;
}

1usize << (usize::BITS - (n - 1).leading_zeros())
}

#[inline]
#[allow(unused)]
pub const fn round_down_pow_of_two(n: usize) -> usize {
if n < 2 {
return n;
}

1usize << (usize::BITS - (n).leading_zeros() - 1)
}

/// 检查一个数是否是2的幂次
#[inline]
#[allow(unused)]
pub const fn is_power_of_two(n: usize) -> bool {
n > 0 && (n & (n - 1)) == 0
}
Copy link
Member

Choose a reason for hiding this comment

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

我怎么感觉这些函数的功能rust已经内置了?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

确实,我改掉

@fslongjin
Copy link
Member

image

测例没过

@xboHodx
Copy link
Contributor Author

xboHodx commented Nov 22, 2025

image 测例没过

这是因为在create_pages时设置PG_UPTODATE。应该有才对,但是内核中对这个标志的处理不足,导致panic。我先改掉,现在提个issue

Copy link
Member

@fslongjin fslongjin left a comment

Choose a reason for hiding this comment

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

image 测例没过。本地运行make test-syscall看看

@xboHodx
Copy link
Contributor Author

xboHodx commented Nov 25, 2025

image 测例没过。本地运行make test-syscall看看

本地跑make test-syscall没问题,单独跑duptest就会报错,在最新主线上跑也是如此。

@fslongjin
Copy link
Member

本地跑make test-syscall没问题,单独跑duptest就会报错,在最新主线上跑也是如此。

这个pr肯定是有问题的,因为主线的是能通过的。并且你的分支跟主线之间有差异,先同步一下再跑看看?

@fslongjin fslongjin merged commit 53df56c into DragonOS-Community:master Nov 26, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants