diff --git a/src/context/file.rs b/src/context/file.rs index a274ef7..86ebf25 100644 --- a/src/context/file.rs +++ b/src/context/file.rs @@ -7,7 +7,7 @@ use crate::scheme::{self, SchemeNamespace, SchemeId}; use crate::syscall::error::{Result, Error, EBADF}; /// A file description -#[derive(Debug)] +#[derive(Clone, Copy, Debug)] pub struct FileDescription { /// The namespace the file was opened from (used for debugging) pub namespace: SchemeNamespace, diff --git a/src/context/memory.rs b/src/context/memory.rs index cfac011..209d908 100644 --- a/src/context/memory.rs +++ b/src/context/memory.rs @@ -35,6 +35,17 @@ pub fn page_flags(flags: MapFlags) -> PageFlags { //TODO: PROT_READ } +pub struct UnmapResult { + pub file_desc: Option, +} +impl Drop for UnmapResult { + fn drop(&mut self) { + if let Some(fd) = self.file_desc.take() { + let _ = fd.desc.close(); + } + } +} + #[derive(Debug, Default)] pub struct UserGrants { pub inner: BTreeSet, @@ -290,7 +301,15 @@ pub struct Grant { mapped: bool, owned: bool, //TODO: This is probably a very heavy way to keep track of fmap'd files, perhaps move to the context? - pub desc_opt: Option, + pub desc_opt: Option, +} +#[derive(Clone, Debug)] +pub struct GrantFileRef { + pub desc: FileDescriptor, + pub offset: usize, + // TODO: Can the flags maybe be stored together with the page flags. Should some flags be kept, + // and others discarded when re-fmapping on clone? + pub flags: MapFlags, } impl Grant { @@ -363,7 +382,7 @@ impl Grant { } } - pub fn map_inactive(src: VirtualAddress, dst: VirtualAddress, size: usize, flags: PageFlags, desc_opt: Option, inactive_table: &mut InactivePageTable) -> Grant { + pub fn map_inactive(src: VirtualAddress, dst: VirtualAddress, size: usize, flags: PageFlags, desc_opt: Option, inactive_table: &mut InactivePageTable) -> Grant { let active_table = unsafe { ActivePageTable::new(src.kind()) }; let mut inactive_mapper = inactive_table.mapper(); @@ -486,7 +505,7 @@ impl Grant { self.flags } - pub fn unmap(mut self) { + pub fn unmap(mut self) -> UnmapResult { assert!(self.mapped); let mut active_table = unsafe { ActivePageTable::new(self.start_address().kind()) }; @@ -507,16 +526,13 @@ impl Grant { flush_all.flush(); - if let Some(desc) = self.desc_opt.take() { - println!("Grant::unmap: close desc {:?}", desc); - //TODO: This imposes a large cost on unmapping, but that cost cannot be avoided without modifying fmap and funmap - let _ = desc.close(); - } - self.mapped = false; + + // TODO: This imposes a large cost on unmapping, but that cost cannot be avoided without modifying fmap and funmap + UnmapResult { file_desc: self.desc_opt.take() } } - pub fn unmap_inactive(mut self, new_table: &mut InactivePageTable) { + pub fn unmap_inactive(mut self, new_table: &mut InactivePageTable) -> UnmapResult { assert!(self.mapped); let start_page = Page::containing_address(self.start_address()); @@ -533,13 +549,10 @@ impl Grant { ipi(IpiKind::Tlb, IpiTarget::Other); - if let Some(desc) = self.desc_opt.take() { - println!("Grant::unmap_inactive: close desc {:?}", desc); - //TODO: This imposes a large cost on unmapping, but that cost cannot be avoided without modifying fmap and funmap - let _ = desc.close(); - } - self.mapped = false; + + // TODO: This imposes a large cost on unmapping, but that cost cannot be avoided without modifying fmap and funmap + UnmapResult { file_desc: self.desc_opt.take() } } /// Extract out a region into a separate grant. The return value is as diff --git a/src/scheme/user.rs b/src/scheme/user.rs index 6035bd5..5d7f9c6 100644 --- a/src/scheme/user.rs +++ b/src/scheme/user.rs @@ -8,7 +8,7 @@ use spin::{Mutex, RwLock}; use crate::context::{self, Context}; use crate::context::file::FileDescriptor; -use crate::context::memory::{DANGLING, page_flags, round_down_pages, Grant, Region}; +use crate::context::memory::{DANGLING, page_flags, round_down_pages, Grant, Region, GrantFileRef}; use crate::event; use crate::paging::{PAGE_SIZE, InactivePageTable, VirtualAddress}; use crate::scheme::{AtomicSchemeId, SchemeId}; @@ -123,7 +123,7 @@ impl UserInner { ).map(|addr| addr.data()) } - fn capture_inner(context_weak: &Weak>, dst_address: usize, address: usize, size: usize, flags: MapFlags, desc_opt: Option) + fn capture_inner(context_weak: &Weak>, dst_address: usize, address: usize, size: usize, flags: MapFlags, desc_opt: Option) -> Result { // TODO: More abstractions over grant creation! @@ -233,7 +233,8 @@ impl UserInner { if address % PAGE_SIZE > 0 { println!("scheme returned unaligned address, causing extra frame to be allocated"); } - let res = UserInner::capture_inner(&context_weak, map.address, address, map.size, map.flags, Some(desc)); + let file_ref = GrantFileRef { desc, offset: map.offset, flags: map.flags }; + let res = UserInner::capture_inner(&context_weak, map.address, address, map.size, map.flags, Some(file_ref)); if let Ok(grant_address) = res { if let Some(context_lock) = context_weak.upgrade() { let context = context_lock.read(); diff --git a/src/syscall/fs.rs b/src/syscall/fs.rs index 7052c0f..642a80a 100644 --- a/src/syscall/fs.rs +++ b/src/syscall/fs.rs @@ -489,11 +489,8 @@ pub fn funmap_old(virtual_address: usize) -> Result { } } - if let Some(desc) = desc_opt { - let scheme_id = { - let description = desc.description.read(); - description.scheme - }; + if let Some(file_ref) = desc_opt { + let scheme_id = { file_ref.desc.description.read().scheme }; let scheme = { let schemes = scheme::schemes(); @@ -502,7 +499,7 @@ pub fn funmap_old(virtual_address: usize) -> Result { }; let res = scheme.funmap_old(virtual_address); - let _ = desc.close(); + let _ = file_ref.desc.close(); res } else { @@ -555,11 +552,8 @@ pub fn funmap(virtual_address: usize, length: usize) -> Result { } } - for (desc, intersection) in notify_files { - let scheme_id = { - let description = desc.description.read(); - description.scheme - }; + for (file_ref, intersection) in notify_files { + let scheme_id = { file_ref.desc.description.read().scheme }; let scheme = { let schemes = scheme::schemes(); @@ -568,7 +562,7 @@ pub fn funmap(virtual_address: usize, length: usize) -> Result { }; let res = scheme.funmap(intersection.start_address().data(), intersection.size()); - let _ = desc.close(); + let _ = file_ref.desc.close(); res?; } diff --git a/src/syscall/process.rs b/src/syscall/process.rs index 3ef08d6..c246568 100644 --- a/src/syscall/process.rs +++ b/src/syscall/process.rs @@ -8,11 +8,11 @@ use alloc::{ use core::alloc::{GlobalAlloc, Layout}; use core::ops::DerefMut; use core::{intrinsics, mem, str}; -use spin::RwLock; +use spin::{RwLock, RwLockWriteGuard}; -use crate::context::file::FileDescriptor; -use crate::context::{ContextId, WaitpidKey}; +use crate::context::file::{FileDescription, FileDescriptor}; use crate::context::memory::{UserGrants, Region}; +use crate::context::{Context, ContextId, WaitpidKey}; use crate::context; #[cfg(not(feature="doc"))] use crate::elf::{self, program_header}; @@ -56,7 +56,7 @@ pub fn clone(flags: CloneFlags, stack_base: usize) -> Result { let mut image = vec![]; let mut stack_opt = None; let mut sigstack_opt = None; - let grants; + let mut grants; let name; let cwd; let files; @@ -266,24 +266,51 @@ pub fn clone(flags: CloneFlags, stack_base: usize) -> Result { // If not cloning virtual memory, use fmap to re-obtain every grant where possible if !flags.contains(CLONE_VM) { - let mut grants = grants.write(); + let grants = Arc::get_mut(&mut grants).ok_or(Error::new(EBUSY))?.get_mut(); + let old_grants = mem::take(&mut grants.inner); - let mut to_remove = BTreeSet::new(); + // TODO: Find some way to do this without having to allocate. - // TODO: Use drain_filter if possible + // TODO: Check that the current process is not allowed to serve any scheme this logic + // could interfere with. Deadlocks would otherwise seem inevitable. - for grant in grants.iter() { - let remove = false; - if let Some(ref _desc) = grant.desc_opt { - println!("todo: clone grant using fmap: {:?}", grant); - } - if remove { - to_remove.insert(Region::from(grant)); - } - } + for mut grant in old_grants.into_iter() { + let region = *grant.region(); + let address = region.start_address().data(); + let size = region.size(); - for region in to_remove { - grants.remove(®ion); + let new_grant = if let Some(ref mut file_ref) = grant.desc_opt.take() { + // TODO: Technically this is redundant as the grants are already secret_cloned. + // Maybe grants with fds can be excluded from that step? + grant.unmap(); + + let FileDescription { scheme, number, .. } = { *file_ref.desc.description.read() }; + let scheme_arc = match crate::scheme::schemes().get(scheme) { + Some(s) => Arc::clone(s), + None => continue, + }; + let map = crate::syscall::data::Map { + address, + size, + offset: file_ref.offset, + flags: file_ref.flags | MapFlags::MAP_FIXED_NOREPLACE, + }; + + let ptr = match scheme_arc.fmap(number, &map) { + Ok(new_range) => new_range as *mut u8, + Err(_) => continue, + }; + + // This will eventually be freed from the parent context after move_to is + // called. + context::contexts().current().ok_or(Error::new(ESRCH))? + .read().grants.write() + .take(&Region::new(VirtualAddress::new(ptr as usize), map.size)) + .ok_or(Error::new(EFAULT))? + } else { + grant + }; + grants.insert(new_grant); } } @@ -525,7 +552,7 @@ pub fn clone(flags: CloneFlags, stack_base: usize) -> Result { Ok(pid) } -fn empty(context: &mut context::Context, reaping: bool) { +fn empty<'lock>(context_lock: &'lock RwLock, mut context: RwLockWriteGuard<'lock, Context>, reaping: bool) -> RwLockWriteGuard<'lock, Context> { if reaping { // Memory should already be unmapped assert!(context.image.is_empty()); @@ -559,17 +586,26 @@ fn empty(context: &mut context::Context, reaping: bool) { let grants = mem::replace(&mut *grants_guard, UserGrants::default()); for grant in grants.inner.into_iter() { - if reaping { + let unmap_result = if reaping { log::error!("{}: {}: Grant should not exist: {:?}", context.id.into(), *context.name.read(), grant); let mut new_table = unsafe { InactivePageTable::from_address(context.arch.get_page_utable()) }; - grant.unmap_inactive(&mut new_table); + grant.unmap_inactive(&mut new_table) } else { - grant.unmap(); + grant.unmap() + }; + + if unmap_result.file_desc.is_some() { + drop(context); + + drop(unmap_result); + + context = context_lock.write(); } } } + context } struct ExecFile(FileHandle); @@ -607,7 +643,7 @@ fn fexec_noreturn( context.name = Arc::new(RwLock::new(name)); - empty(&mut context, false); + context = empty(&context_lock, context, false); context.grants.write().insert(phdr_grant); @@ -1091,7 +1127,7 @@ pub fn exit(status: usize) -> ! { let (vfork, children) = { let mut context = context_lock.write(); - empty(&mut context, false); + context = empty(&context_lock, context, false); let vfork = context.vfork; context.vfork = false; @@ -1439,7 +1475,7 @@ fn reap(pid: ContextId) -> Result { let context_lock = contexts.remove(pid).ok_or(Error::new(ESRCH))?; { let mut context = context_lock.write(); - empty(&mut context, true); + context = empty(&context_lock, context, true); } drop(context_lock);