diff --git a/src/context/context.rs b/src/context/context.rs index f1de4ab1f6..f256b27943 100644 --- a/src/context/context.rs +++ b/src/context/context.rs @@ -355,10 +355,12 @@ impl Context { this_percpu.current_addrsp.borrow().as_ref().unwrap(), prev_addrsp )); - prev_addrsp - .acquire_read() - .used_by - .atomic_clear(this_percpu.cpu_id); + + // See [`crate::percpu::switch_arch_hook`]. + prev_addrsp.used_by.atomic_clear(this_percpu.cpu_id); + + core::sync::atomic::fence(Ordering::SeqCst); + this_percpu.maybe_handle_tlb_shootdown(); } let _old_addrsp = core::mem::replace( @@ -368,9 +370,8 @@ impl Context { match addr_space { Some(ref new) => { + new.used_by.atomic_set(this_percpu.cpu_id); let new_addrsp = new.acquire_read(); - new_addrsp.used_by.atomic_set(this_percpu.cpu_id); - unsafe { new_addrsp.table.utable.make_current(); } diff --git a/src/context/memory.rs b/src/context/memory.rs index 45eb69fc4e..5d62b71713 100644 --- a/src/context/memory.rs +++ b/src/context/memory.rs @@ -82,14 +82,16 @@ impl UnmapResult { #[derive(Debug)] pub struct AddrSpaceWrapper { - inner: RwLock, + pub inner: RwLock, pub tlb_ack: AtomicU32, + pub used_by: LogicalCpuSet, } impl AddrSpaceWrapper { pub fn new() -> Result> { Ok(Arc::new(Self { inner: RwLock::new(AddrSpace::new()?), tlb_ack: AtomicU32::new(0), + used_by: LogicalCpuSet::empty(), })) } pub fn acquire_read(&self) -> RwLockReadGuard<'_, AddrSpace> { @@ -155,8 +157,9 @@ impl AddrSpaceWrapper { let new = Arc::get_mut(&mut new_arc).expect("expected new address space Arc not to be aliased"); + let _this_mapper = &mut guard.table.utable; let this_mapper = &mut guard.table.utable; - let mut this_flusher = Flusher::with_cpu_set(&mut guard.used_by, &self.tlb_ack); + let mut this_flusher = Flusher::with_cpu_set(&self.used_by, &self.tlb_ack); for (grant_base, grant_info) in guard.grants.iter() { let new_grant = match grant_info.provider { @@ -241,7 +244,7 @@ impl AddrSpaceWrapper { let guard = &mut *guard; let mapper = &mut guard.table.utable; - let mut flusher = Flusher::with_cpu_set(&mut guard.used_by, &self.tlb_ack); + let mut flusher = Flusher::with_cpu_set(&self.used_by, &self.tlb_ack); // TODO: Remove allocation (might require BTreeMap::set_key or interior mutability). let regions = guard @@ -305,7 +308,7 @@ impl AddrSpaceWrapper { let mut guard = self.acquire_write(); let guard = &mut *guard; - let mut flusher = Flusher::with_cpu_set(&mut guard.used_by, &self.tlb_ack); + let mut flusher = Flusher::with_cpu_set(&self.used_by, &self.tlb_ack); AddrSpace::munmap_inner( &mut guard.grants, &mut guard.table.utable, @@ -330,12 +333,12 @@ impl AddrSpaceWrapper { let mut src_flusher; let mut src_opt = match src_opt { Some((aw, a)) => { - src_flusher = Flusher::with_cpu_set(&mut a.used_by, &aw.tlb_ack); + src_flusher = Flusher::with_cpu_set(&aw.used_by, &aw.tlb_ack); Some((&mut a.grants, &mut a.table.utable, &mut src_flusher)) } None => None, }; - let mut dst_flusher = Flusher::with_cpu_set(&mut dst.used_by, &dst_lock.tlb_ack); + let mut dst_flusher = Flusher::with_cpu_set(&dst_lock.used_by, &dst_lock.tlb_ack); let dst_base = match requested_dst_base { Some(base) if new_flags.contains(MapFlags::MAP_FIXED_NOREPLACE) => { @@ -711,7 +714,7 @@ impl AddrSpace { selected_span.base, page_flags(flags), &mut self.table.utable, - &mut Flusher::with_cpu_set(&mut self.used_by, &dst_lock.tlb_ack), + &mut Flusher::with_cpu_set(&dst_lock.used_by, &dst_lock.tlb_ack), )?; self.grants.insert(grant); @@ -1329,7 +1332,7 @@ impl Grant { new_flags: PageFlags, file_ref: GrantFileRef, src: Option>, - lock: &AddrSpaceWrapper, + _lock: &AddrSpaceWrapper, mapper: &mut PageMapper, flusher: &mut Flusher, token: &mut CleanLockToken, @@ -1338,7 +1341,8 @@ impl Grant { let mut guard = src.addr_space_guard; let mut src_addrspace = &mut *guard; let mut src_flusher_state = - Flusher::with_cpu_set(&mut src_addrspace.used_by, &lock.tlb_ack).detach(); + Flusher::with_cpu_set(&src.addr_space_lock.used_by, &src.addr_space_lock.tlb_ack) + .detach(); for dst_page in span.pages() { let src_page = src.src_base.next_by(dst_page.offset_from(span.base)); @@ -2401,7 +2405,7 @@ fn correct_inner<'l>( token: &mut CleanLockToken, ) -> Result<(Frame, PageFlush, RwLockWriteGuard<'l, AddrSpace>), PfError> { let mut addr_space = &mut *addr_space_guard; - let mut flusher = Flusher::with_cpu_set(&mut addr_space.used_by, &addr_space_lock.tlb_ack); + let mut flusher = Flusher::with_cpu_set(&addr_space_lock.used_by, &addr_space_lock.tlb_ack); let Some((grant_base, grant_info)) = addr_space.grants.contains(faulting_page) else { debug!("Lacks grant"); @@ -2651,7 +2655,7 @@ fn correct_inner<'l>( addr_space_guard = addr_space_lock.acquire_write(); addr_space = &mut *addr_space_guard; - flusher = Flusher::with_cpu_set(&mut addr_space.used_by, &addr_space_lock.tlb_ack); + flusher = Flusher::with_cpu_set(&addr_space_lock.used_by, &addr_space_lock.tlb_ack); info!("Got frame {:?} from external fmap", frame); @@ -2749,6 +2753,7 @@ fn handle_free_action(base: Frame, phys_contiguous_count: Option) } } } +#[derive(Debug)] struct FlusherState<'addrsp> { // TODO: what capacity? pagequeue: ArrayVec, @@ -2757,6 +2762,7 @@ struct FlusherState<'addrsp> { ackword: &'addrsp AtomicU32, } +#[derive(Debug)] enum PageQueueEntry { Free { base: Frame, @@ -2768,12 +2774,14 @@ enum PageQueueEntry { }, } -pub struct Flusher<'guard, 'addrsp> { - active_cpus: &'guard mut LogicalCpuSet, +#[derive(Debug)] +pub struct Flusher<'a, 'addrsp> { + active_cpus: &'a LogicalCpuSet, state: FlusherState<'addrsp>, } -impl<'guard, 'addrsp> Flusher<'guard, 'addrsp> { - fn with_cpu_set(set: &'guard mut LogicalCpuSet, ackword: &'addrsp AtomicU32) -> Self { + +impl<'a, 'addrsp> Flusher<'a, 'addrsp> { + fn with_cpu_set(set: &'a LogicalCpuSet, ackword: &'addrsp AtomicU32) -> Self { Self { active_cpus: set, state: FlusherState { @@ -2811,13 +2819,18 @@ impl<'guard, 'addrsp> Flusher<'guard, 'addrsp> { let current_cpu_id = crate::cpu_id(); - for cpu_id in self.active_cpus.iter_mut() { + for cpu_id in self.active_cpus.iter() { if cpu_id == current_cpu_id { continue; } crate::percpu::shootdown_tlb_ipi(Some(cpu_id)); - affected_cpu_count += 1; + + core::sync::atomic::fence(Ordering::SeqCst); + + if self.active_cpus.contains(cpu_id) { + affected_cpu_count += 1; + } } if self.active_cpus.contains(current_cpu_id) { @@ -2872,6 +2885,7 @@ impl Drop for Flusher<'_, '_> { } } bitflags::bitflags! { + #[derive(Debug)] pub struct TlbShootdownActions: usize { // Delay the deallocation of one or more contiguous frames. const FREE = 1; diff --git a/src/cpu_set.rs b/src/cpu_set.rs index 65871e0bc5..9e7a5a9b32 100644 --- a/src/cpu_set.rs +++ b/src/cpu_set.rs @@ -63,17 +63,21 @@ impl LogicalCpuSet { pub const fn empty() -> Self { Self([const { AtomicUsize::new(0) }; SET_WORDS]) } + pub const fn all() -> Self { Self([const { AtomicUsize::new(!0) }; SET_WORDS]) } - pub fn contains(&mut self, id: LogicalCpuId) -> bool { + + pub fn contains(&self, id: LogicalCpuId) -> bool { let (word, bit) = parts(id); - *self.0[word].get_mut() & (1 << bit) != 0 + self.0[word].load(Ordering::Acquire) & (1 << bit) != 0 } + pub fn atomic_set(&self, id: LogicalCpuId) { let (word, bit) = parts(id); let _ = self.0[word].fetch_or(1 << bit, Ordering::Release); } + pub fn atomic_clear(&self, id: LogicalCpuId) { let (word, bit) = parts(id); let _ = self.0[word].fetch_and(!(1 << bit), Ordering::Release); @@ -82,15 +86,15 @@ impl LogicalCpuSet { pub fn override_from(&mut self, raw: &RawMask) { self.0 = raw.map(AtomicUsize::new); } + pub fn to_raw(&self) -> RawMask { self.0.each_ref().map(|w| w.load(Ordering::Acquire)) } - pub fn iter_mut(&mut self) -> impl Iterator + '_ { - // TODO: Will this be optimized away? - self.0.iter_mut().enumerate().flat_map(move |(i, w)| { + pub fn iter(&self) -> impl Iterator + '_ { + self.0.iter().enumerate().flat_map(move |(i, w)| { (0..usize::BITS).filter_map(move |b| { - if *w.get_mut() & (1 << b) != 0 { + if w.load(Ordering::Acquire) & (1 << b) != 0 { Some(LogicalCpuId::new(i as u32 * usize::BITS + b)) } else { None diff --git a/src/percpu.rs b/src/percpu.rs index 7453772b6f..fa6adf64ba 100644 --- a/src/percpu.rs +++ b/src/percpu.rs @@ -142,10 +142,19 @@ pub unsafe fn switch_arch_hook() { return; } if let Some(prev_addrsp) = &*cur_addrsp { - prev_addrsp - .acquire_read() - .used_by - .atomic_clear(percpu.cpu_id); + prev_addrsp.used_by.atomic_clear(percpu.cpu_id); + + // See [`Flusher::flush`]. + // + // Without the fence, `wants_tlb_shootdown` check *may* happen + // before the CPU is removed from the `used_by` set. Hence, if a + // shootdown request arises *after* the check and *before* removing + // the CPU from the set, it would be missed and the CPU who + // requested the shootdown would spin forever since the request was + // never ACKed. + core::sync::atomic::fence(Ordering::SeqCst); + + percpu.maybe_handle_tlb_shootdown(); } drop(cur_addrsp); @@ -156,9 +165,9 @@ pub unsafe fn switch_arch_hook() { match &*percpu.current_addrsp.borrow() { Some(next_addrsp) => { + next_addrsp.used_by.atomic_set(percpu.cpu_id); let next = next_addrsp.acquire_read(); - next.used_by.atomic_set(percpu.cpu_id); next.table.utable.make_current(); } _ => {