diff --git a/os/StarryOS/kernel/src/entry.rs b/os/StarryOS/kernel/src/entry.rs index 46aeb7a88c..97dd9631fd 100644 --- a/os/StarryOS/kernel/src/entry.rs +++ b/os/StarryOS/kernel/src/entry.rs @@ -30,6 +30,8 @@ pub fn init(args: &[String], envs: &[String]) { pseudofs::mount_all().expect("Failed to mount pseudofs"); spawn_alarm_task(); + ax_alloc::register_page_reclaim_fn(ax_fs::page_cache_reclaim); + let loc = FS_CONTEXT .lock() .resolve(&args[0]) diff --git a/os/StarryOS/kernel/src/mm/aspace/backend/file.rs b/os/StarryOS/kernel/src/mm/aspace/backend/file.rs index 36df4b6274..526e5966b7 100644 --- a/os/StarryOS/kernel/src/mm/aspace/backend/file.rs +++ b/os/StarryOS/kernel/src/mm/aspace/backend/file.rs @@ -50,21 +50,24 @@ impl FileBackendInner { let aspace = Arc::downgrade(aspace); let handle = self.cache.add_evict_listener({ let this = Arc::downgrade(self); - move |pn, _page| { + move |pn, _page| -> bool { let Some(this) = this.upgrade() else { - return; + // Backend dropped — no mappings remain, safe to free. + return true; }; let Some(aspace) = aspace.upgrade() else { // The address space has been dropped, nothing to do. - return; + return true; }; let Some(mut aspace) = aspace.try_lock() else { - // This can happen during the populate process, when new pages - // are being populated and old pages are being evicted. In this - // case, we delegate the unmapping to the populate process. - return; + // Cannot acquire AddrSpace lock (contention with populate + // or another thread). Return false so the reclaim path + // puts the page back into the cache instead of freeing it + // — dropping the page here would leave a dangling PTE. + return false; }; this.on_evict(pn, &mut aspace); + true } }); self.handle.store(handle, Ordering::Release); diff --git a/os/arceos/modules/axalloc/src/buddy_slab.rs b/os/arceos/modules/axalloc/src/buddy_slab.rs index 6af3242956..e1f8d5392b 100644 --- a/os/arceos/modules/axalloc/src/buddy_slab.rs +++ b/os/arceos/modules/axalloc/src/buddy_slab.rs @@ -165,10 +165,11 @@ impl GlobalAllocator { /// Allocate arbitrary number of bytes. Returns the left bound of the /// allocated region. pub fn alloc(&self, layout: Layout) -> AllocResult> { - let result = { - let inner = self.inner.lock(); - inner.alloc(layout).map_err(crate::AllocError::from) - }; + let result = self + .inner + .lock() + .alloc(layout) + .map_err(crate::AllocError::from); if result.is_ok() { self.usages.lock().alloc(UsageKind::RustHeap, layout.size()); } @@ -177,10 +178,9 @@ impl GlobalAllocator { /// Gives back the allocated region to the byte allocator. pub fn dealloc(&self, pos: NonNull, layout: Layout) { - { - let inner = self.inner.lock(); - unsafe { inner.dealloc(pos, layout) }; - } + // Lock order: inner then usages (consistent with alloc/alloc_pages). + // Guards are temporary — locks are never held simultaneously. + unsafe { self.inner.lock().dealloc(pos, layout) }; self.usages .lock() .dealloc(UsageKind::RustHeap, layout.size()); @@ -193,16 +193,30 @@ impl GlobalAllocator { alignment: usize, kind: UsageKind, ) -> AllocResult { - let result = { - let inner = self.inner.lock(); - inner - .alloc_pages(num_pages, alignment) - .map_err(crate::AllocError::from) - }; - if result.is_ok() { - self.usages.lock().alloc(kind, num_pages * PAGE_SIZE); + let mut result = self.inner.lock().alloc_pages(num_pages, alignment); + if result.is_err() { + for _ in 0..4 { + // Reclaim num_pages (at least 16 to build free-pool headroom). + // page_cache_reclaim doubles this target internally. + // NOTE: for very large contiguous requests, reclaimed pages + // may be too fragmented to satisfy the allocation even when + // the target is met. Consider geometric growth across retries + // if this becomes a problem in practice. + let reclaimed = crate::try_page_reclaim(num_pages.max(16)); + // Retry allocation regardless of whether reclaim ran; + // concurrent reclaim may have freed pages. + result = self.inner.lock().alloc_pages(num_pages, alignment); + if result.is_ok() { + break; + } + if reclaimed == 0 { + break; + } + } } - result + let addr = result.map_err(crate::AllocError::from)?; + self.usages.lock().alloc(kind, num_pages * PAGE_SIZE); + Ok(addr) } /// Allocates contiguous low-memory pages (physical address < 4 GiB). @@ -212,16 +226,22 @@ impl GlobalAllocator { alignment: usize, kind: UsageKind, ) -> AllocResult { - let result = { - let inner = self.inner.lock(); - inner - .alloc_pages_lowmem(num_pages, alignment) - .map_err(crate::AllocError::from) - }; - if result.is_ok() { - self.usages.lock().alloc(kind, num_pages * PAGE_SIZE); + let mut result = self.inner.lock().alloc_pages_lowmem(num_pages, alignment); + if result.is_err() { + for _ in 0..4 { + let reclaimed = crate::try_page_reclaim(num_pages.max(16)); + result = self.inner.lock().alloc_pages_lowmem(num_pages, alignment); + if result.is_ok() { + break; + } + if reclaimed == 0 { + break; + } + } } - result + let addr = result.map_err(crate::AllocError::from)?; + self.usages.lock().alloc(kind, num_pages * PAGE_SIZE); + Ok(addr) } /// Allocates contiguous pages starting from the given address. @@ -237,10 +257,9 @@ impl GlobalAllocator { /// Gives back the allocated pages starts from `pos` to the page allocator. pub fn dealloc_pages(&self, pos: usize, num_pages: usize, kind: UsageKind) { - { - let inner = self.inner.lock(); - inner.dealloc_pages(pos, num_pages); - } + // Lock order: inner then usages (consistent with alloc_pages). + // Guards are temporary — locks are never held simultaneously. + self.inner.lock().dealloc_pages(pos, num_pages); self.usages.lock().dealloc(kind, num_pages * PAGE_SIZE); } diff --git a/os/arceos/modules/axalloc/src/lib.rs b/os/arceos/modules/axalloc/src/lib.rs index 661fda1200..0722252ef3 100644 --- a/os/arceos/modules/axalloc/src/lib.rs +++ b/os/arceos/modules/axalloc/src/lib.rs @@ -19,6 +19,29 @@ use strum::{IntoStaticStr, VariantArray}; const PAGE_SIZE: usize = 0x1000; +/// A function that tries to reclaim physical pages (e.g. by evicting +/// clean file-backed page cache pages). Returns the number of pages freed. +pub type PageReclaimFn = fn(num_pages: usize) -> usize; + +static PAGE_RECLAIM_FN: ax_kspin::SpinNoIrq> = ax_kspin::SpinNoIrq::new(None); + +/// Register a callback that the allocator will invoke when a page allocation +/// cannot be satisfied. +pub fn register_page_reclaim_fn(f: PageReclaimFn) { + *PAGE_RECLAIM_FN.lock() = Some(f); +} + +/// Try to reclaim physical pages by invoking the registered callback. +/// Returns the number of pages actually freed. +/// +/// The `SpinNoIrq` guard is released before calling into the reclaim +/// function so that the reclaim path (and any evict listeners it +/// triggers) runs with interrupts enabled. +pub fn try_page_reclaim(num_pages: usize) -> usize { + let reclaim_fn = { *PAGE_RECLAIM_FN.lock() }; + reclaim_fn.map_or(0, |f| f(num_pages)) +} + mod page; pub use page::GlobalPage; diff --git a/os/arceos/modules/axfs-ng/src/highlevel/file.rs b/os/arceos/modules/axfs-ng/src/highlevel/file.rs index 7706edb063..bd0a373f8f 100644 --- a/os/arceos/modules/axfs-ng/src/highlevel/file.rs +++ b/os/arceos/modules/axfs-ng/src/highlevel/file.rs @@ -5,7 +5,7 @@ use alloc::{ }; use core::{ num::NonZeroUsize, - sync::atomic::{AtomicU8, Ordering}, + sync::atomic::{AtomicBool, AtomicU8, Ordering}, task::Context, }; @@ -465,7 +465,12 @@ impl Drop for PageCache { } } -type EvictListenerFn = Box; +/// Eviction listener callback. Returns `true` if the listener +/// successfully invalidated all mappings for the evicted page. +/// When `false` is returned, the caller must **not** drop the page +/// (doing so would free the physical frame while PTEs still reference +/// it); instead the page is returned to the cache for a later retry. +type EvictListenerFn = Box bool + Send + Sync>; struct EvictListener { listener: EvictListenerFn, @@ -493,6 +498,129 @@ impl CachedFileShared { evict_listeners: Mutex::new(LinkedList::default()), } } + + /// Scan the LRU and evict up to `max` clean pages. + /// + /// Two-phase eviction: + /// 1. Under `page_cache` lock: identify clean pages, pop them from the + /// cache, and move them into a local buffer. + /// 2. Outside `page_cache` lock: invoke evict listeners. If all + /// listeners confirm the PTE unmap, the page is dropped (freeing its + /// physical frame). If any listener cannot unmap (e.g., AddrSpace + /// lock contention), the page is re-inserted into the cache to + /// prevent use-after-free. + /// + /// Returns the number of pages successfully evicted. + /// + /// # Lock ordering + /// + /// `page_cache` is released before acquiring `evict_listeners`, + /// eliminating the latent deadlock risk that exists when listeners + /// are called under the cache lock. + fn try_evict_clean_pages(&self, max: usize) -> usize { + let limit = max.min(256); + + // Phase 1: Pop clean pages from LRU under page_cache lock. + // Two-pass: first collect page numbers (borrows cache immutably), + // then pop by number (borrows cache mutably). + let mut pending: Vec<(u32, PageCache)> = Vec::new(); + { + let Some(mut cache) = self.page_cache.try_lock() else { + return 0; + }; + let mut to_pop = [0u32; 256]; + let mut cnt = 0; + for (&pn, page) in cache.iter().rev() { + if !page.dirty && cnt < limit { + to_pop[cnt] = pn; + cnt += 1; + } + } + for &pn in to_pop[..cnt].iter() { + if let Some(page) = cache.pop(&pn) { + pending.push((pn, page)); + } + } + } // page_cache lock released + + // Phase 2: Invoke listeners outside page_cache lock. + let mut evicted = 0; + for (pn, page) in pending.into_iter() { + let mut all_ok = true; + for listener in self.evict_listeners.lock().iter() { + if !(listener.listener)(pn, &page) { + all_ok = false; + break; + } + } + if all_ok { + // All listeners confirmed unmap — drop page (frees physical frame). + drop(page); + evicted += 1; + } else { + // Listener could not unmap (e.g., AddrSpace lock contention). + // Re-insert page into cache to avoid freeing a physical frame + // that still has live PTEs pointing to it. + let mut cache = self.page_cache.lock(); + cache.put(pn, page); + } + } + evicted + } +} + +struct ReclaimGuard; + +impl Drop for ReclaimGuard { + fn drop(&mut self) { + RECLAIM_IN_PROGRESS.store(false, Ordering::Release); + } +} + +static GLOBAL_CACHED_FILES: spin::RwLock>> = + spin::RwLock::new(alloc::vec::Vec::new()); + +static RECLAIM_IN_PROGRESS: AtomicBool = AtomicBool::new(false); + +pub fn page_cache_reclaim(num_pages: usize) -> usize { + if RECLAIM_IN_PROGRESS.swap(true, Ordering::AcqRel) { + return 0; + } + let _guard = ReclaimGuard; + + let mut reclaimed = 0; + let target = num_pages.max(16) * 2; + let mut file_count = 0; + + if let Some(guard) = GLOBAL_CACHED_FILES.try_read() { + for weak in guard.iter() { + if let Some(file) = weak.upgrade() { + let freed = file.try_evict_clean_pages(target - reclaimed); + reclaimed += freed; + file_count += 1; + if reclaimed >= target { + break; + } + } + } + } else { + return 0; + } + + if reclaimed > 0 { + debug!( + "page_cache_reclaim: evicted {} clean pages across {} files", + reclaimed, file_count + ); + } + + reclaimed +} + +fn register_cached_file(file: &Arc) { + let mut guard = GLOBAL_CACHED_FILES.write(); + guard.retain(|w| w.upgrade().is_some()); + guard.push(Arc::downgrade(file)); } /// A file handle with an LRU page cache for buffered I/O. @@ -536,22 +664,29 @@ impl CachedFile { let in_memory = location.filesystem().name() == "tmpfs"; let mut guard = location.user_data(); - let shared = if let Some(shared) = guard.get::().and_then(|it| it.get()) { - shared - } else { - let (shared, user_data) = if in_memory { - let shared = Arc::new(CachedFileShared::new_unbounded()); - (shared.clone(), FileUserData::Strong(shared)) + let (shared, is_new) = + if let Some(shared) = guard.get::().and_then(|it| it.get()) { + (shared, false) } else { - let shared = Arc::new(CachedFileShared::new()); - let user_data = FileUserData::Weak(Arc::downgrade(&shared)); - (shared, user_data) + let (shared, user_data) = if in_memory { + let shared = Arc::new(CachedFileShared::new_unbounded()); + (shared.clone(), FileUserData::Strong(shared)) + } else { + let shared = Arc::new(CachedFileShared::new()); + let user_data = FileUserData::Weak(Arc::downgrade(&shared)); + (shared, user_data) + }; + guard.insert(user_data); + (shared, true) }; - guard.insert(user_data); - shared - }; drop(guard); + // In-memory files (tmpfs) have no backing store, so evicting clean + // pages would lose data. Only register disk-backed files for reclaim. + if is_new && !in_memory { + register_cached_file(&shared); + } + Self { inner: location, shared, @@ -576,7 +711,7 @@ impl CachedFile { /// [`remove_evict_listener`](Self::remove_evict_listener). pub fn add_evict_listener(&self, listener: F) -> usize where - F: Fn(u32, &PageCache) + Send + Sync + 'static, + F: Fn(u32, &PageCache) -> bool + Send + Sync + 'static, { let pointer = Box::new(EvictListener { listener: Box::new(listener), @@ -599,7 +734,12 @@ impl CachedFile { fn evict_cache(&self, file: &FileNode, pn: u32, page: &mut PageCache) -> VfsResult<()> { for listener in self.shared.evict_listeners.lock().iter() { - (listener.listener)(pn, page); + // In the LRU-eviction path (triggered by page_or_insert), the + // populate process holds AddrSpace and handles the unmap via + // PopulateCallback. The listener's return value is irrelevant + // here — if try_lock fails, the caller is the populate process + // itself and it will unmap the old page after inserting the new one. + let _ = (listener.listener)(pn, page); } if page.dirty { let page_start = pn as u64 * PAGE_SIZE as u64; diff --git a/os/arceos/modules/axsync/src/mutex.rs b/os/arceos/modules/axsync/src/mutex.rs index 6b94e8e1df..2afdc50892 100644 --- a/os/arceos/modules/axsync/src/mutex.rs +++ b/os/arceos/modules/axsync/src/mutex.rs @@ -7,9 +7,9 @@ use ax_task::{WaitQueue, current, might_sleep}; /// A [`lock_api::RawMutex`] implementation. /// /// When the mutex is locked, the current task will block and be put into the -/// wait queue. When the mutex is unlocked, ownership is released before waking -/// at most one waiting task; the woken task then acquires the mutex with the -/// normal compare-exchange path. +/// wait queue. When the mutex is unlocked, ownership is handed off to at most +/// one task waiting on the queue; if no tasks are waiting, the mutex simply +/// becomes unlocked. pub struct RawMutex { wq: WaitQueue, owner_id: AtomicU64, @@ -129,8 +129,16 @@ unsafe impl lock_api::RawMutex for RawMutex { ); #[cfg(feature = "lockdep")] crate::lockdep::release(self); - self.owner_id.store(0, Ordering::Release); - self.wq.notify_one(true); + // Wake one waiting thread. The callback receives the waiter's ID. + // When the wait queue is empty, notify_one_with calls the callback + // with id=0, which clears owner_id via the swap below. This is + // what makes is_locked_inner() return false — the unlock handoff + // DEPENDS on notify_one_with always invoking the callback, even + // for empty queues. If that contract changes, add an explicit + // owner_id.store(0, Ordering::Release) fallback here. + self.wq.notify_one_with(true, |id: u64| { + self.owner_id.swap(id, Ordering::Release); + }); } #[inline(always)] @@ -166,10 +174,7 @@ impl RawMutex { owner_id, current_id, "Thread({current_id}) tried to acquire mutex it already owns.", ); - // Wait until the lock is released. The woken waiter - // competes through the normal CAS path, avoiding a state - // where the owner id names a task that has not returned a - // guard yet. + // Wait until someone hands off lock to me or lock is released self.wq .wait_until(|| self.is_owner(current_id) || !self.is_locked_inner()); // This check is necessary: some newcomers may race with a wakened one. @@ -184,7 +189,7 @@ impl RawMutex { #[inline(always)] #[track_caller] #[cfg(feature = "lockdep")] - fn lock_nested(&self, subclass: crate::lockdep::LockSubclass) { + fn lock_nested(&self, subclass: LockSubclass) { might_sleep(); let current_id = current().id().as_u64(); @@ -197,7 +202,8 @@ impl RawMutex { #[track_caller] #[cfg(not(feature = "lockdep"))] fn try_lock_plain(&self) -> bool { - might_sleep(); + // try_lock is a single atomic CAS — it never blocks or sleeps, + // so it is safe to call from atomic context (cf. Linux mutex_trylock). let current_id = current().id().as_u64(); self.try_lock_after_prepare(current_id) } @@ -210,8 +216,9 @@ impl RawMutex { #[inline(always)] #[track_caller] #[cfg(feature = "lockdep")] - fn try_lock_nested(&self, subclass: crate::lockdep::LockSubclass) -> bool { - might_sleep(); + fn try_lock_nested(&self, subclass: LockSubclass) -> bool { + // try_lock is a single atomic CAS — it never blocks or sleeps, + // so it is safe to call from atomic context. let current_id = current().id().as_u64(); let lockdep = crate::lockdep::LockdepAcquire::prepare_nested(self, true, subclass);