Skip to content
Closed
Show file tree
Hide file tree
Changes from all 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 @@ -22,6 +22,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);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ 注册本身正确,但由于 try_page_reclaim() 未被分配器调用(见 axalloc/lib.rs 的 inline comment),此处注册的回调实际上是死代码。需先完成分配器后端集成。

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ 无测试且无可重现的验证方法

PR 没有添加任何测试,也没有提供可重现的非板级验证命令。预期行为描述(「cargo build inside StarryOS no longer OOMs」)需要物理板级环境才能验证。建议:

  1. 添加单元测试覆盖 page_cache_reclaim() 基本逻辑(注册文件→写入页面→触发回收→验证干净页面被驱逐)
  2. 或提供可在 QEMU 中运行的验证命令


let loc = FS_CONTEXT
.lock()
.resolve(&args[0])
Expand Down
48 changes: 32 additions & 16 deletions os/arceos/modules/axalloc/src/buddy_slab.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,15 +191,24 @@ impl GlobalAllocator {
alignment: usize,
kind: UsageKind,
) -> AllocResult<usize> {
let result = self
.inner
.lock()
.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 {
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 @@ -209,15 +218,22 @@ impl GlobalAllocator {
alignment: usize,
kind: UsageKind,
) -> AllocResult<usize> {
let result = self
.inner
.lock()
.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 Down
21 changes: 16 additions & 5 deletions os/arceos/modules/axalloc/src/default_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,11 +181,22 @@ impl GlobalAllocator {
alignment: usize,
kind: UsageKind,
) -> AllocResult<usize> {
let addr = self
.palloc
.lock()
.alloc_pages(num_pages, alignment)
.map_err(crate::AllocError::from)?;
let mut result = self.palloc.lock().alloc_pages(num_pages, alignment);
if result.is_err() {
for _ in 0..4 {
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.palloc.lock().alloc_pages(num_pages, alignment);
if result.is_ok() {
break;
}
if reclaimed == 0 {
break;
}
}
}
let addr = result.map_err(crate::AllocError::from)?;
if !matches!(kind, UsageKind::RustHeap) {
self.usages.lock().alloc(kind, num_pages * PAGE_SIZE);
}
Expand Down
27 changes: 27 additions & 0 deletions os/arceos/modules/axalloc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,33 @@ 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);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nice:fnCopy 类型,直接解引用拷贝函数指针后释放锁,避免了持 SpinNoIrq 调用回调的问题。


/// 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);
}
Comment on lines +29 to +31

/// 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. Listeners registered by the
/// address-space backend acquire blocking locks that call `might_sleep()`,
/// which panics if interrupts are disabled.
pub fn try_page_reclaim(num_pages: usize) -> usize {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔴 try_page_reclaim() 从未被分配器后端调用——回收机制是死代码

commit message 声称「default_impl 和 buddy_slab 后端在分配失败时调用 try_page_reclaim() 最多 4 次,每次成功回收后重试」,但实际上 default_impl.rsbuddy_slab.rs 的分配路径中 没有任何地方调用 try_page_reclaim()

当前代码只是定义了 API 并在 entry.rs 中注册了回调,但分配失败路径没有集成——回调永远不会被触发。

修复方向:在 default_impl.rsalloc_pages() 中,当 palloc.lock().alloc_pages(...) 返回 NoMemory 时,循环调用 super::try_page_reclaim() 并重试分配。

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

阻塞:try_page_reclaim 是死代码——从未被调用。

在整个代码库中搜索,try_page_reclaim 仅在 axalloc/src/lib.rs 中定义,但分配器的两个后端实现(buddy_slab.rs 和 default_impl.rs)以及 page.rs 的分配失败路径都没有调用它。这意味着当分配器无法满足页面请求时,回收机制不会被触发。

PR body 声称 "Page allocation failures trigger reclaim + retry" 和 "cargo build inside StarryOS no longer OOMs",但当前实现无法实现这两个目标。

建议修复:在 alloc_pages 等分配失败路径中,加入对 try_page_reclaim 的调用并重试分配。例如在 GlobalPage::alloc() 或分配器后端的 alloc_pages 方法中,当返回 NoMemory 时调用 try_page_reclaim(num_pages) 并重试。

// Copy out the function pointer under the lock, then release the
// SpinNoIrq guard before invoking the callback.
let reclaim_fn = { *PAGE_RECLAIM_FN.lock() };
reclaim_fn.map_or(0, |f| f(num_pages))
}

mod page;
pub use page::GlobalPage;

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

Expand Down Expand Up @@ -392,6 +392,105 @@ 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.
///
/// # Context
///
/// When called from the page-reclaim path, the reclaim callback is
/// invoked with interrupts enabled: `try_page_reclaim` releases its
/// `SpinNoIrq` guard before invoking the callback, so `might_sleep()`
/// in the blocking `evict_listeners.lock()` is safe.
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;
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

此处 evict_listeners.lock() 在持有 page_cache(try_lock)锁期间调用。如果某个监听器回调反过来尝试获取同文件的 page_cache 锁,将产生死锁。实际中监听器通常是页表刷新回调,不会回访文件的 page_cache,风险很低。但建议在注释中明确说明锁序约束:page_cache → evict_listeners,避免后续修改引入死锁。

for &pn in to_evict[..cnt].iter() {
if let Some(page) = cache.pop(&pn) {
for listener in self.evict_listeners.lock().iter() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

阻塞性问题:回收路径中调用阻塞锁

evict_listeners 仍然是 axsync::Mutex(阻塞锁,内部调用 might_sleep())。回收路径通过 try_page_reclaimpage_cache_reclaimtry_evict_clean_pages 调用链执行,此时处于原子上下文(IRQ 禁用,在分配器 SpinNoIrq 锁内)。

如果另一个 CPU 正持有 evict_listeners 锁,当前 CPU 调用 .lock() 将尝试阻塞,触发 might_sleep() 恐慌。

commit 4a9e883 的提交信息说「Evict listeners registered by the address-space backend can call blocking functions that sleep」,但这个理由只说明了监听器回调内部的行为,并未解决回收路径在原子上下文中调用阻塞锁的问题。

建议修复方案

  • 方案 A(推荐):在 try_evict_clean_pages 中使用 spin::Mutex 作为 evict_listeners 类型。LinkedList 内部使用 LinkedListAtomicLink(原子链表),spin::Mutex 不会阻塞,从原子上下文调用安全。
  • 方案 B:若监听器回调确实需要阻塞锁(如获取页表锁),则在 try_evict_clean_pages 中先在 spin 锁下快照需要通知的 (pn, page) 对,释放 page_cache 锁后再通知监听器。但这样会增加复杂度。
  • 方案 C(最小改动):在 try_evict_clean_pages 中对 evict_listeners 使用 try_lock(),如果锁被占用则跳过通知(非阻塞降级)。

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This concern is not actually a blocking issue. Analysis:

  1. IRQ state: try_page_reclaim releases its SpinNoIrq guard before invoking the callback (see the comment in os/arceos/modules/axalloc/src/lib.rs: "The SpinNoIrq guard is released before calling into the reclaim function"). Therefore the page_cache_reclaim -> try_evict_clean_pages -> evict_listeners.lock() call chain executes in a normal thread context with interrupts enabled. Calling might_sleep() is safe.

  2. Design intent: Commit 4a9e88341 deliberately changed evict_listeners from spin::Mutex back to axsync::Mutex because the evict listener callbacks registered by the address-space backend may call blocking functions (e.g., acquiring page table locks). Using a blocking mutex is the correct design choice.

  3. Documentation added: The latest commit adds doc comments to try_evict_clean_pages explicitly documenting the lock ordering constraint (page_cache -> evict_listeners) and the reclaim context (interrupts enabled), to prevent future maintainers from being confused.

  4. Validation: All clippy checks (including lockdep feature) and cargo fmt pass.

(listener.listener)(pn, &page);
}
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

锁序文档写得很好,明确了 page_cache → evict_listeners 的约束和调用上下文。

cnt
}
}

struct ReclaimGuard;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

最新 commit 将 evict_listenersspin::Mutex 改回了阻塞 Mutex。在此处 .lock() 会调用 might_sleep(),若从中断上下文(IRQ 禁用)进入回收路径,会导致 panic。

建议改用 spin::Mutex 或在此处使用 try_lock() 以确保在原子上下文中安全。在正常线程上下文中无影响,但分配器使用 SpinNoIrq 表明 alloc_pages 可能从中断上下文调用。

impl Drop for ReclaimGuard {
fn drop(&mut self) {
RECLAIM_IN_PROGRESS.store(false, Ordering::Release);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

使用 ReclaimGuard RAII 模式确保 RECLAIM_IN_PROGRESS 在 panic 时也能恢复,比手动 store(false) 更安全。

}

static GLOBAL_CACHED_FILES: spin::RwLock<alloc::vec::Vec<Weak<CachedFileShared>>> =
spin::RwLock::new(alloc::vec::Vec::new());
Comment on lines +450 to +451

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

改进建议(非阻塞)try_evict_clean_pages 中使用 alloc::vec::Vec::new() 收集待驱逐页面号,在内存极度紧张时可能导致堆分配失败。类似地,page_cache_reclaim() 中的 .collect::<Vec<Arc<...>>>() 也有堆分配。

建议后续迭代中考虑:

  1. 使用 cache.pop_lru() 替代手动扫描 + Vec 收集,逐个驱逐干净页面直到达到目标数量
  2. page_cache_reclaim 中直接遍历 Weak 列表逐个 upgrade() + 驱逐,避免中间 Vec 分配

当前实现在大多数场景下可以正常工作(RECLAIM_IN_PROGRESS 防止递归),作为首版实现可以接受,但建议作为后续优化点记录。


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));
}
Comment on lines +490 to 494

