rcore-os / rCore-Tutorial-Book-v3

A book about how to write OS kernels in Rust easily.
https://rcore-os.github.io/rCore-Tutorial-Book-v3/
GNU General Public License v3.0
1.23k stars 233 forks source link

rCore-Tutorial-Book-v3/chapter6/2fs-implementation #102

Open utterances-bot opened 2 years ago

utterances-bot commented 2 years ago

简易文件系统 easy-fs — rCore-Tutorial-Book-v3 3.6.0-alpha.1 文档

https://rcore-os.github.io/rCore-Tutorial-Book-v3/chapter6/2fs-implementation.html

lindyang commented 2 years ago

代码 const INDIRECT1_BOUND: usize = DIRECT_BOUND + INODE_INDIRECT1_COUNT;

缺少对 DIRECT_BOUND 的定义。

YdrMaster commented 2 years ago

看到本节(以及之前各章节),我对 API 的命名有不同看法,get 用的太多了。rust 官方有命名规范的,其中明确提出不宜滥用 get


既然 BlockCacheManager 每次移除都要遍历,为啥还要用 VecDeque 呢?直接用 Vec 反倒能 O(1) 移除。

#[derive(Default)]
pub struct BlockCacheManager {
    queue: Vec<(usize, Arc<Mutex<BlockCache>>)>,
}

impl BlockCacheManager {
    pub fn get_block_cache(
        &mut self,
        block_id: usize,
        block_device: Arc<dyn BlockDevice>,
    ) -> Arc<Mutex<BlockCache>> {
        if let Some((_, cache)) = self.queue.iter().find(|(id, _)| *id == block_id) {
            cache.clone()
        } else {
            // substitute
            if self.queue.len() == BLOCK_CACHE_SIZE {
                // from front to tail
                if let Some((idx, _)) = self
                    .queue
                    .iter()
                    .enumerate()
                    .find(|(_, (_, cache))| Arc::strong_count(cache) == 1)
                {
                    self.queue.swap_remove(idx);
                } else {
                    panic!("Run out of BlockCache!");
                }
            }
            // load block into mem and push back
            let block_cache = Arc::new(Mutex::new(BlockCache::new(block_id, block_device.clone())));
            self.queue.push((block_id, Arc::clone(&block_cache)));
            block_cache
        }
    }
}
wyfcyx commented 2 years ago

@YdrMaster 非常感谢您的反馈!其实我本身也是一个Rust初学者,还在努力熟悉社区的各项规范中,您提到的API接口命名的问题确实存在,后面我会去加以调整,如果您看到有特别违和的接口命名也可以直接指出。关于后面的BlockCacheManager,其实当时是想实现一个类似于LRU的替换算法,如果使用swap_remove的话就会破坏掉目前按照首次使用时间递增的顺序。

YdrMaster commented 2 years ago

@wyfcyx 客气了,共同努力。Rust 实现 LRU 大概是非 unsafe 不可的,可以考虑用这个 crate,当然我感觉对于教学来说增加这种复杂性没有必要。不过目前的 VecDeque 又有点不知所谓,没有用到复杂度换来的顺序,所以我想还不如换成 Vec 算了。

MrZLeo commented 2 years ago
pub fn dealloc(&self, block_device: &Arc<dyn BlockDevice>, bit: usize) {
        let (block_pos, bits64_pos, inner_pos) = decomposition(bit);
        get_block_cache(
            block_pos + self.start_block_id,
            Arc::clone(block_device)
        ).lock().modify(0, |bitmap_block: &mut BitmapBlock| {
            assert!(bitmap_block[bits64_pos] & (1u64 << inner_pos) > 0);
            bitmap_block[bits64_pos] -= 1u64 << inner_pos;
        });
    }

其中的 bitmap_block[bits64_pos] -= 1u64 << inner_pos; 写成 bitmap_block[bits64_pos] &= !(1u64 << inner_pos); 比较合理

MrZLeo commented 2 years ago

目录项 Dirent DirEntry 最大允许保存长度为 27 的文件/目录名

另外,DirEntry 中创建空目录项的方法 empty 的方法名在一般的Rust规范中等同于Java中的isEmpty,有特殊的含义,建议更改命名

itewqq commented 2 years ago

请教一下,EasyFileSystem中的open函数,为什么inode_bitmap和data_bitmap都是重新构造的呢?一个已写入了 easy-fs 镜像的块设备上面不也应该存储着这些bitmap嘛?

itewqq commented 2 years ago

vfs中的Inode::create 一开始判断是否是directory的时候,应该不需要用&mut DiskNode?这里是否改成 read_disk_inode 更合适一些?

wyfcyx commented 2 years ago

请教一下,EasyFileSystem中的open函数,为什么inode_bitmap和data_bitmap都是重新构造的呢?一个已写入了 easy-fs 镜像的块设备上面不也应该存储着这些bitmap嘛?

请注意EasyFileSystem是一个驻留在内存中的数据结构,其中的Bitmap字段也只表示位图的元数据,真正保存在块设备上的位图需要转换为BitmapBlock类型进行读写。

wyfcyx commented 2 years ago

vfs中的Inode::create 一开始判断是否是directory的时候,应该不需要用&mut DiskNode?这里是否改成 read_disk_inode 更合适一些?

这个可以看看最新版代码,文档还没来得及更新。

itewqq commented 2 years ago

