Use SpinLock on FileTable for efficiency

kernel/libs/aster-util/src/slot_vec.rs

@@ -52,6 +52,13 @@ impl<T> SlotVec<T> {
         self.slots.get(idx)?.as_ref()
     }
 
+    /// Get the mutable reference of the item at position `idx`.
+    ///
+    /// Return `None` if `idx` is out of bounds or the item is not exist.
+    pub fn get_mut(&mut self, idx: usize) -> Option<&mut T> {
+        self.slots.get_mut(idx)?.as_mut()
+    }
+
     /// Put an item into the vector.
     /// It may be put into any existing empty slots or the back of the vector.
     ///

kernel/src/fs/file_table.rs

@@ -18,7 +18,6 @@ use crate::{
     net::socket::Socket,
     prelude::*,
     process::{
-        process_table,
         signal::{constants::SIGIO, signals::kernel::KernelSignal},
         Pid, Process,
     },
@@ -125,6 +124,7 @@ impl FileTable {
 
         let closed_file = removed_entry.file;
         if let Some(closed_inode_file) = closed_file.downcast_ref::<InodeHandle>() {
+            // FIXME: Operation below should not hold any mutex if `self` is protected by a spinlock externally
             closed_inode_file.release_range_locks();
         }
         Some(closed_file)
@@ -162,6 +162,7 @@ impl FileTable {
             removed_entry.notify_fd_events(&events);
             closed_files.push(removed_entry.file.clone());
             if let Some(inode_file) = removed_entry.file.downcast_ref::<InodeHandle>() {
+                // FIXME: Operation below should not hold any mutex if `self` is protected by a spinlock externally
                 inode_file.release_range_locks();
             }
         }
@@ -189,6 +190,12 @@ impl FileTable {
             .ok_or(Error::with_message(Errno::EBADF, "fd not exits"))
     }
 
+    pub fn get_entry_mut(&mut self, fd: FileDesc) -> Result<&mut FileTableEntry> {
+        self.table
+            .get_mut(fd as usize)
+            .ok_or(Error::with_message(Errno::EBADF, "fd not exits"))
+    }
+
     pub fn fds_and_files(&self) -> impl Iterator<Item = (FileDesc, &'_ Arc<dyn FileLike>)> {
         self.table
             .idxes_and_items()
@@ -230,7 +237,7 @@ impl Drop for FileTable {
     }
 }
 
-#[derive(Copy, Clone)]
+#[derive(Copy, Clone, Debug)]
 pub enum FdEvents {
     Close(FileDesc),
     DropFileTable,
@@ -242,7 +249,7 @@ pub struct FileTableEntry {
     file: Arc<dyn FileLike>,
     flags: AtomicU8,
     subject: Subject<FdEvents>,
-    owner: RwLock<Option<Owner>>,
+    owner: Option<Owner>,
 }
 
 impl FileTableEntry {
@@ -251,7 +258,7 @@ impl FileTableEntry {
             file,
             flags: AtomicU8::new(flags.bits()),
             subject: Subject::new(),
-            owner: RwLock::new(None),
+            owner: None,
         }
     }
 
@@ -260,7 +267,7 @@ impl FileTableEntry {
     }
 
     pub fn owner(&self) -> Option<Pid> {
-        self.owner.read().as_ref().map(|(pid, _)| *pid)
+        self.owner.as_ref().map(|(pid, _)| *pid)
     }
 
     /// Set a process (group) as owner of the file descriptor.
@@ -268,36 +275,31 @@ impl FileTableEntry {
     /// Such that this process (group) will receive `SIGIO` and `SIGURG` signals
     /// for I/O events on the file descriptor, if `O_ASYNC` status flag is set
     /// on this file.
-    pub fn set_owner(&self, owner: Pid) -> Result<()> {
+    pub fn set_owner(&mut self, owner: Option<&Arc<Process>>) -> Result<()> {
-        if self.owner().is_some_and(|pid| pid == owner) {
+        match owner {
+            None => {
+                // Unset the owner if the given pid is zero
+                if let Some((_, observer)) = self.owner.as_ref() {
+                    let _ = self.file.unregister_observer(&Arc::downgrade(observer));
+                }
+                let _ = self.owner.take();
+            }
+            Some(owner_process) => {
+                let owner_pid = owner_process.pid();
+                if let Some((pid, observer)) = self.owner.as_ref() {
+                    if *pid == owner_pid {
                         return Ok(());
                     }
 
-        // Unset the owner if the given pid is zero.
-        let new_owner = if owner == 0 {
-            None
-        } else {
-            let process = process_table::get_process(owner as _).ok_or(Error::with_message(
-                Errno::ESRCH,
-                "cannot set_owner with an invalid pid",
-            ))?;
-            let observer = OwnerObserver::new(self.file.clone(), Arc::downgrade(&process));
-            Some((owner, observer))
-        };
-
-        let mut self_owner = self.owner.write();
-        if let Some((_, observer)) = self_owner.as_ref() {
                     let _ = self.file.unregister_observer(&Arc::downgrade(observer));
                 }
 
-        *self_owner = match new_owner {
+                let observer = OwnerObserver::new(self.file.clone(), Arc::downgrade(owner_process));
-            None => None,
-            Some((pid, observer)) => {
                 self.file
                     .register_observer(observer.weak_self(), IoEvents::empty())?;
-                Some((pid, observer))
+                let _ = self.owner.insert((owner_pid, observer));
+            }
         }
-        };
         Ok(())
     }
 
@@ -328,7 +330,7 @@ impl Clone for FileTableEntry {
             file: self.file.clone(),
             flags: AtomicU8::new(self.flags.load(Ordering::Relaxed)),
             subject: Subject::new(),
-            owner: RwLock::new(self.owner.read().clone()),
+            owner: self.owner.clone(),
         }
     }
 }

kernel/src/fs/inode_handle/mod.rs

@@ -266,13 +266,16 @@ impl InodeHandle_ {
     }
 
     fn release_range_locks(&self) {
+        if self.dentry.inode().extension().is_none() {
+            return;
+        }
+
         let range_lock = RangeLockItemBuilder::new()
             .type_(RangeLockType::Unlock)
             .range(FileRange::new(0, OFFSET_MAX).unwrap())
             .build()
             .unwrap();
-
+        self.unlock_range_lock(&range_lock);
-        self.unlock_range_lock(&range_lock)
     }
 
     fn unlock_range_lock(&self, lock: &RangeLockItem) {

kernel/src/process/clone.rs

@@ -394,16 +394,16 @@ fn clone_fs(
 }
 
 fn clone_files(
-    parent_file_table: &Arc<Mutex<FileTable>>,
+    parent_file_table: &Arc<SpinLock<FileTable>>,
     clone_flags: CloneFlags,
-) -> Arc<Mutex<FileTable>> {
+) -> Arc<SpinLock<FileTable>> {
     // if CLONE_FILES is set, the child and parent shares the same file table
     // Otherwise, the child will deep copy a new file table.
     // FIXME: the clone may not be deep copy.
     if clone_flags.contains(CloneFlags::CLONE_FILES) {
         parent_file_table.clone()
     } else {
-        Arc::new(Mutex::new(parent_file_table.lock().clone()))
+        Arc::new(SpinLock::new(parent_file_table.lock().clone()))
     }
 }
 

kernel/src/process/process/builder.rs

@@ -28,7 +28,7 @@ pub struct ProcessBuilder<'a> {
     argv: Option<Vec<CString>>,
     envp: Option<Vec<CString>>,
     process_vm: Option<ProcessVm>,
-    file_table: Option<Arc<Mutex<FileTable>>>,
+    file_table: Option<Arc<SpinLock<FileTable>>>,
     fs: Option<Arc<RwMutex<FsResolver>>>,
     umask: Option<Arc<RwLock<FileCreationMask>>>,
     resource_limits: Option<ResourceLimits>,
@@ -67,7 +67,7 @@ impl<'a> ProcessBuilder<'a> {
         self
     }
 
-    pub fn file_table(&mut self, file_table: Arc<Mutex<FileTable>>) -> &mut Self {
+    pub fn file_table(&mut self, file_table: Arc<SpinLock<FileTable>>) -> &mut Self {
         self.file_table = Some(file_table);
         self
     }
@@ -152,7 +152,7 @@ impl<'a> ProcessBuilder<'a> {
         let process_vm = process_vm.or_else(|| Some(ProcessVm::alloc())).unwrap();
 
         let file_table = file_table
-            .or_else(|| Some(Arc::new(Mutex::new(FileTable::new_with_stdio()))))
+            .or_else(|| Some(Arc::new(SpinLock::new(FileTable::new_with_stdio()))))
             .unwrap();
 
         let fs = fs

kernel/src/process/process/mod.rs

@@ -79,7 +79,7 @@ pub struct Process {
     /// Process group
     pub(super) process_group: Mutex<Weak<ProcessGroup>>,
     /// File table
-    file_table: Arc<Mutex<FileTable>>,
+    file_table: Arc<SpinLock<FileTable>>,
     /// FsResolver
     fs: Arc<RwMutex<FsResolver>>,
     /// umask
@@ -180,7 +180,7 @@ impl Process {
         process_vm: ProcessVm,
 
         fs: Arc<RwMutex<FsResolver>>,
-        file_table: Arc<Mutex<FileTable>>,
+        file_table: Arc<SpinLock<FileTable>>,
 
         umask: Arc<RwLock<FileCreationMask>>,
         resource_limits: ResourceLimits,
@@ -611,7 +611,7 @@ impl Process {
 
     // ************** File system ****************
 
-    pub fn file_table(&self) -> &Arc<Mutex<FileTable>> {
+    pub fn file_table(&self) -> &Arc<SpinLock<FileTable>> {
         &self.file_table
     }
 
@@ -724,7 +724,7 @@ mod test {
             String::new(),
             ProcessVm::alloc(),
             Arc::new(RwMutex::new(FsResolver::new())),
-            Arc::new(Mutex::new(FileTable::new())),
+            Arc::new(SpinLock::new(FileTable::new())),
             Arc::new(RwLock::new(FileCreationMask::default())),
             ResourceLimits::default(),
             Nice::default(),

kernel/src/syscall/chmod.rs

@@ -13,8 +13,10 @@ use crate::{
 pub fn sys_fchmod(fd: FileDesc, mode: u16, ctx: &Context) -> Result<SyscallReturn> {
     debug!("fd = {}, mode = 0o{:o}", fd, mode);
 
+    let file = {
         let file_table = ctx.process.file_table().lock();
-    let file = file_table.get_file(fd)?;
+        file_table.get_file(fd)?.clone()
+    };
     file.set_mode(InodeMode::from_bits_truncate(mode))?;
     Ok(SyscallReturn::Return(0))
 }

kernel/src/syscall/chown.rs

@@ -20,8 +20,10 @@ pub fn sys_fchown(fd: FileDesc, uid: i32, gid: i32, ctx: &Context) -> Result<Sys
         return Ok(SyscallReturn::Return(0));
     }
 
+    let file = {
         let file_table = ctx.process.file_table().lock();
-    let file = file_table.get_file(fd)?;
+        file_table.get_file(fd)?.clone()
+    };
     if let Some(uid) = uid {
         file.set_owner(uid)?;
     }

kernel/src/syscall/fcntl.rs

@@ -11,7 +11,7 @@ use crate::{
         },
     },
     prelude::*,
-    process::Pid,
+    process::{process_table, Pid},
 };
 
 pub fn sys_fcntl(fd: FileDesc, cmd: i32, arg: u64, ctx: &Context) -> Result<SyscallReturn> {
@@ -142,8 +142,6 @@ fn handle_getown(fd: FileDesc, ctx: &Context) -> Result<SyscallReturn> {
 }
 
 fn handle_setown(fd: FileDesc, arg: u64, ctx: &Context) -> Result<SyscallReturn> {
-    let file_table = ctx.process.file_table().lock();
-    let file_entry = file_table.get_entry(fd)?;
     // A process ID is specified as a positive value; a process group ID is specified as a negative value.
     let abs_arg = (arg as i32).unsigned_abs();
     if abs_arg > i32::MAX as u32 {
@@ -151,7 +149,19 @@ fn handle_setown(fd: FileDesc, arg: u64, ctx: &Context) -> Result<SyscallReturn>
     }
     let pid = Pid::try_from(abs_arg)
         .map_err(|_| Error::with_message(Errno::EINVAL, "invalid process (group) id"))?;
-    file_entry.set_owner(pid)?;
+
+    let owner_process = if pid == 0 {
+        None
+    } else {
+        Some(process_table::get_process(pid).ok_or(Error::with_message(
+            Errno::ESRCH,
+            "cannot set_owner with an invalid pid",
+        ))?)
+    };
+
+    let mut file_table = ctx.process.file_table().lock();
+    let file_entry = file_table.get_entry_mut(fd)?;
+    file_entry.set_owner(owner_process.as_ref())?;
     Ok(SyscallReturn::Return(0))
 }
 

kernel/src/syscall/ioctl.rs

@@ -15,8 +15,11 @@ pub fn sys_ioctl(fd: FileDesc, cmd: u32, arg: Vaddr, ctx: &Context) -> Result<Sy
         "fd = {}, ioctl_cmd = {:?}, arg = 0x{:x}",
         fd, ioctl_cmd, arg
     );
+
+    let file = {
         let file_table = ctx.process.file_table().lock();
-    let file = file_table.get_file(fd)?;
+        file_table.get_file(fd)?.clone()
+    };
     let res = match ioctl_cmd {
         IoctlCmd::FIONBIO => {
             let is_nonblocking = ctx.get_user_space().read_val::<i32>(arg)? != 0;

kernel/src/syscall/lseek.rs

@@ -8,6 +8,7 @@ use crate::{
 
 pub fn sys_lseek(fd: FileDesc, offset: isize, whence: u32, ctx: &Context) -> Result<SyscallReturn> {
     debug!("fd = {}, offset = {}, whence = {}", fd, offset, whence);
+
     let seek_from = match whence {
         0 => {
             if offset < 0 {
@@ -19,8 +20,11 @@ pub fn sys_lseek(fd: FileDesc, offset: isize, whence: u32, ctx: &Context) -> Res
         2 => SeekFrom::End(offset),
         _ => return_errno!(Errno::EINVAL),
     };
+    let file = {
         let file_table = ctx.process.file_table().lock();
-    let file = file_table.get_file(fd)?;
+        file_table.get_file(fd)?.clone()
+    };
+
     let offset = file.seek(seek_from)?;
     Ok(SyscallReturn::Return(offset as _))
 }

kernel/src/syscall/stat.rs

@@ -15,8 +15,11 @@ use crate::{
 pub fn sys_fstat(fd: FileDesc, stat_buf_ptr: Vaddr, ctx: &Context) -> Result<SyscallReturn> {
     debug!("fd = {}, stat_buf_addr = 0x{:x}", fd, stat_buf_ptr);
 
+    let file = {
         let file_table = ctx.process.file_table().lock();
-    let file = file_table.get_file(fd)?;
+        file_table.get_file(fd)?.clone()
+    };
+
     let stat = Stat::from(file.metadata());
     ctx.get_user_space().write_val(stat_buf_ptr, &stat)?;
     Ok(SyscallReturn::Return(0))

kernel/src/syscall/statfs.rs

@@ -29,13 +29,16 @@ pub fn sys_statfs(path_ptr: Vaddr, statfs_buf_ptr: Vaddr, ctx: &Context) -> Resu
 pub fn sys_fstatfs(fd: FileDesc, statfs_buf_ptr: Vaddr, ctx: &Context) -> Result<SyscallReturn> {
     debug!("fd = {}, statfs_buf_addr = 0x{:x}", fd, statfs_buf_ptr);
 
+    let fs = {
         let file_table = ctx.process.file_table().lock();
         let file = file_table.get_file(fd)?;
         let inode_handle = file
             .downcast_ref::<InodeHandle>()
             .ok_or(Error::with_message(Errno::EBADF, "not inode"))?;
-    let dentry = inode_handle.dentry();
+        inode_handle.dentry().fs()
-    let statfs = Statfs::from(dentry.fs().sb());
+    };
+
+    let statfs = Statfs::from(fs.sb());
     ctx.get_user_space().write_val(statfs_buf_ptr, &statfs)?;
     Ok(SyscallReturn::Return(0))
 }

kernel/src/syscall/truncate.rs

@@ -16,8 +16,11 @@ pub fn sys_ftruncate(fd: FileDesc, len: isize, ctx: &Context) -> Result<SyscallR
 
     check_length(len, ctx)?;
 
+    let file = {
         let file_table = ctx.process.file_table().lock();
-    let file = file_table.get_file(fd)?;
+        file_table.get_file(fd)?.clone()
+    };
+
     file.resize(len as usize)?;
     Ok(SyscallReturn::Return(0))
 }