From b8e0f01af534fd5c0a205466b521f7d7e68158ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20L=C3=B3pez?= Date: Thu, 15 Jun 2023 15:20:10 +0200 Subject: [PATCH 1/3] fs/init: do not allocate file names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When unpacking the filesystem archive, simply take a reference to the slice containing the file name and avoid needless allocations. Signed-off-by: Carlos López --- src/fs/init.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/fs/init.rs b/src/fs/init.rs index 77fcb75ea..9ba5cc50f 100644 --- a/src/fs/init.rs +++ b/src/fs/init.rs @@ -12,8 +12,6 @@ use super::*; extern crate alloc; use alloc::slice; -use alloc::string::String; -use alloc::vec::Vec; const PACKIT_MAGIC: [u8; 4] = [0x50, 0x4b, 0x49, 0x54]; @@ -57,14 +55,14 @@ impl PackItHeader { } } -struct FileHeader { +struct FileHeader<'a> { name_len: u16, file_size: u64, - name: String, + name: &'a str, } -impl FileHeader { - fn load(buf: &[u8]) -> Result { +impl<'a> FileHeader<'a> { + fn load(buf: &'a [u8]) -> Result { if buf.len() < 12 { log::error!("Unexpected end of archive"); return Err(SvsmError::FileSystem(FsError::inval())); @@ -82,7 +80,7 @@ impl FileHeader { return Err(SvsmError::FileSystem(FsError::inval())); } - let Ok(name) = String::from_utf8(Vec::from(&buf[12..header_len])) else { + let Ok(name) = core::str::from_utf8(&buf[12..header_len]) else { log::error!("Invalid filename in archive"); return Err(SvsmError::FileSystem(FsError::inval())); }; @@ -100,7 +98,7 @@ impl FileHeader { } fn file_name(&self) -> &str { - self.name.as_str() + self.name } fn file_size(&self) -> usize { From 9dbd5730409b675370327f762385a254c95c89a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20L=C3=B3pez?= Date: Thu, 15 Jun 2023 15:21:33 +0200 Subject: [PATCH 2/3] fs/init: implement archive unpacking as an iterator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refactor the archive unpacking code using the Iterator trait, simplifying the main loop in populate_ram_fs(). This introduces a new FsArchive structure that keeps track of the unpacking state. Signed-off-by: Carlos López --- src/fs/init.rs | 88 +++++++++++++++++++++++++++++++++++++------------- 1 file changed, 65 insertions(+), 23 deletions(-) diff --git a/src/fs/init.rs b/src/fs/init.rs index 9ba5cc50f..230f8022c 100644 --- a/src/fs/init.rs +++ b/src/fs/init.rs @@ -15,6 +15,7 @@ use alloc::slice; const PACKIT_MAGIC: [u8; 4] = [0x50, 0x4b, 0x49, 0x54]; +#[derive(Clone, Copy, Debug)] struct PackItHeader { /// Header Magic (PKIT) magic: [u8; 4], @@ -55,6 +56,60 @@ impl PackItHeader { } } +#[derive(Clone, Debug)] +struct FsArchive<'a> { + _hdr: PackItHeader, + data: &'a [u8], + current: usize, +} + +impl<'a> FsArchive<'a> { + fn load(data: &'a [u8]) -> Result { + let _hdr = PackItHeader::load(data)?; + let current = _hdr.len(); + Ok(Self { + _hdr, + data, + current, + }) + } +} + +impl<'a> core::iter::Iterator for FsArchive<'a> { + type Item = Result, SvsmError>; + + fn next(&mut self) -> Option { + if self.current >= self.data.len() { + return None; + } + + let hdr = match FileHeader::load(&self.data[self.current..]) { + Ok(hdr) => hdr, + Err(e) => return Some(Err(e)), + }; + + let Some(start) = self.current.checked_add(hdr.header_size()) else { + return Some(Err(SvsmError::FileSystem(FsError::inval()))); + }; + let Some(end) = start.checked_add(hdr.file_size()) else { + return Some(Err(SvsmError::FileSystem(FsError::inval()))); + }; + let Some(data) = self.data.get(start..end) else { + return Some(Err(SvsmError::FileSystem(FsError::inval()))); + }; + + self.current += hdr.total_size(); + + Some(Ok(FsFile { hdr, data })) + } +} + +struct FsFile<'a> { + hdr: FileHeader<'a>, + data: &'a [u8], +} + +#[derive(Clone, Debug)] struct FileHeader<'a> { name_len: u16, file_size: u64, @@ -133,32 +188,19 @@ pub fn populate_ram_fs(kernel_fs_start: u64, kernel_fs_end: u64) -> Result<(), S let vstart = guard.virt_addr().offset(pstart.page_offset()); let data: &[u8] = unsafe { slice::from_raw_parts(vstart.as_ptr(), size) }; - let hdr = PackItHeader::load(data)?; - - let mut current = hdr.len(); - while current < size { - let fh = FileHeader::load(&data[current..])?; - - let start = current - .checked_add(fh.header_size()) - .ok_or(SvsmError::FileSystem(FsError::inval()))?; - let end = start - .checked_add(fh.file_size()) - .ok_or(SvsmError::FileSystem(FsError::inval()))?; - - let file = create_all(fh.file_name())?; - let file_data = data - .get(start..end) - .ok_or(SvsmError::FileSystem(FsError::inval()))?; - file.truncate(0)?; - let written = file.write(file_data)?; - if written != fh.file_size() { - log::error!("Incomplete data write to {}", fh.file_name()); + let archive = FsArchive::load(data)?; + + for file in archive { + let file = file?; + let handle = create_all(file.hdr.file_name())?; + handle.truncate(0)?; + let written = handle.write(file.data)?; + if written != file.hdr.file_size() { + log::error!("Incomplete data write to {}", file.hdr.file_name()); return Err(SvsmError::FileSystem(FsError::inval())); } - log::info!(" Unpacked {}", fh.file_name()); - current += fh.total_size(); + log::info!(" Unpacked {}", file.hdr.file_name()); } log::info!("Unpacking done"); From ca877ca0f30fcce817467b8dd4fbeece103fcf5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20L=C3=B3pez?= Date: Thu, 15 Jun 2023 15:25:58 +0200 Subject: [PATCH 3/3] fs/fs: do not allocate on path manipulation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Leverage iterators when manipulating stringed paths to avoid a vector allocation on each path walk. Signed-off-by: Carlos López --- src/fs/fs.rs | 56 ++++++++++++++++++++++++++++------------------------ 1 file changed, 30 insertions(+), 26 deletions(-) diff --git a/src/fs/fs.rs b/src/fs/fs.rs index c461a4c8c..ce7444762 100644 --- a/src/fs/fs.rs +++ b/src/fs/fs.rs @@ -142,25 +142,26 @@ fn uninitialize_fs() { } } -fn split_path_allow_empty(path: &str) -> Vec<&str> { - path.split('/').filter(|x| !x.is_empty()).collect() +fn split_path_allow_empty(path: &str) -> impl Iterator + DoubleEndedIterator { + path.split('/').filter(|x| !x.is_empty()) } -fn split_path(path: &str) -> Result, SvsmError> { - let path_items = split_path_allow_empty(path); - - if path_items.is_empty() { - return Err(SvsmError::FileSystem(FsError::inval())); - } - +fn split_path(path: &str) -> Result + DoubleEndedIterator, SvsmError> { + let mut path_items = split_path_allow_empty(path).peekable(); + path_items + .peek() + .ok_or(SvsmError::FileSystem(FsError::inval()))?; Ok(path_items) } -fn walk_path(path_items: &[&str]) -> Result, SvsmError> { +fn walk_path<'a, I>(path_items: I) -> Result, SvsmError> +where + I: Iterator, +{ let mut current_dir = unsafe { FS_ROOT.root_dir() }; - for item in path_items.iter() { - let dir_name = FileName::from(*item); + for item in path_items { + let dir_name = FileName::from(item); let dir_entry = current_dir.lookup_entry(dir_name)?; current_dir = match dir_entry { DirEntry::File(_) => return Err(SvsmError::FileSystem(FsError::file_not_found())), @@ -171,11 +172,14 @@ fn walk_path(path_items: &[&str]) -> Result, SvsmError> { Ok(current_dir) } -fn walk_path_create(path_items: &[&str]) -> Result, SvsmError> { +fn walk_path_create<'a, I>(path_items: I) -> Result, SvsmError> +where + I: Iterator, +{ let mut current_dir = unsafe { FS_ROOT.root_dir() }; - for item in path_items.iter() { - let dir_name = FileName::from(*item); + for item in path_items { + let dir_name = FileName::from(item); let lookup = current_dir.lookup_entry(dir_name); let dir_entry = match lookup { Ok(entry) => entry, @@ -192,8 +196,8 @@ fn walk_path_create(path_items: &[&str]) -> Result, SvsmError pub fn open(path: &str) -> Result { let mut path_items = split_path(path)?; - let file_name = FileName::from(path_items.pop().unwrap()); - let current_dir = walk_path(&path_items)?; + let file_name = FileName::from(path_items.next_back().unwrap()); + let current_dir = walk_path(path_items)?; let dir_entry = current_dir.lookup_entry(file_name)?; @@ -205,8 +209,8 @@ pub fn open(path: &str) -> Result { pub fn create(path: &str) -> Result { let mut path_items = split_path(path)?; - let file_name = FileName::from(path_items.pop().unwrap()); - let current_dir = walk_path(&path_items)?; + let file_name = FileName::from(path_items.next_back().unwrap()); + let current_dir = walk_path(path_items)?; let file = current_dir.create_file(file_name)?; Ok(FileHandle::new(&file)) @@ -215,8 +219,8 @@ pub fn create(path: &str) -> Result { /// Creates a file with all sub-directories pub fn create_all(path: &str) -> Result { let mut path_items = split_path(path)?; - let file_name = FileName::from(path_items.pop().unwrap()); - let current_dir = walk_path_create(&path_items)?; + let file_name = FileName::from(path_items.next_back().unwrap()); + let current_dir = walk_path_create(path_items)?; if file_name.length() == 0 { return Err(SvsmError::FileSystem(FsError::inval())); @@ -229,8 +233,8 @@ pub fn create_all(path: &str) -> Result { pub fn mkdir(path: &str) -> Result<(), SvsmError> { let mut path_items = split_path(path)?; - let dir_name = FileName::from(path_items.pop().unwrap()); - let current_dir = walk_path(&path_items)?; + let dir_name = FileName::from(path_items.next_back().unwrap()); + let current_dir = walk_path(path_items)?; current_dir.create_directory(dir_name)?; @@ -239,15 +243,15 @@ pub fn mkdir(path: &str) -> Result<(), SvsmError> { pub fn unlink(path: &str) -> Result<(), SvsmError> { let mut path_items = split_path(path)?; - let entry_name = FileName::from(path_items.pop().unwrap()); - let dir = walk_path(&path_items)?; + let entry_name = FileName::from(path_items.next_back().unwrap()); + let dir = walk_path(path_items)?; dir.unlink(entry_name) } pub fn list_dir(path: &str) -> Result, SvsmError> { let items = split_path_allow_empty(path); - let dir = walk_path(&items)?; + let dir = walk_path(items)?; Ok(dir.list()) }