From 5b515c0c9b1d0fab62bbca6ec80fb5a8c5fea311 Mon Sep 17 00:00:00 2001 From: Wildan M Date: Wed, 13 May 2026 06:34:39 +0700 Subject: [PATCH 1/4] Implement fmap for read cache --- src/mount/redox/resource.rs | 76 +++++++++++++++++++++++++++++-------- src/mount/redox/scheme.rs | 7 ++-- 2 files changed, 63 insertions(+), 20 deletions(-) diff --git a/src/mount/redox/resource.rs b/src/mount/redox/resource.rs index 400e257aaa..3fbf16fc5e 100644 --- a/src/mount/redox/resource.rs +++ b/src/mount/redox/resource.rs @@ -27,7 +27,13 @@ pub trait Resource { fn set_path(&mut self, path: &str); - fn read(&mut self, buf: &mut [u8], offset: u64, tx: &mut Transaction) -> Result; + fn read( + &mut self, + fmaps: &mut Fmaps, + buf: &mut [u8], + offset: u64, + tx: &mut Transaction, + ) -> Result; fn write(&mut self, buf: &[u8], offset: u64, tx: &mut Transaction) -> Result; @@ -193,7 +199,13 @@ impl Resource for DirResource { self.path = path.to_string(); } - fn read(&mut self, _buf: &mut [u8], _offset: u64, _tx: &mut Transaction) -> Result { + fn read( + &mut self, + _fmaps: &mut Fmaps, + _buf: &mut [u8], + _offset: u64, + _tx: &mut Transaction, + ) -> Result { Err(Error::new(EISDIR)) } @@ -410,7 +422,7 @@ impl FileMmapInfo { } pub fn in_use(&self) -> bool { - self.open_fds > 0 || !self.ranges.is_empty() + self.open_fds > 0 || self.ranges.iter().any(|(_, fmap)| fmap.rc > 0) } } @@ -457,10 +469,44 @@ impl Resource for FileResource { self.path = path.to_string(); } - fn read(&mut self, buf: &mut [u8], offset: u64, tx: &mut Transaction) -> Result { + fn read( + &mut self, + fmaps: &mut Fmaps, + buf: &mut [u8], + offset: u64, + tx: &mut Transaction, + ) -> Result { if self.flags & O_ACCMODE != O_RDWR && self.flags & O_ACCMODE != O_RDONLY { return Err(Error::new(EBADF)); } + if let Some(fmap_info) = fmaps.get_mut(&self.node_ptr.id()) { + if !fmap_info.base.is_null() { + let requested_end = offset + buf.len() as u64; + let mut next_offset = offset; + let mut cacheable = true; + + for (range, _fmap) in fmap_info.ranges.conflicts(offset..requested_end) { + if range.start > next_offset { + // there's a hole + cacheable = false; + break; + } + next_offset = next_offset.max(range.end); + } + + if cacheable && next_offset >= requested_end { + unsafe { + core::ptr::copy_nonoverlapping( + fmap_info.base.add(offset as usize), + buf.as_mut_ptr(), + buf.len(), + ); + } + return Ok(buf.len()); + } + } + } + let atime = SystemTime::now().duration_since(UNIX_EPOCH).unwrap(); tx.read_node( self.node_ptr, @@ -567,26 +613,26 @@ impl Resource for FileResource { for (range, v_opt) in affected_fmaps { //dbg!(&range); + let range_size = range.end - range.start; if let Some(mut fmap) = v_opt { fmap.rc += 1; fmap.flags |= flags; //FIXME: Use result? - let _ = fmap_info - .ranges - .insert(range.start, range.end - range.start, fmap); + let _ = fmap_info.ranges.insert(range.start, range_size, fmap); } else { let map = unsafe { Fmap::new( self.node_ptr, flags, - unaligned_size, - offset, + range_size as usize, + range.start, fmap_info.base, tx, )? }; //FIXME: Use result? - let _ = fmap_info.ranges.insert(offset, aligned_size as u64, map); + //TODO: Save aligned size for unmap + let _ = fmap_info.ranges.insert(range.start, range_size, map); } } //dbg!(&self.fmaps); @@ -624,12 +670,10 @@ impl Resource for FileResource { )?; } - if fmap.rc > 0 { - //FIXME: Use result? - let _ = fmap_info - .ranges - .insert(range.start, range.end - range.start, fmap); - } + //FIXME: Use result? + let _ = fmap_info + .ranges + .insert(range.start, range.end - range.start, fmap); } //dbg!(&self.fmaps); diff --git a/src/mount/redox/scheme.rs b/src/mount/redox/scheme.rs index 8315cee4e0..2e6b729759 100644 --- a/src/mount/redox/scheme.rs +++ b/src/mount/redox/scheme.rs @@ -623,7 +623,7 @@ impl<'sock, D: Disk> SchemeSync for FileScheme<'sock, D> { let Some(Handle::Resource(file)) = self.handles.get_mut(&id) else { return Err(Error::new(EBADF)); }; - self.fs.tx(|tx| file.read(buf, offset, tx)) + self.fs.tx(|tx| file.read(&mut self.fmap, buf, offset, tx)) } fn write( @@ -1006,13 +1006,12 @@ impl<'sock, D: Disk> SchemeSync for FileScheme<'sock, D> { self.fs.tx(|tx| file.fmap(fmaps, flags, size, offset, tx)) } - #[allow(unused_variables)] fn munmap( &mut self, id: usize, offset: u64, size: usize, - flags: MunmapFlags, + _flags: MunmapFlags, _ctx: &CallerCtx, ) -> Result<()> { let Some(Handle::Resource(file)) = self.handles.get_mut(&id) else { @@ -1181,7 +1180,7 @@ impl<'sock, D: Disk> SchemeSync for FileScheme<'sock, D> { &mut self, id: usize, kind: StdFsCallKind, - payload: &mut [u8], + _payload: &mut [u8], metadata: StdFsCallMeta, ctx: &CallerCtx, ) -> Result { From 1d1176de4081abfb932721ab7e88fde1f81aa69d Mon Sep 17 00:00:00 2001 From: Wildan M Date: Wed, 13 May 2026 06:48:39 +0700 Subject: [PATCH 2/4] Add fmap version for invalidation --- src/mount/redox/resource.rs | 39 ++++++++++++++++++++++++++++++++----- src/mount/redox/scheme.rs | 2 +- 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/src/mount/redox/resource.rs b/src/mount/redox/resource.rs index 3fbf16fc5e..39c16f3cbc 100644 --- a/src/mount/redox/resource.rs +++ b/src/mount/redox/resource.rs @@ -35,7 +35,13 @@ pub trait Resource { tx: &mut Transaction, ) -> Result; - fn write(&mut self, buf: &[u8], offset: u64, tx: &mut Transaction) -> Result; + fn write( + &mut self, + fmaps: &mut Fmaps, + buf: &[u8], + offset: u64, + tx: &mut Transaction, + ) -> Result; fn fsize(&mut self, tx: &mut Transaction) -> Result; @@ -209,7 +215,13 @@ impl Resource for DirResource { Err(Error::new(EISDIR)) } - fn write(&mut self, _buf: &[u8], _offset: u64, _tx: &mut Transaction) -> Result { + fn write( + &mut self, + _fmaps: &mut Fmaps, + _buf: &[u8], + _offset: u64, + _tx: &mut Transaction, + ) -> Result { Err(Error::new(EBADF)) } @@ -332,6 +344,7 @@ pub struct Fmap { rc: usize, flags: MapFlags, last_page_tail: u16, + version: u64, } impl Fmap { @@ -370,6 +383,7 @@ impl Fmap { rc: 1, flags, last_page_tail: (unaligned_size % PAGE_SIZE) as u16, + version: 0, }) } @@ -409,6 +423,7 @@ pub struct FileMmapInfo { size: usize, pub ranges: RangeTree, pub open_fds: usize, + pub version: u64, } impl FileMmapInfo { @@ -418,6 +433,7 @@ impl FileMmapInfo { size: 0, ranges: RangeTree::new(), open_fds: 0, + version: 0, } } @@ -485,8 +501,8 @@ impl Resource for FileResource { let mut next_offset = offset; let mut cacheable = true; - for (range, _fmap) in fmap_info.ranges.conflicts(offset..requested_end) { - if range.start > next_offset { + for (range, fmap) in fmap_info.ranges.conflicts(offset..requested_end) { + if range.start > next_offset || fmap.version != fmap_info.version { // there's a hole cacheable = false; break; @@ -517,7 +533,13 @@ impl Resource for FileResource { ) } - fn write(&mut self, buf: &[u8], offset: u64, tx: &mut Transaction) -> Result { + fn write( + &mut self, + fmaps: &mut Fmaps, + buf: &[u8], + offset: u64, + tx: &mut Transaction, + ) -> Result { if self.flags & O_ACCMODE != O_RDWR && self.flags & O_ACCMODE != O_WRONLY { return Err(Error::new(EBADF)); } @@ -527,6 +549,9 @@ impl Resource for FileResource { } else { offset }; + if let Some(fmap_info) = fmaps.get_mut(&self.node_ptr.id()) { + fmap_info.version += 1; + } let mtime = SystemTime::now().duration_since(UNIX_EPOCH).unwrap(); tx.write_node( self.node_ptr, @@ -617,6 +642,7 @@ impl Resource for FileResource { if let Some(mut fmap) = v_opt { fmap.rc += 1; fmap.flags |= flags; + fmap.version = fmap_info.version; //FIXME: Use result? let _ = fmap_info.ranges.insert(range.start, range_size, fmap); } else { @@ -818,6 +844,7 @@ impl range_tree::Value for Fmap { rc: self.rc, flags: self.flags, last_page_tail: 0, + version: self.version, }), Fmap { rc: self.rc, @@ -827,11 +854,13 @@ impl range_tree::Value for Fmap { } else { 0 }, + version: self.version, }, next_range.map(|_range| Fmap { rc: self.rc, flags: self.flags, last_page_tail: self.last_page_tail, + version: self.version, }), ) } diff --git a/src/mount/redox/scheme.rs b/src/mount/redox/scheme.rs index 2e6b729759..c88dffd242 100644 --- a/src/mount/redox/scheme.rs +++ b/src/mount/redox/scheme.rs @@ -638,7 +638,7 @@ impl<'sock, D: Disk> SchemeSync for FileScheme<'sock, D> { let Some(Handle::Resource(file)) = self.handles.get_mut(&id) else { return Err(Error::new(EBADF)); }; - self.fs.tx(|tx| file.write(buf, offset, tx)) + self.fs.tx(|tx| file.write(&mut self.fmap, buf, offset, tx)) } fn fsize(&mut self, id: usize, _ctx: &CallerCtx) -> Result { From 3ed2bee9bb9001f0810572d45f99a8b1335ee46c Mon Sep 17 00:00:00 2001 From: Wildan M Date: Wed, 13 May 2026 07:08:07 +0700 Subject: [PATCH 3/4] Fix CI run --- .gitlab-ci.yml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index b5a0896e65..846edc0bef 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -1,13 +1,14 @@ image: "redoxos/redoxer" +workflow: + rules: + - if: '$CI_COMMIT_BRANCH == "master" && $CI_PROJECT_NAMESPACE == "redox-os"' + - if: '$CI_MERGE_REQUEST_TARGET_BRANCH_NAME == "master"' + stages: - build - test -cache: - paths: - - target/ - build:linux: stage: build script: cargo +nightly build --verbose From d03893a2533bb10a60ff4409929eda0e21868b66 Mon Sep 17 00:00:00 2001 From: Wildan M Date: Wed, 13 May 2026 11:23:45 +0700 Subject: [PATCH 4/4] Add munmap to solve leak --- src/mount/redox/resource.rs | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/mount/redox/resource.rs b/src/mount/redox/resource.rs index 39c16f3cbc..edb30997e9 100644 --- a/src/mount/redox/resource.rs +++ b/src/mount/redox/resource.rs @@ -440,6 +440,14 @@ impl FileMmapInfo { pub fn in_use(&self) -> bool { self.open_fds > 0 || self.ranges.iter().any(|(_, fmap)| fmap.rc > 0) } + + pub fn stale(&self) -> bool { + // TODO: should this be any? + // TODO: stale by duration/memory pressure + self.ranges + .iter() + .all(|(_, fmap)| fmap.version != self.version) + } } impl Drop for FileMmapInfo { @@ -708,10 +716,16 @@ impl Resource for FileResource { // Notify filesystem of close tx.on_close_node(self.node_ptr)?; - /*TODO: leaks memory, but why? - // Remove from fmaps list - fmaps.remove(&self.node_ptr.id()); - */ + // if this fmap version is outdated it's no use + if fmap_info.stale() { + let fmap = fmaps + .remove(&self.node_ptr.id()) + .expect("fmap_info must exist"); + + if let Err(e) = unsafe { libredox::call::munmap(fmap.base as *mut _, fmap.size) } { + log::error!("Munmap error {e}"); + } + } } Ok(())