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 diff --git a/src/mount/redox/resource.rs b/src/mount/redox/resource.rs index 400e257aaa..edb30997e9 100644 --- a/src/mount/redox/resource.rs +++ b/src/mount/redox/resource.rs @@ -27,9 +27,21 @@ 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; + fn write( + &mut self, + fmaps: &mut Fmaps, + buf: &[u8], + offset: u64, + tx: &mut Transaction, + ) -> Result; fn fsize(&mut self, tx: &mut Transaction) -> Result; @@ -193,11 +205,23 @@ 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)) } - 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)) } @@ -320,6 +344,7 @@ pub struct Fmap { rc: usize, flags: MapFlags, last_page_tail: u16, + version: u64, } impl Fmap { @@ -358,6 +383,7 @@ impl Fmap { rc: 1, flags, last_page_tail: (unaligned_size % PAGE_SIZE) as u16, + version: 0, }) } @@ -397,6 +423,7 @@ pub struct FileMmapInfo { size: usize, pub ranges: RangeTree, pub open_fds: usize, + pub version: u64, } impl FileMmapInfo { @@ -406,11 +433,20 @@ impl FileMmapInfo { size: 0, ranges: RangeTree::new(), open_fds: 0, + version: 0, } } 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) + } + + 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) } } @@ -457,10 +493,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 || fmap.version != fmap_info.version { + // 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, @@ -471,7 +541,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)); } @@ -481,6 +557,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, @@ -567,26 +646,27 @@ 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; + fmap.version = fmap_info.version; //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 +704,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); @@ -638,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(()) @@ -774,6 +858,7 @@ impl range_tree::Value for Fmap { rc: self.rc, flags: self.flags, last_page_tail: 0, + version: self.version, }), Fmap { rc: self.rc, @@ -783,11 +868,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 8315cee4e0..c88dffd242 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( @@ -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 { @@ -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 {