Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(mm): 修复fat文件系统的PageCache同步问题 #1005

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 78 additions & 10 deletions kernel/src/filesystem/fat/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1391,17 +1391,44 @@ impl IndexNode for LockedFATInode {
buf: &mut [u8],
_data: SpinLockGuard<FilePrivateData>,
) -> Result<usize, SystemError> {
let len = core::cmp::min(len, buf.len());
let buf = &mut buf[0..len];
let mut guard: SpinLockGuard<FATInode> = self.0.lock();
match &guard.inode_type {
FATDirEntry::File(f) | FATDirEntry::VolId(f) => {
let r = f.read(
&guard.fs.upgrade().unwrap(),
&mut buf[0..len],
offset as u64,
);
guard.update_metadata();
Copy link
Member

Choose a reason for hiding this comment

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

这个为啥删了?这个是用来更新读取时间的。(只是现在还没实现)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

这个方法会把inode记录的size跟磁盘的size同步导致错误,我看看是不是要修改一下

Copy link
Member

Choose a reason for hiding this comment

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

这个方法会把inode记录的size跟磁盘的size同步导致错误,我看看是不是要修改一下

啊哈,这又是什么bug哈哈

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

就是说,如果启用pagecache的话inode的size跟fat文件的size不是保持一致的,但是这个方法会直接让inode的size更新为fat的size导致用户态获取的文件大小不正确
image

Copy link
Member

Choose a reason for hiding this comment

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

哦哦,那最终回写的时候就会把它弄成一致的,对吧?

Copy link
Collaborator Author

@MemoryShore MemoryShore Nov 12, 2024

Choose a reason for hiding this comment

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

对的,不过如果需要更新读取时间的话感觉不能直接删掉,而是要修改一下方法的逻辑,不能直接把size同步

Copy link
Member

Choose a reason for hiding this comment

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

对的,不过如果需要更新读取时间的话感觉不能直接删掉,而是要修改一下方法的逻辑,不能直接把size同步

确实

return r;
// log::debug!(
// "read_at len: {len}, offset: {offset}, file_size:{:?}",
// guard.metadata.size
// );

let file_size = guard.metadata.size;

let len = if offset < file_size as usize {
core::cmp::min(file_size as usize, offset + len) - offset
} else {
0
};

if let Some(page_cache) = &guard.page_cache {
let r = page_cache.read(
offset,
&mut buf[0..len],
|file_offset: usize, buf: &mut [u8]| {
f.read(&guard.fs.upgrade().unwrap(), buf, file_offset as u64)
},
);
return r;
} else {
let r = f.read(
&guard.fs.upgrade().unwrap(),
&mut buf[0..len],
offset as u64,
);
guard.update_metadata();
return r;
}
}

FATDirEntry::Dir(_) => {
return Err(SystemError::EISDIR);
}
Expand All @@ -1419,18 +1446,30 @@ impl IndexNode for LockedFATInode {
buf: &[u8],
_data: SpinLockGuard<FilePrivateData>,
) -> Result<usize, SystemError> {
let len = core::cmp::min(len, buf.len());
let buf = &buf[0..len];
let mut guard: SpinLockGuard<FATInode> = self.0.lock();
let page_cache = guard.page_cache.clone();
let fs: &Arc<FATFileSystem> = &guard.fs.upgrade().unwrap();

match &mut guard.inode_type {
FATDirEntry::File(f) | FATDirEntry::VolId(f) => {
let r = f.write(fs, &buf[0..len], offset as u64);
guard.update_metadata();
return r;
if let Some(page_cache) = page_cache {
let write_len = page_cache.write(offset, buf)?;
let old_size = guard.metadata.size;
guard.metadata.size = core::cmp::max(old_size, (offset + write_len) as i64);
fslongjin marked this conversation as resolved.
Show resolved Hide resolved
return Ok(write_len);
} else {
let r = f.write(fs, &buf[0..len], offset as u64);
guard.update_metadata();
return r;
}
}

FATDirEntry::Dir(_) => {
return Err(SystemError::EISDIR);
}

FATDirEntry::UnInit => {
error!("FATFS: param: Inode_type uninitialized.");
return Err(SystemError::EROFS);
Expand Down Expand Up @@ -1854,6 +1893,35 @@ impl IndexNode for LockedFATInode {
fn page_cache(&self) -> Option<Arc<PageCache>> {
self.0.lock().page_cache.clone()
}

fn read_direct(
&self,
offset: usize,
len: usize,
buf: &mut [u8],
_data: SpinLockGuard<FilePrivateData>,
) -> Result<usize, SystemError> {
let mut guard: SpinLockGuard<FATInode> = self.0.lock();
match &guard.inode_type {
FATDirEntry::File(f) | FATDirEntry::VolId(f) => {
let r = f.read(
&guard.fs.upgrade().unwrap(),
&mut buf[0..len],
offset as u64,
);
guard.update_metadata();
return r;
}

FATDirEntry::Dir(_) => {
return Err(SystemError::EISDIR);
}
FATDirEntry::UnInit => {
error!("FATFS: param: Inode_type uninitialized.");
return Err(SystemError::EROFS);
}
}
}
}

impl Default for FATFsInfo {
Expand Down
Loading
Loading