Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions os/StarryOS/kernel/src/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand Down
17 changes: 10 additions & 7 deletions os/StarryOS/kernel/src/mm/aspace/backend/file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
79 changes: 49 additions & 30 deletions os/arceos/modules/axalloc/src/buddy_slab.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<NonNull<u8>> {
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());
}
Expand All @@ -177,10 +178,9 @@ impl GlobalAllocator {

/// Gives back the allocated region to the byte allocator.
pub fn dealloc(&self, pos: NonNull<u8>, 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());
Expand All @@ -193,16 +193,30 @@ impl GlobalAllocator {
alignment: usize,
kind: UsageKind,
) -> AllocResult<usize> {
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).
Expand All @@ -212,16 +226,22 @@ impl GlobalAllocator {
alignment: usize,
kind: UsageKind,
) -> AllocResult<usize> {
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.
Expand All @@ -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);
}

Expand Down
23 changes: 23 additions & 0 deletions os/arceos/modules/axalloc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Option<PageReclaimFn>> = 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;

Expand Down
158 changes: 142 additions & 16 deletions os/arceos/modules/axfs-ng/src/highlevel/file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use alloc::{
};
use core::{
num::NonZeroUsize,
sync::atomic::{AtomicU8, Ordering},
sync::atomic::{AtomicBool, AtomicU8, Ordering},
task::Context,
};

Expand Down Expand Up @@ -465,7 +465,12 @@ impl Drop for PageCache {
}
}

type EvictListenerFn = Box<dyn Fn(u32, &PageCache) + Send + Sync>;
/// 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<dyn Fn(u32, &PageCache) -> bool + Send + Sync>;

struct EvictListener {
listener: EvictListenerFn,
Expand Down Expand Up @@ -493,6 +498,115 @@ impl CachedFileShared {
evict_listeners: Mutex::new(LinkedList::default()),
}
}

/// Scan the LRU and evict up to `max` clean pages.
///
/// Collects non-dirty pages from the LRU cache (least-recently-used
/// first), pops them, notifies all registered evict listeners, and
/// drops the pages (freeing their backing physical memory).
/// Returns the number of pages evicted.
///
/// # Lock ordering: `page_cache` -> `evict_listeners`
///
/// This function holds the `page_cache` lock while acquiring
/// `evict_listeners`. Listeners must never acquire `page_cache`
/// (or any lock taken before `evict_listeners`), or a deadlock will
/// occur. The current callers (address-space backend page-table
/// invalidation) do not touch the file's page cache, so this
/// ordering is safe in practice.
///
/// TODO: restructure to drop `page_cache` lock before acquiring
/// `evict_listeners` by collecting evicted pages into a stack buffer.
/// This would eliminate the latent deadlock risk for future listeners.
fn try_evict_clean_pages(&self, max: usize) -> usize {
let Some(mut cache) = self.page_cache.try_lock() else {
return 0;
};
let mut to_evict = [0u32; 256];
let limit = max.min(256);
let mut cnt = 0;
for (&pn, page) in cache.iter().rev() {
if !page.dirty && cnt < limit {
to_evict[cnt] = pn;
cnt += 1;
}
}
let mut evicted = 0;
for &pn in to_evict[..cnt].iter() {
if let Some(page) = cache.pop(&pn) {
let mut all_ok = true;
for listener in self.evict_listeners.lock().iter() {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里不能在 page_cache 锁内直接调用 eviction listener 并随后让 page drop。当前 Starry 的 listener 在 FileBackendInner::register_listener() 中会先 aspace.try_lock(),拿不到地址空间锁时直接返回;这在普通 page_or_insert() eviction 里还能由 populate callback 延后处理,但 reclaim 路径没有保存 (pn, page) 也没有重试/回滚,函数返回后 clean page 的物理页会被释放,用户页表里可能仍然保留指向该物理页的 PTE。也就是说,这个“避免死锁”的 try_lock 分支会把死锁风险转成悬空映射/use-after-free。

建议把 reclaim 改成两阶段:在持有 page_cache 时只挑选并移出候选页,释放 page_cache 后调用 listener;并且 listener/回调需要能反馈是否完成了所有映射失效。若任何 listener 因锁竞争无法完成 unmap,就不能 drop 该 page(要放回 cache,或像 populate 的 PopulateCallback 一样延后到持有 AddrSpace 时再释放)。

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

复核 cb849d4b1 后,这个问题仍然存在。新提交把 try_evict_clean_pages() 的返回值改成实际 pop 的数量,并补了 TODO,但当前代码仍然在 page_cache 锁内调用 listener,Starry 侧 listener 在 aspace.try_lock() 失败时仍然直接返回;reclaim 路径随后会 drop page 释放物理页,没有 PopulateCallback 那样的保活/延后 unmap 机制。

这里需要先让 reclaim eviction 能确认所有相关映射已经失效;如果 listener 因锁竞争或错误无法完成 unmap,就不能释放这个 page,需要放回 cache 或保留到后续能在持有 AddrSpace 时安全 unmap。

if !(listener.listener)(pn, &page) {
all_ok = false;
}
}
if all_ok {
// All listeners confirmed unmap — safe to drop (free physical frame).
evicted += 1;
} else {
// At least one listener could not unmap (e.g., AddrSpace lock
// contention). Put the page back to avoid freeing a physical
// frame that still has live PTEs pointing to it.
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<alloc::vec::Vec<Weak<CachedFileShared>>> =
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<CachedFileShared>) {
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.
Expand Down Expand Up @@ -536,22 +650,29 @@ impl CachedFile {
let in_memory = location.filesystem().name() == "tmpfs";

let mut guard = location.user_data();
let shared = if let Some(shared) = guard.get::<FileUserData>().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::<FileUserData>().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,
Expand All @@ -576,7 +697,7 @@ impl CachedFile {
/// [`remove_evict_listener`](Self::remove_evict_listener).
pub fn add_evict_listener<F>(&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),
Expand All @@ -599,7 +720,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;
Expand Down
Loading