@wyfcyx 理解了,非常感谢您的答疑解惑!

Direktor799 commented 2 years ago

想请教一下关于BlockCacheManager的实现,VecDeque中只以block_id作为标识的话,同时读写不同设备的同一个block时不会有冲突吗

1730735794 commented 2 years ago

请问这里块缓冲管理器用lazy_static的话,不是会导致即使有多个设备也只会有一个BlockCacheManager吗?然而BlockCacheManager中又只用了block_id判断当前磁盘块是否位于缓冲区中,这样可能会导致当设备A的第0块在缓冲区中时,我需要使用设备B的第0块,但BlockCacheManager错误的认为第0块已经在缓冲区中,导致返回了错误的数据。

wyfcyx commented 2 years ago

回复 @Direktor799 和 @1730735794 :目前Tutorial假定计算机中只有一个块设备,如果同时需要处理多个块设备的话确实会有问题。

Direktor799 commented 2 years ago

@wyfcyx 明白了,感谢

renranwuyue commented 2 years ago

easy_fs_pack函数命令行参数解析那块推荐使用 clap3的derive api 非常方便

# Cargo.toml
[dependencies]
clap = { version = "3.1.15", features = ["derive"] }
/// EasyFileSystem packer
#[derive(Parser)] // 需要导入 clap::Parser
#[clap(author, version, about, long_about = None)]
struct App {
    /// Executable source dir(with backslash)
    #[clap(short, long)]
    source: String,
    /// Executable target dir(with backslash)
    #[clap(short, long)]
    target: String,
}
let app:App = App::parse();
whfuyn commented 2 years ago
    pub fn get_ref<T>(&self, offset: usize) -> &T where T: Sized {
        let type_size = core::mem::size_of::<T>();
        assert!(offset + type_size <= BLOCK_SZ);
        let addr = self.addr_of_offset(offset);
        unsafe { &*(addr as *const T) }
    }

这里的get_refget_mut感觉都得标上unsafe。

我的理解是做这种转换要求目标内存有正确的对齐并且是个合法的T类型的值,不然属于UB,因为编译器的有些优化会依赖这点。

sirzjp commented 2 years ago

get_mut 得到可变应用数据并未修改,而是在modify中修改,self.modified = true; 应该放到modify中

CelestialMelody commented 2 years ago

磁盘块管理器中 create 方法

    let data_total_blocks = total_blocks - 1 - inode_total_blocks;
    let data_bitmap_blocks = (data_total_blocks + 4096) / 4097;

有两个问题:

  1. 为什么这里 data_total_blocks要减去1呢?
  2. 为什么这里是除 4097 而不是 4096?除 4096 不正确吗?
wyfcyx commented 2 years ago

@CelestialMelody

  1. 这里的1对应的是超级块SuperBlock
  2. 我们希望位图覆盖后面的数据块的前提下数据块尽量多。设数据的位图占据x个块,则该位图能管理的数据块不超过4096x。数据区域总共data_total_blocks个块,除了数据位图的块剩下都是数据块,也就是位图管理的数据块为data_total_blocks-x个块。于是有不等式data_total_blocks-x<=4096x,得到x>=data_total_blocks/4097。数据块尽量多也就要求位图块数尽量少,于是取x的最小整数解也就是data_total_blocks/4097上取整,也就是代码中的表达式。
rainsmell commented 1 year ago

"而互斥访问在单核上的意义在于提供内部可变性通过编译" 请问这句话该怎么理解啊

Ayana-chan commented 1 year ago

每个文件/目录在磁盘上均以一个 DiskInode 的形式存储。

应当改成

每个文件/目录的索引节点在磁盘上均以一个 DiskInode 的形式存储。

zhangzijie-pro commented 11 months ago

get_block_cache( self.block_id, Arc::clone(&self.block_device) ).lock().read(self.blockoffset, f) lock()后返回的是LockResult<MutexGuard<', T>>,应该后面加unwrap()处理一下才可以使用read()吧

olinex commented 8 months ago

不知道各位有没有遇到这个问题: 当尝试从文件系统镜像中读取一个比较大的可执行文件时(超过 8kb), 进程的内核栈就会溢出从而导致内核崩溃. 我自己的分析如下:

pub struct BlockCache {
    cache: [u8; BLOCK_SZ], #这个地方cache的数据解构是保存在栈上的
    block_id: usize,
    block_device: Arc<dyn BlockDevice>,
    modified: bool,
}
impl BlockCache {
    /// Load a new BlockCache from disk.
    # new 函数会在 syscall exec 函数中调用, 因为要从块设备中读取文件
    pub fn new(
        block_id: usize,
        block_device: Arc<dyn BlockDevice>
    ) -> Self {
        # 这一段字节数组会在被 alloc 在某个进程的内存栈中
        let mut cache = [0u8; BLOCK_SZ];
        block_device.read_block(block_id, &mut cache);
        Self {
            cache,
            block_id,
            block_device,
            modified: false,
        }
    }
}

由于进程的内核栈仅有 8kb 大小, 在加载大文件时就会导致栈溢出. 建议应该通过 box<[0u8; BLOCK_SZ]>封装一下, 将缓存移到内核的堆空间当中.

由于我是参考文档所有的代码都按照自己的理解重写了一遍, 所以可能这只是我自己遇到的问题. 不知道大家有没有遇到呢?