From 494c88e993a1854b0a5b0d52830e87fc32dd87e2 Mon Sep 17 00:00:00 2001 From: LI Qing Date: Fri, 24 May 2024 10:32:30 +0800 Subject: [PATCH] Refine ProcFs by improving the use of locks --- kernel/aster-nix/src/fs/procfs/mod.rs | 41 +++++++++---------- .../src/fs/procfs/template/builder.rs | 41 +++++++++++++------ .../aster-nix/src/fs/procfs/template/dir.rs | 40 ++++++++---------- .../aster-nix/src/fs/procfs/template/file.rs | 11 ++--- .../aster-nix/src/fs/procfs/template/mod.rs | 8 +++- .../aster-nix/src/fs/procfs/template/sym.rs | 11 ++--- kernel/aster-nix/src/fs/utils/inode.rs | 22 +++++----- 7 files changed, 95 insertions(+), 79 deletions(-) diff --git a/kernel/aster-nix/src/fs/procfs/mod.rs b/kernel/aster-nix/src/fs/procfs/mod.rs index e67233e28..beecbc758 100644 --- a/kernel/aster-nix/src/fs/procfs/mod.rs +++ b/kernel/aster-nix/src/fs/procfs/mod.rs @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MPL-2.0 -use core::sync::atomic::{AtomicUsize, Ordering}; +use core::sync::atomic::{AtomicU64, Ordering}; use self::{ pid::PidDirOps, @@ -21,33 +21,26 @@ mod template; /// Magic number. const PROC_MAGIC: u64 = 0x9fa0; /// Root Inode ID. -const PROC_ROOT_INO: usize = 1; +const PROC_ROOT_INO: u64 = 1; /// Block size. const BLOCK_SIZE: usize = 1024; pub struct ProcFS { - sb: RwLock, - root: RwLock>>, - inode_allocator: AtomicUsize, + sb: SuperBlock, + root: Arc, + inode_allocator: AtomicU64, } impl ProcFS { pub fn new() -> Arc { - let procfs = { - let sb = SuperBlock::new(PROC_MAGIC, BLOCK_SIZE, NAME_MAX); - Arc::new(Self { - sb: RwLock::new(sb), - root: RwLock::new(None), - inode_allocator: AtomicUsize::new(PROC_ROOT_INO), - }) - }; - - let root = RootDirOps::new_inode(&procfs); - *procfs.root.write() = Some(root); - procfs + Arc::new_cyclic(|weak_fs| Self { + sb: SuperBlock::new(PROC_MAGIC, BLOCK_SIZE, NAME_MAX), + root: RootDirOps::new_inode(weak_fs.clone()), + inode_allocator: AtomicU64::new(PROC_ROOT_INO + 1), + }) } - pub(in crate::fs::procfs) fn alloc_id(&self) -> usize { + pub(in crate::fs::procfs) fn alloc_id(&self) -> u64 { self.inode_allocator.fetch_add(1, Ordering::SeqCst) } } @@ -58,11 +51,11 @@ impl FileSystem for ProcFS { } fn root_inode(&self) -> Arc { - self.root.read().as_ref().unwrap().clone() + self.root.clone() } fn sb(&self) -> SuperBlock { - self.sb.read().clone() + self.sb.clone() } fn flags(&self) -> FsFlags { @@ -74,8 +67,12 @@ impl FileSystem for ProcFS { struct RootDirOps; impl RootDirOps { - pub fn new_inode(fs: &Arc) -> Arc { - let root_inode = ProcDirBuilder::new(Self).fs(fs.clone()).build().unwrap(); + pub fn new_inode(fs: Weak) -> Arc { + let root_inode = ProcDirBuilder::new(Self) + .fs(fs) + .ino(PROC_ROOT_INO) + .build() + .unwrap(); let weak_ptr = Arc::downgrade(&root_inode); process_table::register_observer(weak_ptr); root_inode diff --git a/kernel/aster-nix/src/fs/procfs/template/builder.rs b/kernel/aster-nix/src/fs/procfs/template/builder.rs index ed3f28fd7..0d85ae87f 100644 --- a/kernel/aster-nix/src/fs/procfs/template/builder.rs +++ b/kernel/aster-nix/src/fs/procfs/template/builder.rs @@ -32,7 +32,7 @@ impl ProcDirBuilder { self.optional_builder(|ob| ob.parent(parent)) } - pub fn fs(self, fs: Arc) -> Self { + pub fn fs(self, fs: Weak) -> Self { self.optional_builder(|ob| ob.fs(fs)) } @@ -40,9 +40,13 @@ impl ProcDirBuilder { self.optional_builder(|ob| ob.volatile()) } + pub fn ino(self, ino: u64) -> Self { + self.optional_builder(|ob| ob.ino(ino)) + } + pub fn build(mut self) -> Result>> { - let (fs, parent, is_volatile) = self.optional_builder.take().unwrap().build()?; - Ok(ProcDir::new(self.dir, fs, parent, is_volatile)) + let (fs, parent, ino, is_volatile) = self.optional_builder.take().unwrap().build()?; + Ok(ProcDir::new(self.dir, fs, parent, ino, is_volatile)) } fn optional_builder(mut self, f: F) -> Self @@ -80,7 +84,7 @@ impl ProcFileBuilder { } pub fn build(mut self) -> Result>> { - let (fs, _, is_volatile) = self.optional_builder.take().unwrap().build()?; + let (fs, _, _, is_volatile) = self.optional_builder.take().unwrap().build()?; Ok(ProcFile::new(self.file, fs, is_volatile)) } @@ -119,7 +123,7 @@ impl ProcSymBuilder { } pub fn build(mut self) -> Result>> { - let (fs, _, is_volatile) = self.optional_builder.take().unwrap().build()?; + let (fs, _, _, is_volatile) = self.optional_builder.take().unwrap().build()?; Ok(ProcSym::new(self.sym, fs, is_volatile)) } @@ -136,7 +140,8 @@ impl ProcSymBuilder { #[derive(Default)] struct OptionalBuilder { parent: Option>, - fs: Option>, + fs: Option>, + ino: Option, is_volatile: bool, } @@ -146,24 +151,36 @@ impl OptionalBuilder { self } - pub fn fs(mut self, fs: Arc) -> Self { + pub fn fs(mut self, fs: Weak) -> Self { self.fs = Some(fs); self } + pub fn ino(mut self, ino: u64) -> Self { + self.ino = Some(ino); + self + } + pub fn volatile(mut self) -> Self { self.is_volatile = true; self } #[allow(clippy::type_complexity)] - pub fn build(self) -> Result<(Arc, Option>, bool)> { + pub fn build( + self, + ) -> Result<( + Weak, + Option>, + Option, + bool, + )> { if self.parent.is_none() && self.fs.is_none() { return_errno_with_message!(Errno::EINVAL, "must have parent or fs"); } - let fs = self - .fs - .unwrap_or_else(|| self.parent.as_ref().unwrap().upgrade().unwrap().fs()); + let fs = self.fs.unwrap_or_else(|| { + Arc::downgrade(&self.parent.as_ref().unwrap().upgrade().unwrap().fs()) + }); // The volatile property is inherited from parent. let is_volatile = { @@ -176,6 +193,6 @@ impl OptionalBuilder { is_volatile }; - Ok((fs, self.parent, is_volatile)) + Ok((fs, self.parent, self.ino, is_volatile)) } } diff --git a/kernel/aster-nix/src/fs/procfs/template/dir.rs b/kernel/aster-nix/src/fs/procfs/template/dir.rs index 3e235293c..6ddcb378a 100644 --- a/kernel/aster-nix/src/fs/procfs/template/dir.rs +++ b/kernel/aster-nix/src/fs/procfs/template/dir.rs @@ -21,31 +21,37 @@ pub struct ProcDir { inner: D, this: Weak>, parent: Option>, - cached_children: RwLock)>>, + cached_children: RwMutex)>>, common: Common, } impl ProcDir { pub fn new( dir: D, - fs: Arc, + fs: Weak, parent: Option>, + ino: Option, is_volatile: bool, ) -> Arc { let common = { - let procfs = fs.downcast_ref::().unwrap(); + let ino = ino.unwrap_or_else(|| { + let arc_fs = fs.upgrade().unwrap(); + let procfs = arc_fs.downcast_ref::().unwrap(); + procfs.alloc_id() + }); + let metadata = Metadata::new_dir( - procfs.alloc_id(), + ino as _, InodeMode::from_bits_truncate(0o555), - &fs.sb(), + super::BLOCK_SIZE, ); - Common::new(metadata, Arc::downgrade(&fs), is_volatile) + Common::new(metadata, fs, is_volatile) }; Arc::new_cyclic(|weak_self| Self { inner: dir, this: weak_self.clone(), parent, - cached_children: RwLock::new(SlotVec::new()), + cached_children: RwMutex::new(SlotVec::new()), common, }) } @@ -58,7 +64,7 @@ impl ProcDir { self.parent.as_ref().and_then(|p| p.upgrade()) } - pub fn cached_children(&self) -> &RwLock)>> { + pub fn cached_children(&self) -> &RwMutex)>> { &self.cached_children } } @@ -108,20 +114,15 @@ impl Inode for ProcDir { let this_inode = self.this(); visitor.visit( ".", - this_inode.common.metadata().ino as u64, - this_inode.common.metadata().type_, + this_inode.common.ino(), + this_inode.common.type_(), *offset, )?; *offset += 1; } if *offset == 1 { let parent_inode = self.parent().unwrap_or(self.this()); - visitor.visit( - "..", - parent_inode.metadata().ino as u64, - parent_inode.metadata().type_, - *offset, - )?; + visitor.visit("..", parent_inode.ino(), parent_inode.type_(), *offset)?; *offset += 1; } @@ -134,12 +135,7 @@ impl Inode for ProcDir { .map(|(idx, (name, child))| (idx + 2, (name, child))) .skip_while(|(idx, _)| idx < &start_offset) { - visitor.visit( - name.as_ref(), - child.metadata().ino as u64, - child.metadata().type_, - idx, - )?; + visitor.visit(name.as_ref(), child.ino(), child.type_(), idx)?; *offset = idx + 1; } Ok(()) diff --git a/kernel/aster-nix/src/fs/procfs/template/file.rs b/kernel/aster-nix/src/fs/procfs/template/file.rs index e15ba31d8..7b27abd3f 100644 --- a/kernel/aster-nix/src/fs/procfs/template/file.rs +++ b/kernel/aster-nix/src/fs/procfs/template/file.rs @@ -17,15 +17,16 @@ pub struct ProcFile { } impl ProcFile { - pub fn new(file: F, fs: Arc, is_volatile: bool) -> Arc { + pub fn new(file: F, fs: Weak, is_volatile: bool) -> Arc { let common = { - let procfs = fs.downcast_ref::().unwrap(); + let arc_fs = fs.upgrade().unwrap(); + let procfs = arc_fs.downcast_ref::().unwrap(); let metadata = Metadata::new_file( - procfs.alloc_id(), + procfs.alloc_id() as _, InodeMode::from_bits_truncate(0o444), - &fs.sb(), + super::BLOCK_SIZE, ); - Common::new(metadata, Arc::downgrade(&fs), is_volatile) + Common::new(metadata, fs, is_volatile) }; Arc::new(Self { inner: file, diff --git a/kernel/aster-nix/src/fs/procfs/template/mod.rs b/kernel/aster-nix/src/fs/procfs/template/mod.rs index 68e134d6f..065795e4b 100644 --- a/kernel/aster-nix/src/fs/procfs/template/mod.rs +++ b/kernel/aster-nix/src/fs/procfs/template/mod.rs @@ -8,9 +8,9 @@ pub use self::{ file::FileOps, sym::SymOps, }; -use super::ProcFS; +use super::{ProcFS, BLOCK_SIZE}; use crate::{ - fs::utils::{FileSystem, InodeMode, Metadata}, + fs::utils::{FileSystem, InodeMode, InodeType, Metadata}, prelude::*, process::{Gid, Uid}, }; @@ -47,6 +47,10 @@ impl Common { self.metadata.read().ino as _ } + pub fn type_(&self) -> InodeType { + self.metadata.read().type_ + } + pub fn size(&self) -> usize { self.metadata.read().size } diff --git a/kernel/aster-nix/src/fs/procfs/template/sym.rs b/kernel/aster-nix/src/fs/procfs/template/sym.rs index 7a7b98480..420334139 100644 --- a/kernel/aster-nix/src/fs/procfs/template/sym.rs +++ b/kernel/aster-nix/src/fs/procfs/template/sym.rs @@ -17,15 +17,16 @@ pub struct ProcSym { } impl ProcSym { - pub fn new(sym: S, fs: Arc, is_volatile: bool) -> Arc { + pub fn new(sym: S, fs: Weak, is_volatile: bool) -> Arc { let common = { - let procfs = fs.downcast_ref::().unwrap(); + let arc_fs = fs.upgrade().unwrap(); + let procfs = arc_fs.downcast_ref::().unwrap(); let metadata = Metadata::new_symlink( - procfs.alloc_id(), + procfs.alloc_id() as _, InodeMode::from_bits_truncate(0o777), - &fs.sb(), + super::BLOCK_SIZE, ); - Common::new(metadata, Arc::downgrade(&fs), is_volatile) + Common::new(metadata, fs, is_volatile) }; Arc::new(Self { inner: sym, common }) } diff --git a/kernel/aster-nix/src/fs/utils/inode.rs b/kernel/aster-nix/src/fs/utils/inode.rs index 86bfefd96..b7dd09f31 100644 --- a/kernel/aster-nix/src/fs/utils/inode.rs +++ b/kernel/aster-nix/src/fs/utils/inode.rs @@ -7,7 +7,7 @@ use core::time::Duration; use aster_rights::Full; use core2::io::{Error as IoError, ErrorKind as IoErrorKind, Result as IoResult, Write}; -use super::{DirentVisitor, FileSystem, IoctlCmd, SuperBlock}; +use super::{DirentVisitor, FileSystem, IoctlCmd}; use crate::{ events::IoEvents, fs::device::{Device, DeviceType}, @@ -136,12 +136,12 @@ pub struct Metadata { } impl Metadata { - pub fn new_dir(ino: usize, mode: InodeMode, sb: &SuperBlock) -> Self { + pub fn new_dir(ino: usize, mode: InodeMode, blk_size: usize) -> Self { Self { dev: 0, ino, size: 2, - blk_size: sb.bsize, + blk_size, blocks: 1, atime: Default::default(), mtime: Default::default(), @@ -155,12 +155,12 @@ impl Metadata { } } - pub fn new_file(ino: usize, mode: InodeMode, sb: &SuperBlock) -> Self { + pub fn new_file(ino: usize, mode: InodeMode, blk_size: usize) -> Self { Self { dev: 0, ino, size: 0, - blk_size: sb.bsize, + blk_size, blocks: 0, atime: Default::default(), mtime: Default::default(), @@ -174,12 +174,12 @@ impl Metadata { } } - pub fn new_symlink(ino: usize, mode: InodeMode, sb: &SuperBlock) -> Self { + pub fn new_symlink(ino: usize, mode: InodeMode, blk_size: usize) -> Self { Self { dev: 0, ino, size: 0, - blk_size: sb.bsize, + blk_size, blocks: 0, atime: Default::default(), mtime: Default::default(), @@ -192,12 +192,12 @@ impl Metadata { rdev: 0, } } - pub fn new_device(ino: usize, mode: InodeMode, sb: &SuperBlock, device: &dyn Device) -> Self { + pub fn new_device(ino: usize, mode: InodeMode, blk_size: usize, device: &dyn Device) -> Self { Self { dev: 0, ino, size: 0, - blk_size: sb.bsize, + blk_size, blocks: 0, atime: Default::default(), mtime: Default::default(), @@ -211,12 +211,12 @@ impl Metadata { } } - pub fn new_socket(ino: usize, mode: InodeMode, sb: &SuperBlock) -> Metadata { + pub fn new_socket(ino: usize, mode: InodeMode, blk_size: usize) -> Metadata { Self { dev: 0, ino, size: 0, - blk_size: sb.bsize, + blk_size, blocks: 0, atime: Default::default(), mtime: Default::default(),