diff --git a/kernel/libs/aster-util/src/slot_vec.rs b/kernel/libs/aster-util/src/slot_vec.rs index 787d238aa..4758190a2 100644 --- a/kernel/libs/aster-util/src/slot_vec.rs +++ b/kernel/libs/aster-util/src/slot_vec.rs @@ -52,6 +52,13 @@ impl SlotVec { 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. /// diff --git a/kernel/src/fs/file_table.rs b/kernel/src/fs/file_table.rs index ddc2488c4..45f316673 100644 --- a/kernel/src/fs/file_table.rs +++ b/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::() { + // 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::() { + // 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)> { 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, flags: AtomicU8, subject: Subject, - owner: RwLock>, + owner: Option, } 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 { - 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<()> { - if self.owner().is_some_and(|pid| pid == owner) { - return Ok(()); - } + pub fn set_owner(&mut self, owner: Option<&Arc>) -> Result<()> { + 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 _ = self.file.unregister_observer(&Arc::downgrade(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 { - None => None, - Some((pid, observer)) => { + let observer = OwnerObserver::new(self.file.clone(), Arc::downgrade(owner_process)); 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(), } } } diff --git a/kernel/src/fs/inode_handle/mod.rs b/kernel/src/fs/inode_handle/mod.rs index f4bd07e2f..10af3e261 100644 --- a/kernel/src/fs/inode_handle/mod.rs +++ b/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) { diff --git a/kernel/src/process/clone.rs b/kernel/src/process/clone.rs index f997ae422..29a23f07f 100644 --- a/kernel/src/process/clone.rs +++ b/kernel/src/process/clone.rs @@ -394,16 +394,16 @@ fn clone_fs( } fn clone_files( - parent_file_table: &Arc>, + parent_file_table: &Arc>, clone_flags: CloneFlags, -) -> Arc> { +) -> Arc> { // 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())) } } diff --git a/kernel/src/process/process/builder.rs b/kernel/src/process/process/builder.rs index 87504dde4..f645221e6 100644 --- a/kernel/src/process/process/builder.rs +++ b/kernel/src/process/process/builder.rs @@ -28,7 +28,7 @@ pub struct ProcessBuilder<'a> { argv: Option>, envp: Option>, process_vm: Option, - file_table: Option>>, + file_table: Option>>, fs: Option>>, umask: Option>>, resource_limits: Option, @@ -67,7 +67,7 @@ impl<'a> ProcessBuilder<'a> { self } - pub fn file_table(&mut self, file_table: Arc>) -> &mut Self { + pub fn file_table(&mut self, file_table: Arc>) -> &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 diff --git a/kernel/src/process/process/mod.rs b/kernel/src/process/process/mod.rs index 4e1ef7182..688e250cf 100644 --- a/kernel/src/process/process/mod.rs +++ b/kernel/src/process/process/mod.rs @@ -79,7 +79,7 @@ pub struct Process { /// Process group pub(super) process_group: Mutex>, /// File table - file_table: Arc>, + file_table: Arc>, /// FsResolver fs: Arc>, /// umask @@ -180,7 +180,7 @@ impl Process { process_vm: ProcessVm, fs: Arc>, - file_table: Arc>, + file_table: Arc>, umask: Arc>, resource_limits: ResourceLimits, @@ -611,7 +611,7 @@ impl Process { // ************** File system **************** - pub fn file_table(&self) -> &Arc> { + pub fn file_table(&self) -> &Arc> { &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(), diff --git a/kernel/src/syscall/chmod.rs b/kernel/src/syscall/chmod.rs index d8ede284f..caaf36378 100644 --- a/kernel/src/syscall/chmod.rs +++ b/kernel/src/syscall/chmod.rs @@ -13,8 +13,10 @@ use crate::{ pub fn sys_fchmod(fd: FileDesc, mode: u16, ctx: &Context) -> Result { debug!("fd = {}, mode = 0o{:o}", fd, mode); - let file_table = ctx.process.file_table().lock(); - let file = file_table.get_file(fd)?; + let file = { + let file_table = ctx.process.file_table().lock(); + file_table.get_file(fd)?.clone() + }; file.set_mode(InodeMode::from_bits_truncate(mode))?; Ok(SyscallReturn::Return(0)) } diff --git a/kernel/src/syscall/chown.rs b/kernel/src/syscall/chown.rs index 55ba1559f..a13510eb7 100644 --- a/kernel/src/syscall/chown.rs +++ b/kernel/src/syscall/chown.rs @@ -20,8 +20,10 @@ pub fn sys_fchown(fd: FileDesc, uid: i32, gid: i32, ctx: &Context) -> Result Result { @@ -142,8 +142,6 @@ fn handle_getown(fd: FileDesc, ctx: &Context) -> Result { } fn handle_setown(fd: FileDesc, arg: u64, ctx: &Context) -> Result { - 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 } 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)) } diff --git a/kernel/src/syscall/ioctl.rs b/kernel/src/syscall/ioctl.rs index 15ae801ac..50a0139b5 100644 --- a/kernel/src/syscall/ioctl.rs +++ b/kernel/src/syscall/ioctl.rs @@ -15,8 +15,11 @@ pub fn sys_ioctl(fd: FileDesc, cmd: u32, arg: Vaddr, ctx: &Context) -> Result { let is_nonblocking = ctx.get_user_space().read_val::(arg)? != 0; diff --git a/kernel/src/syscall/lseek.rs b/kernel/src/syscall/lseek.rs index 6819a9c21..6ad9c0919 100644 --- a/kernel/src/syscall/lseek.rs +++ b/kernel/src/syscall/lseek.rs @@ -8,6 +8,7 @@ use crate::{ pub fn sys_lseek(fd: FileDesc, offset: isize, whence: u32, ctx: &Context) -> Result { 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_table = ctx.process.file_table().lock(); - let file = file_table.get_file(fd)?; + let file = { + let file_table = ctx.process.file_table().lock(); + file_table.get_file(fd)?.clone() + }; + let offset = file.seek(seek_from)?; Ok(SyscallReturn::Return(offset as _)) } diff --git a/kernel/src/syscall/stat.rs b/kernel/src/syscall/stat.rs index 89fc63e08..39239faa2 100644 --- a/kernel/src/syscall/stat.rs +++ b/kernel/src/syscall/stat.rs @@ -15,8 +15,11 @@ use crate::{ pub fn sys_fstat(fd: FileDesc, stat_buf_ptr: Vaddr, ctx: &Context) -> Result { debug!("fd = {}, stat_buf_addr = 0x{:x}", fd, stat_buf_ptr); - let file_table = ctx.process.file_table().lock(); - let file = file_table.get_file(fd)?; + let file = { + let file_table = ctx.process.file_table().lock(); + 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)) diff --git a/kernel/src/syscall/statfs.rs b/kernel/src/syscall/statfs.rs index 81b9bb4cc..bc60fbca0 100644 --- a/kernel/src/syscall/statfs.rs +++ b/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 { debug!("fd = {}, statfs_buf_addr = 0x{:x}", fd, statfs_buf_ptr); - let file_table = ctx.process.file_table().lock(); - let file = file_table.get_file(fd)?; - let inode_handle = file - .downcast_ref::() - .ok_or(Error::with_message(Errno::EBADF, "not inode"))?; - let dentry = inode_handle.dentry(); - let statfs = Statfs::from(dentry.fs().sb()); + let fs = { + let file_table = ctx.process.file_table().lock(); + let file = file_table.get_file(fd)?; + let inode_handle = file + .downcast_ref::() + .ok_or(Error::with_message(Errno::EBADF, "not inode"))?; + inode_handle.dentry().fs() + }; + + let statfs = Statfs::from(fs.sb()); ctx.get_user_space().write_val(statfs_buf_ptr, &statfs)?; Ok(SyscallReturn::Return(0)) } diff --git a/kernel/src/syscall/truncate.rs b/kernel/src/syscall/truncate.rs index 7d79fbc63..11e7628ed 100644 --- a/kernel/src/syscall/truncate.rs +++ b/kernel/src/syscall/truncate.rs @@ -16,8 +16,11 @@ pub fn sys_ftruncate(fd: FileDesc, len: isize, ctx: &Context) -> Result