diff --git a/src/context/memory.rs b/src/context/memory.rs index b84275b009..2084b9453f 100644 --- a/src/context/memory.rs +++ b/src/context/memory.rs @@ -4,6 +4,7 @@ use syscall::GrantFlags; use core::cmp; use core::fmt::Debug; use core::num::NonZeroUsize; +use core::sync::atomic::Ordering; use spin::{RwLock, RwLockWriteGuard, Once, RwLockUpgradableGuard}; use syscall::{ flag::MapFlags, @@ -678,19 +679,19 @@ impl Grant { // TODO: is_pinned pub fn allocated_shared_one_page(frame: Frame, page: Page, flags: PageFlags, mapper: &mut PageMapper, mut flusher: impl Flusher, is_pinned: bool) -> Result { - match get_page_info(frame).expect("needs page info").lock() { - ref mut info => { - // This may not necessarily hold, as even pinned memory can remain shared (e.g. - // proc: borrow), but it would probably be possible to forbid borrowing memory - // there as well. - // - //assert_eq!(info.refcount(), RefCount::One); + let info = get_page_info(frame).expect("needs page info"); - // Semantically, the page will be shared between the "context struct" and whatever - // else. - info.add_ref(RefKind::Shared).expect("must be possible if previously Zero"); - } - } + // TODO: + // + // This may not necessarily hold, as even pinned memory can remain shared (e.g. + // proc: borrow), but it would probably be possible to forbid borrowing memory + // there as well. + // + // assert_eq!(info.refcount(), RefCount::One); + + // Semantically, the page will be shared between the "context struct" and whatever + // else. + info.add_ref(RefKind::Shared).expect("must be possible if previously Zero"); unsafe { flusher.consume(mapper.map_phys(page.start_address(), frame.start_address(), flags).ok_or(Error::new(ENOMEM))?); @@ -807,11 +808,9 @@ impl Grant { }; let frame = if let Some(page_info) = get_page_info(frame) { - let mut guard = page_info.lock(); - - match guard.add_ref(RefKind::Shared) { + match page_info.add_ref(RefKind::Shared) { Ok(()) => frame, - Err(AddRefError::CowToShared) => cow(frame, &mut *guard, RefKind::Shared).map_err(|_| Error::new(ENOMEM))?, + Err(AddRefError::CowToShared) => cow(frame, page_info, RefKind::Shared).map_err(|_| Error::new(ENOMEM))?, Err(AddRefError::SharedToCow) => unreachable!(), Err(AddRefError::RcOverflow) => return Err(Error::new(ENOMEM)), } @@ -947,13 +946,12 @@ impl Grant { let src_frame = { let src_page_info = get_page_info(src_frame).expect("allocated page was not present in the global page array"); - let mut guard = src_page_info.lock(); - match guard.add_ref(rk) { + match src_page_info.add_ref(rk) { Ok(()) => src_frame, Err(AddRefError::RcOverflow) => return Err(Enomem), Err(AddRefError::CowToShared) => { - let new_frame = cow(src_frame, &mut *guard, rk).map_err(|_| Enomem)?; + let new_frame = cow(src_frame, src_page_info, rk).map_err(|_| Enomem)?; // TODO: Flusher unsafe { @@ -965,7 +963,7 @@ impl Grant { // Cannot be shared and CoW simultaneously. Err(AddRefError::SharedToCow) => { // TODO: Copy in place, or use a zeroed page? - cow(src_frame, &mut *guard, rk).map_err(|_| Enomem)? + cow(src_frame, src_page_info, rk).map_err(|_| Enomem)? }, } }; @@ -1052,8 +1050,7 @@ impl Grant { // madvise(range, PHYSICALLY_CONTIGUOUS). if use_info && let Some(info) = get_page_info(frame) { - let mut guard = info.lock(); - if guard.remove_ref() == RefCount::Zero { + if info.remove_ref() == RefCount::Zero { deallocate_frames(frame, 1); }; } else { @@ -1376,7 +1373,7 @@ pub enum PfError { RecursionLimitExceeded, } -fn cow(old_frame: Frame, old_info: &mut PageInfo, initial_ref_kind: RefKind) -> Result { +fn cow(old_frame: Frame, old_info: &PageInfo, initial_ref_kind: RefKind) -> Result { assert_ne!(old_info.refcount(), RefCount::Zero); if old_info.refcount() == RefCount::One { @@ -1399,8 +1396,8 @@ fn cow(old_frame: Frame, old_info: &mut PageInfo, initial_ref_kind: RefKind) -> pub fn init_frame(init_rc: RefCount) -> Result { let new_frame = crate::memory::allocate_frames(1).ok_or(PfError::Oom)?; let page_info = get_page_info(new_frame).expect("all allocated frames need an associated page info"); - let mut guard = page_info.lock(); - guard.refcount = init_rc.to_raw(); + assert_eq!(page_info.refcount(), RefCount::Zero); + page_info.refcount.store(init_rc.to_raw(), Ordering::Relaxed); Ok(new_frame) } @@ -1490,12 +1487,11 @@ fn correct_inner<'l>(addr_space_lock: &'l Arc>, mut addr_space Provider::Allocated { .. } | Provider::AllocatedShared { .. } if access == AccessMode::Write => { match faulting_pageinfo_opt { Some((_, None)) => unreachable!("allocated page needs frame to be valid"), - Some((frame, Some(info_lock))) => { - let mut guard = info_lock.lock(); - if guard.allows_writable() { + Some((frame, Some(info))) => { + if info.allows_writable() { frame } else { - cow(frame, &mut *guard, RefKind::Cow)? + cow(frame, info, RefKind::Cow)? } }, _ => map_zeroed(&mut addr_space.table.utable, faulting_page, grant_flags, true)?, @@ -1508,12 +1504,11 @@ fn correct_inner<'l>(addr_space_lock: &'l Arc>, mut addr_space // TODO: Can this match arm even be reached? In other words, can the TLB cache // remember that pages are not present? - Some((frame, Some(page_info_lock))) => { - let guard = page_info_lock.lock(); + Some((frame, Some(page_info))) => { // Keep in mind that allow_writable must always be true if this code is reached // for AllocatedShared, since shared pages cannot be mapped lazily (without // using AddrSpace backrefs). - allow_writable = guard.allows_writable(); + allow_writable = page_info.allows_writable(); frame } @@ -1561,14 +1556,13 @@ fn correct_inner<'l>(addr_space_lock: &'l Arc>, mut addr_space frame }; - let info_lock = get_page_info(src_frame).expect("all allocated frames need a PageInfo"); - let mut info = info_lock.lock(); + let info = get_page_info(src_frame).expect("all allocated frames need a PageInfo"); match info.add_ref(RefKind::Shared) { Ok(()) => src_frame, Err(AddRefError::RcOverflow) => return Err(PfError::Oom), Err(AddRefError::CowToShared) => { - let new_frame = cow(src_frame, &mut *info, RefKind::Shared)?; + let new_frame = cow(src_frame, info, RefKind::Shared)?; let mut guard = foreign_address_space.write(); diff --git a/src/memory/mod.rs b/src/memory/mod.rs index 3e826ab10f..fa120700e1 100644 --- a/src/memory/mod.rs +++ b/src/memory/mod.rs @@ -3,6 +3,7 @@ use core::cmp; use core::num::NonZeroUsize; +use core::sync::atomic::{AtomicUsize, Ordering}; use crate::arch::rmm::LockedAllocator; use crate::common::try_box_slice_new; @@ -16,7 +17,7 @@ use rmm::{ FrameAllocator, FrameCount, }; -use spin::{RwLock, Mutex}; +use spin::RwLock; use crate::syscall::flag::{PartialAllocStrategy, PhysallocFlags}; use crate::syscall::error::{ENOMEM, Error}; @@ -183,7 +184,11 @@ impl RaiiFrame { impl Drop for RaiiFrame { fn drop(&mut self) { - get_page_info(self.inner).expect("RaiiFrame lacking PageInfo").lock().refcount = 0; + let info = get_page_info(self.inner).expect("RaiiFrame lacking PageInfo"); + + if info.refcount.load(Ordering::Relaxed) == 0 { + info.refcount.store(0, Ordering::Release); + } crate::memory::deallocate_frames(self.inner, 1); } } @@ -205,7 +210,7 @@ pub struct PageInfo { /// /// Bits 0..=N-1 are used for the actual reference count, whereas bit N-1 indicates the page is /// shared if set, and CoW if unset. The flag is not meaningful when the refcount is 0 or 1. - pub refcount: usize, + pub refcount: AtomicUsize, // TODO: AtomicFlags? pub flags: FrameFlags, @@ -229,7 +234,7 @@ pub static SECTIONS: RwLock> = RwLock::new(Vec::new()); pub struct Section { base: Frame, - frames: Box<[Mutex]>, + frames: Box<[PageInfo]>, } pub const MAX_SECTION_SIZE_BITS: u32 = 27; @@ -251,7 +256,7 @@ pub fn init_mm() { sections.push(Box::leak(Box::new(Section { base, // TODO: zeroed rather than PageInfo::new()? - frames: try_box_slice_new(|| Mutex::new(PageInfo::new()), section_page_count).expect("failed to allocate pages array"), + frames: try_box_slice_new(PageInfo::new, section_page_count).expect("failed to allocate pages array"), })) as &'static Section); pages_left -= section_page_count; @@ -273,41 +278,38 @@ pub enum AddRefError { impl PageInfo { pub fn new() -> Self { Self { - refcount: 0, + refcount: AtomicUsize::new(0), flags: FrameFlags::NONE, } } - pub fn add_ref(&mut self, kind: RefKind) -> Result<(), AddRefError> { + pub fn add_ref(&self, kind: RefKind) -> Result<(), AddRefError> { let old = self.refcount(); + match (self.refcount(), kind) { - (RefCount::Zero, _) => self.refcount = 1, - (RefCount::One, RefKind::Cow) => self.refcount = 2, - (RefCount::One, RefKind::Shared) => self.refcount = 2 | RC_SHARED_NOT_COW, - (RefCount::Cow(prev), RefKind::Cow) => self.refcount = prev.get().checked_add(1).ok_or(AddRefError::RcOverflow)?, - (RefCount::Shared(prev), RefKind::Shared) => self.refcount = prev.get().checked_add(1).ok_or(AddRefError::RcOverflow)? | RC_SHARED_NOT_COW, + (RefCount::Zero, _) => self.refcount.store(1, Ordering::Relaxed), + (RefCount::One, RefKind::Cow) => self.refcount.store(2, Ordering::Relaxed), + (RefCount::One, RefKind::Shared) => self.refcount.store(2 | RC_SHARED_NOT_COW, Ordering::Relaxed), + (RefCount::Cow(prev), RefKind::Cow) | (RefCount::Shared(prev), RefKind::Shared) => { + self.refcount.fetch_add(1, Ordering::Relaxed); + } (RefCount::Cow(prev), RefKind::Shared) => return Err(AddRefError::CowToShared), (RefCount::Shared(prev), RefKind::Cow) => return Err(AddRefError::SharedToCow), } Ok(()) } #[must_use = "must deallocate if refcount reaches zero"] - pub fn remove_ref(&mut self) -> RefCount { + pub fn remove_ref(&self) -> RefCount { let old = self.refcount(); - match self.refcount() { + RefCount::from_raw(match self.refcount() { RefCount::Zero => panic!("refcount was already zero when calling remove_ref!"), - RefCount::One => self.refcount = 0, - RefCount::Cow(prev) => self.refcount -= 1, - RefCount::Shared(prev) => { - self.refcount = prev.get() - 1; + RefCount::One => { + self.refcount.store(0, Ordering::Relaxed); - if self.refcount > 1 { - self.refcount |= RC_SHARED_NOT_COW; - } + 0 } - } - - self.refcount() + RefCount::Cow(_) | RefCount::Shared(_) => self.refcount.fetch_sub(1, Ordering::Relaxed) - 1, + }) } pub fn allows_writable(&self) -> bool { match self.refcount() { @@ -318,17 +320,9 @@ impl PageInfo { } pub fn refcount(&self) -> RefCount { - if let Some(nz_refcount) = NonZeroUsize::new(self.refcount) { - if self.refcount == 1 { - RefCount::One - } else if self.refcount & RC_SHARED_NOT_COW == RC_SHARED_NOT_COW { - RefCount::Shared(NonZeroUsize::new(self.refcount & !RC_SHARED_NOT_COW).unwrap()) - } else { - RefCount::Cow(nz_refcount) - } - } else { - RefCount::Zero - } + let refcount = self.refcount.load(Ordering::Relaxed); + + RefCount::from_raw(refcount) } } #[derive(Clone, Copy, Debug)] @@ -345,6 +339,21 @@ pub enum RefCount { Cow(NonZeroUsize), } impl RefCount { + pub fn from_raw(raw: usize) -> Self { + let refcount = raw & !RC_SHARED_NOT_COW; + + if let Some(nz_refcount) = NonZeroUsize::new(refcount) { + if refcount == 1 { + RefCount::One + } else if raw & RC_SHARED_NOT_COW == RC_SHARED_NOT_COW { + RefCount::Shared(nz_refcount) + } else { + RefCount::Cow(nz_refcount) + } + } else { + RefCount::Zero + } + } pub fn to_raw(self) -> usize { match self { Self::Zero => 0, @@ -354,7 +363,7 @@ impl RefCount { } } } -pub fn get_page_info(frame: Frame) -> Option<&'static Mutex> { +pub fn get_page_info(frame: Frame) -> Option<&'static PageInfo> { let sections = SECTIONS.read(); let idx = sections