From 2660c3e07a346b91e47b734a95ca011d221c68a9 Mon Sep 17 00:00:00 2001 From: 4lDO2 <4lDO2@protonmail.com> Date: Mon, 17 Jul 2023 15:17:55 +0200 Subject: [PATCH] WIP: Pin fmap grants that are borrowed too. This might not be the most ideal solution, since a GiB grant can be EBUSY blocked if a single page is used by an indefinitely-blocking UserScheme. But, the alternatives would impose lots of additional complexity, such as increasing the PageInfo size, adding more refcounting arrays to grants, etc. --- rmm | 2 +- src/context/memory.rs | 67 ++++++++++++++++++++++++++++++++++--------- src/scheme/proc.rs | 2 +- 3 files changed, 55 insertions(+), 16 deletions(-) diff --git a/rmm b/rmm index d9d892261e..8e22e69c94 160000 --- a/rmm +++ b/rmm @@ -1 +1 @@ -Subproject commit d9d892261eb567b7cb0fdc5b8c524b9978f37c38 +Subproject commit 8e22e69c940e61e3268c2e6adeffca9bc65d1cd5 diff --git a/src/context/memory.rs b/src/context/memory.rs index 2084b9453f..362bd5b1b7 100644 --- a/src/context/memory.rs +++ b/src/context/memory.rs @@ -482,6 +482,20 @@ impl UserGrants { .take_while(move |(base, info)| PageSpan::new(**base, info.page_count).intersects(span)) .map(|(base, info)| (*base, info)) } + // TODO: DEDUPLICATE CODE! + pub fn conflicts_mut(&mut self, span: PageSpan) -> impl Iterator + '_ { + let start = self.contains(span.base); + + // If there is a grant that contains the base page, start searching at the base of that + // grant, rather than the requested base here. + let start_span = start.map(|(base, info)| PageSpan::new(base, info.page_count)).unwrap_or(span); + + self + .inner + .range_mut(start_span.base..) + .take_while(move |(base, info)| PageSpan::new(**base, info.page_count).intersects(span)) + .map(|(base, info)| (*base, info)) + } /// Return a free region with the specified size // TODO: Alignment (x86_64: 4 KiB, 2 MiB, or 1 GiB). // TODO: Support finding grant close to a requested address? @@ -655,7 +669,7 @@ pub enum Provider { /// Since the address space is not tracked here, all nonpresent pages must be present before /// the fmap operation completes, unless MAP_LAZY is specified. They are tracked using /// PageInfo, or treated as PhysBorrowed if any frame lacks a PageInfo. - FmapBorrowed { file_ref: GrantFileRef }, + FmapBorrowed { file_ref: GrantFileRef, pin_refcount: usize }, } #[derive(Debug)] @@ -828,7 +842,7 @@ impl Grant { page_count: span.count, mapped: true, flags: new_flags, - provider: Provider::FmapBorrowed { file_ref }, + provider: Provider::FmapBorrowed { file_ref, pin_refcount: 0 }, } }) } @@ -839,7 +853,7 @@ impl Grant { /// pages that are unmaped or moved will not be made visible to the destination address space. pub fn borrow( src_address_space_lock: Arc>, - src_address_space: &AddrSpace, + src_address_space: &mut AddrSpace, src_base: Page, dst_base: Page, page_count: usize, @@ -861,7 +875,11 @@ impl Grant { let src_span = PageSpan::new(src_base, page_count); let mut prev_span = None; - for (src_grant_base, src_grant) in src_address_space.grants.conflicts(src_span) { + for (src_grant_base, src_grant) in src_address_space.grants.conflicts_mut(src_span) { + if let Provider::FmapBorrowed { ref mut pin_refcount, .. } = src_grant.provider { + *pin_refcount += 1; + } + let grant_span = PageSpan::new(src_grant_base, src_grant.page_count); let prev_span = prev_span.replace(grant_span); @@ -1030,6 +1048,20 @@ impl Grant { #[must_use = "will not unmap itself"] pub fn unmap(mut self, mapper: &mut PageMapper, mut flusher: impl Flusher) -> UnmapResult { assert!(self.info.mapped); + assert!(!self.info.is_pinned()); + + if let Provider::External { ref address_space, src_base, .. } = self.info.provider { + let mut guard = address_space.write(); + + for (_, grant) in guard.grants.conflicts_mut(PageSpan::new(src_base, self.info.page_count)) { + match grant.provider { + Provider::FmapBorrowed { ref mut pin_refcount, .. } => *pin_refcount = pin_refcount.checked_sub(1).expect("fmap pinning code is wrong"), + _ => continue, + } + } + + // TODO: Verify deadlock immunity + } for page in self.span().pages() { // Lazy mappings do not need to be unmapped. @@ -1070,7 +1102,7 @@ impl Grant { size: self.info.page_count * PAGE_SIZE, file_desc: match provider { Provider::Allocated { cow_file_ref } => cow_file_ref, - Provider::FmapBorrowed { file_ref } => Some(file_ref), + Provider::FmapBorrowed { file_ref, .. } => Some(file_ref), _ => None, } } @@ -1112,7 +1144,7 @@ impl Grant { Provider::Allocated { ref cow_file_ref } => Provider::Allocated { cow_file_ref: cow_file_ref.clone() }, Provider::AllocatedShared { .. } => Provider::AllocatedShared { is_pinned_userscheme_borrow: false }, Provider::PhysBorrowed { base } => Provider::PhysBorrowed { base: base.clone() }, - Provider::FmapBorrowed { ref file_ref } => Provider::FmapBorrowed { file_ref: file_ref.clone() } + Provider::FmapBorrowed { ref file_ref, .. } => Provider::FmapBorrowed { file_ref: file_ref.clone(), pin_refcount: 0 }, } }, }); @@ -1121,7 +1153,7 @@ impl Grant { match self.info.provider { Provider::PhysBorrowed { ref mut base } => *base = base.next_by(middle_page_offset), - Provider::FmapBorrowed { ref mut file_ref } | Provider::Allocated { cow_file_ref: Some(ref mut file_ref) } => file_ref.base_offset += middle_page_offset * PAGE_SIZE, + Provider::FmapBorrowed { ref mut file_ref, .. } | Provider::Allocated { cow_file_ref: Some(ref mut file_ref) } => file_ref.base_offset += middle_page_offset * PAGE_SIZE, Provider::Allocated { cow_file_ref: None } | Provider::AllocatedShared { .. } | Provider::External { .. } => (), } @@ -1146,10 +1178,13 @@ impl Grant { }, Provider::PhysBorrowed { base } => Provider::PhysBorrowed { base: base.next_by(this_span.count) }, - Provider::FmapBorrowed { ref file_ref } => Provider::FmapBorrowed { file_ref: GrantFileRef { - base_offset: file_ref.base_offset + this_span.count * PAGE_SIZE, - description: Arc::clone(&file_ref.description), - }}, + Provider::FmapBorrowed { ref file_ref, .. } => Provider::FmapBorrowed { + file_ref: GrantFileRef { + base_offset: file_ref.base_offset + this_span.count * PAGE_SIZE, + description: Arc::clone(&file_ref.description), + }, + pin_refcount: 0, + }, } }, }); @@ -1162,7 +1197,11 @@ impl Grant { } impl GrantInfo { pub fn is_pinned(&self) -> bool { - matches!(self.provider, Provider::External { is_pinned_userscheme_borrow: true, .. } | Provider::AllocatedShared { is_pinned_userscheme_borrow: true, .. }) + matches!(self.provider, + Provider::External { is_pinned_userscheme_borrow: true, .. } + | Provider::AllocatedShared { is_pinned_userscheme_borrow: true, .. } + | Provider::FmapBorrowed { pin_refcount: 1.., .. } + ) } pub fn unpin(&mut self) { if let Provider::External { ref mut is_pinned_userscheme_borrow, .. } | Provider::AllocatedShared { ref mut is_pinned_userscheme_borrow, .. } = self.provider { @@ -1236,7 +1275,7 @@ impl GrantInfo { pub fn file_ref(&self) -> Option<&GrantFileRef> { // TODO: This would be bad for PhysBorrowed head/tail buffers, but otherwise the physical // base address could be included in offset, for PhysBorrowed. - if let Provider::FmapBorrowed { ref file_ref } | Provider::Allocated { cow_file_ref: Some(ref file_ref) } = self.provider { + if let Provider::FmapBorrowed { ref file_ref, .. } | Provider::Allocated { cow_file_ref: Some(ref file_ref) } = self.provider { Some(file_ref) } else { None @@ -1588,7 +1627,7 @@ fn correct_inner<'l>(addr_space_lock: &'l Arc>, mut addr_space } // TODO: NonfatalInternalError if !MAP_LAZY and this page fault occurs. - Provider::FmapBorrowed { ref file_ref } => { + Provider::FmapBorrowed { ref file_ref, .. } => { let file_ref = file_ref.clone(); let flags = map_flags(grant_info.flags()); drop(addr_space_guard); diff --git a/src/scheme/proc.rs b/src/scheme/proc.rs index 74aff40cc7..984f74205a 100644 --- a/src/scheme/proc.rs +++ b/src/scheme/proc.rs @@ -694,7 +694,7 @@ impl KernelScheme for ProcScheme { let result_base = if consume { AddrSpace::r#move(&mut *dst_addr_space, &mut *src_addr_space, src_span, requested_dst_base, map.flags, &mut notify_files)? } else { - dst_addr_space.mmap(requested_dst_base, src_page_count, map.flags, &mut notify_files, |dst_page, flags, dst_mapper, flusher| Ok(Grant::borrow(Arc::clone(addrspace), &*src_addr_space, src_span.base, dst_page, src_span.count, flags, dst_mapper, flusher, true, true, false)?))? + dst_addr_space.mmap(requested_dst_base, src_page_count, map.flags, &mut notify_files, |dst_page, flags, dst_mapper, flusher| Ok(Grant::borrow(Arc::clone(addrspace), &mut *src_addr_space, src_span.base, dst_page, src_span.count, flags, dst_mapper, flusher, true, true, false)?))? }; handle_notify_files(notify_files);