-
Notifications
You must be signed in to change notification settings - Fork 126
feat(mm): page reclaim for file-backed memory pressure #804
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 4 commits
f4c578f
182055c
39b6e03
ba2bfdd
9c16bb3
2040f87
5efa224
20bfc55
4a9e883
6d21eca
b863629
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. PR 没有添加任何测试,也没有提供可重现的非板级验证命令。预期行为描述(「cargo build inside StarryOS no longer OOMs」)需要物理板级环境才能验证。建议:
|
||
|
|
||
| let loc = FS_CONTEXT | ||
| .lock() | ||
| .resolve(&args[0]) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -191,15 +191,29 @@ impl GlobalAllocator { | |
| alignment: usize, | ||
| kind: UsageKind, | ||
| ) -> AllocResult<usize> { | ||
| let result = self | ||
| let mut result = self | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
let mut result = self.inner.lock().alloc_pages(num_pages, alignment);同样问题也出现在第 223 行的
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 阻塞性: 当前代码: let mut result = self
.inner
.lock()
.alloc_pages(num_pages, alignment);
let mut result = self.inner.lock().alloc_pages(num_pages, alignment);同样在第 226 行的 |
||
| .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); | ||
| .alloc_pages(num_pages, alignment); | ||
| if result.is_err() { | ||
| for _ in 0..4 { | ||
| let reclaimed = crate::try_page_reclaim(num_pages.max(16)); | ||
| if reclaimed == 0 { | ||
| break; | ||
| } | ||
| debug!( | ||
| "page reclaim freed {} pages, retrying allocation ({} pages, kind={:?})", | ||
| reclaimed, num_pages, kind | ||
| ); | ||
| result = self.inner.lock().alloc_pages(num_pages, alignment); | ||
| if result.is_ok() { | ||
| 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). | ||
|
|
@@ -209,15 +223,29 @@ impl GlobalAllocator { | |
| alignment: usize, | ||
| kind: UsageKind, | ||
| ) -> AllocResult<usize> { | ||
| let result = self | ||
| let mut 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); | ||
| .alloc_pages_lowmem(num_pages, alignment); | ||
| if result.is_err() { | ||
| for _ in 0..4 { | ||
| let reclaimed = crate::try_page_reclaim(num_pages.max(16)); | ||
| if reclaimed == 0 { | ||
| break; | ||
| } | ||
| debug!( | ||
| "page reclaim freed {} pages, retrying dma32 allocation ({} pages, kind={:?})", | ||
| reclaimed, num_pages, kind | ||
| ); | ||
| result = self.inner.lock().alloc_pages_lowmem(num_pages, alignment); | ||
| if result.is_ok() { | ||
| 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. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nice: |
||
|
|
||
| /// 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 { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 commit message 声称「default_impl 和 buddy_slab 后端在分配失败时调用 try_page_reclaim() 最多 4 次,每次成功回收后重试」,但实际上 当前代码只是定义了 API 并在 修复方向:在
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,7 +6,7 @@ use alloc::{ | |
| use core::{ | ||
| num::NonZeroUsize, | ||
| ops::Range, | ||
| sync::atomic::{AtomicU8, Ordering}, | ||
| sync::atomic::{AtomicBool, AtomicU8, Ordering}, | ||
| task::Context, | ||
| }; | ||
|
|
||
|
|
@@ -375,23 +375,90 @@ intrusive_adapter!(EvictListenerAdapter = Box<EvictListener>: EvictListener { li | |
|
|
||
| struct CachedFileShared { | ||
| page_cache: Mutex<LruCache<u32, PageCache>>, | ||
| evict_listeners: Mutex<LinkedList<EvictListenerAdapter>>, | ||
| evict_listeners: spin::Mutex<LinkedList<EvictListenerAdapter>>, | ||
| } | ||
|
|
||
| impl CachedFileShared { | ||
| pub fn new() -> Self { | ||
| Self { | ||
| page_cache: Mutex::new(LruCache::new(NonZeroUsize::new(64).unwrap())), | ||
| evict_listeners: Mutex::new(LinkedList::default()), | ||
| evict_listeners: spin::Mutex::new(LinkedList::default()), | ||
| } | ||
| } | ||
|
|
||
| #[allow(dead_code)] | ||
| pub fn new_unbounded() -> Self { | ||
| Self { | ||
| page_cache: Mutex::new(LruCache::unbounded()), | ||
| evict_listeners: Mutex::new(LinkedList::default()), | ||
| evict_listeners: spin::Mutex::new(LinkedList::default()), | ||
| } | ||
| } | ||
|
|
||
| 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 = alloc::vec::Vec::new(); | ||
| for (&pn, page) in cache.iter() { | ||
| if !page.dirty && to_evict.len() < max { | ||
| to_evict.push(pn); | ||
| } | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 此处 |
||
| let count = to_evict.len(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
当前:cache 锁 → 建议分离 pop 和 notify 阶段:先在 cache 锁内收集要驱逐的页面并 pop 出来(已脱离 cache),释放 cache 锁后再通知 listener: fn try_evict_clean_pages(&self, max: usize) -> usize {
let Some(mut cache) = self.page_cache.try_lock() else { return 0; };
let mut evicted_pages: alloc::vec::Vec<(u32, PageCache)> = alloc::vec::Vec::new();
let clean_pns: alloc::vec::Vec<u32> = cache.iter()
.filter(|(_, p)| !p.dirty).take(max).map(|(&pn, _)| pn).collect();
for pn in clean_pns {
if let Some(page) = cache.pop(&pn) {
evicted_pages.push((pn, page));
}
}
let count = evicted_pages.len();
drop(cache); // 释放 cache 锁后再通知 listener
for (pn, page) in &evicted_pages {
for listener in self.evict_listeners.lock().iter() {
(listener.listener)(*pn, page);
}
}
count
}这样 listener 回调可以安全地访问 page_cache 而不会死锁。 |
||
| for pn in to_evict { | ||
| if let Some(page) = cache.pop(&pn) { | ||
| for listener in self.evict_listeners.lock().iter() { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 阻塞性问题:回收路径中调用阻塞锁
如果另一个 CPU 正持有 commit 4a9e883 的提交信息说「Evict listeners registered by the address-space backend can call blocking functions that sleep」,但这个理由只说明了监听器回调内部的行为,并未解决回收路径在原子上下文中调用阻塞锁的问题。 建议修复方案:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This concern is not actually a blocking issue. Analysis:
|
||
| (listener.listener)(pn, &page); | ||
| } | ||
| } | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 锁序文档写得很好,明确了 |
||
| count | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 使用 |
||
| } | ||
|
|
||
| static GLOBAL_CACHED_FILES: spin::RwLock<alloc::vec::Vec<Weak<CachedFileShared>>> = | ||
| spin::RwLock::new(alloc::vec::Vec::new()); | ||
|
Comment on lines
+450
to
+451
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 改进建议(非阻塞): 建议后续迭代中考虑:
当前实现在大多数场景下可以正常工作(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::Acquire) { | ||
|
|
||
| return 0; | ||
| } | ||
|
|
||
| let mut reclaimed = 0; | ||
| let Some(mut guard) = GLOBAL_CACHED_FILES.try_write() else { | ||
| RECLAIM_IN_PROGRESS.store(false, Ordering::Release); | ||
| return 0; | ||
| }; | ||
| let files: alloc::vec::Vec<Arc<CachedFileShared>> = | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 这里两次调用 let (alive, dead): (Vec<_>, Vec<_>) = guard.drain(..).partition(|w| w.upgrade().is_some());
*guard = alive;或者用 |
||
| guard.iter().filter_map(|w| w.upgrade()).collect(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 阻塞:回收路径中的堆分配在内存压力下可能失败。 page_cache_reclaim 的设计目的是在内存不足时被调用,但其实现包含多处堆分配:
在内存压力下,这些 Vec 分配本身可能失败或再次触发回收,导致无限递归或 OOM。 建议修复:避免堆分配——例如用 try_write 锁遍历 Weak 列表逐个 upgrade 后立即驱逐,使用有界栈上缓冲区或直接在 LRU 迭代时驱逐。
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 非阻塞建议:此处 可考虑使用固定大小的栈缓冲区(类似 |
||
| guard.retain(|w| w.upgrade().is_some()); | ||
| drop(guard); | ||
|
|
||
| let target = num_pages.max(64) * 4; | ||
| for file in &files { | ||
| let freed = file.try_evict_clean_pages(target - reclaimed); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 回收路径中的堆分配与内存压力场景矛盾。
如果堆内存已耗尽,这些分配可能失败或递归进入 reclaim。建议:
例如: let guard = GLOBAL_CACHED_FILES.read();
for weak in guard.iter() {
if let Some(file) = weak.upgrade() {
reclaimed += file.try_evict_clean_pages(target - reclaimed);
if reclaimed >= target { break; }
}
} |
||
| reclaimed += freed; | ||
| if reclaimed >= target { | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| if reclaimed > 0 { | ||
| debug!( | ||
| "page_cache_reclaim: evicted {} clean pages across {} files", | ||
| reclaimed, | ||
| files.len() | ||
| ); | ||
| } | ||
|
|
||
| RECLAIM_IN_PROGRESS.store(false, Ordering::Release); | ||
|
|
||
| reclaimed | ||
| } | ||
|
|
||
| fn register_cached_file(file: &Arc<CachedFileShared>) { | ||
| GLOBAL_CACHED_FILES.write().push(Arc::downgrade(file)); | ||
| } | ||
|
Comment on lines
+490
to
494
|
||
|
|
||
| /// A file handle with an LRU page cache for buffered I/O. | ||
|
|
@@ -435,22 +502,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, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,10 +17,7 @@ pub struct RawMutex { | |
| pub(crate) lockdep: crate::lockdep::LockdepMap, | ||
| } | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lockdep subclass 信息丢失:当前 |
||
| #[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>; | ||
|
|
@@ -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(); | ||
|
|
@@ -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"))] | ||
|
|
@@ -183,11 +180,11 @@ impl RawMutex { | |
| #[inline(always)] | ||
| #[track_caller] | ||
| #[cfg(feature = "lockdep")] | ||
| fn lock_nested(&self, subclass: crate::lockdep::LockSubclass) { | ||
| fn lock_nested(&self, _subclass: LockSubclass) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 非阻塞建议: lockdep 使用 subclass 来区分同一锁类的不同嵌套层级,以检测锁序违规。将所有 subclass 统一为 0 意味着 lockdep 无法正确校验嵌套层级的锁序。在当前项目中,唯一的非零 subclass 调用仅有一处,实际死锁风险较低,但未来若有更多嵌套锁场景,建议恢复条件编译的 subclass 类型(如 |
||
| might_sleep(); | ||
| let current_id = current().id().as_u64(); | ||
|
|
||
| let lockdep = crate::lockdep::LockdepAcquire::prepare_nested(self, false, subclass); | ||
| let lockdep = crate::lockdep::LockdepAcquire::prepare(self, false); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 编译错误:
修复方向:
|
||
| self.lock_after_prepare(current_id); | ||
| lockdep.finish(true); | ||
| } | ||
|
|
@@ -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) | ||
| } | ||
|
|
@@ -209,11 +207,12 @@ 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); | ||
| let lockdep = crate::lockdep::LockdepAcquire::prepare(self, true); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 同上, |
||
| let acquired = self.try_lock_after_prepare(current_id); | ||
| lockdep.finish(acquired); | ||
| acquired | ||
|
|
||
There was a problem hiding this comment.
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),此处注册的回调实际上是死代码。需先完成分配器后端集成。