/// A file handle with an LRU page cache for buffered I/O.
Expand Down Expand Up @@ -435,22 +534,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 Down
17 changes: 8 additions & 9 deletions os/arceos/modules/axsync/src/mutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,7 @@ pub struct RawMutex {
pub(crate) lockdep: crate::lockdep::LockdepMap,
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lockdep subclass 信息丢失:当前 lock_nestedtry_lock_nested 忽略 _subclass 参数,始终传 0。原来 TMPFS_DIR_ENTRIES_NESTED_SUBCLASS = 1 的嵌套锁子类信息被丢弃。虽然对当前正确性影响有限(lockdep 在此项目中主要做基本的死锁检测),但如果未来有更多嵌套锁场景,需要恢复条件编译的 LockSubclass 类型和 prepare_nested(..., subclass) 路径。建议在注释中说明这个 trade-off。

#[cfg(not(feature = "lockdep"))]
pub type LockSubclass = u32;
#[cfg(feature = "lockdep")]
pub type LockSubclass = crate::lockdep::LockSubclass;

pub trait LockdepMutexExt<T: ?Sized> {
fn lock_nested(&self, subclass: LockSubclass) -> MutexGuard<'_, T>;
Expand Down Expand Up @@ -99,7 +96,7 @@ unsafe impl lock_api::RawMutex for RawMutex {
#[track_caller]
fn lock(&self) {
#[cfg(feature = "lockdep")]
self.lock_nested(ax_lockdep::DEFAULT_LOCK_SUBCLASS);
self.lock_nested(0);

#[cfg(not(feature = "lockdep"))]
self.lock_plain();
Expand All @@ -110,7 +107,7 @@ unsafe impl lock_api::RawMutex for RawMutex {
fn try_lock(&self) -> bool {
#[cfg(feature = "lockdep")]
{
self.try_lock_nested(ax_lockdep::DEFAULT_LOCK_SUBCLASS)
self.try_lock_nested(0)
}

#[cfg(not(feature = "lockdep"))]
Expand Down Expand Up @@ -183,7 +180,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();

Expand All @@ -196,7 +193,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)
}
Expand All @@ -209,8 +207,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);
Expand Down
Loading