From 8c36964bb940494cd8d1ad14cab0a93b0fd77598 Mon Sep 17 00:00:00 2001 From: Chen Chengjun Date: Wed, 10 Sep 2025 11:59:16 +0000 Subject: [PATCH] Introduce VmPrinter to write kernel generated data --- Cargo.lock | 1 + kernel/comps/systree/Cargo.toml | 1 + kernel/comps/systree/src/lib.rs | 9 ++ kernel/comps/systree/src/test.rs | 12 +- kernel/comps/systree/src/utils.rs | 49 +++----- kernel/libs/aster-util/src/lib.rs | 1 + kernel/libs/aster-util/src/printer.rs | 158 +++++++++++++++++++++++++ kernel/src/fs/cgroupfs/systree_node.rs | 4 +- kernel/src/fs/sysfs/test.rs | 33 ++++-- kernel/src/fs/utils/systree_inode.rs | 8 +- 10 files changed, 222 insertions(+), 54 deletions(-) create mode 100644 kernel/libs/aster-util/src/printer.rs diff --git a/Cargo.lock b/Cargo.lock index 7266485e7..1f54d43a8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -283,6 +283,7 @@ dependencies = [ name = "aster-systree" version = "0.1.0" dependencies = [ + "aster-util", "bitflags 2.9.1", "component", "inherit-methods-macro", diff --git a/kernel/comps/systree/Cargo.toml b/kernel/comps/systree/Cargo.toml index fd0c5c42d..c7ae36b87 100644 --- a/kernel/comps/systree/Cargo.toml +++ b/kernel/comps/systree/Cargo.toml @@ -9,6 +9,7 @@ edition = "2021" bitflags = "2.5" ostd = { path = "../../../ostd" } component = { path = "../../libs/comp-sys/component" } +aster-util = { path = "../../libs/aster-util" } inherit-methods-macro = { git = "https://github.com/asterinas/inherit-methods-macro", rev = "98f7e3e"} spin = "0.9" diff --git a/kernel/comps/systree/src/lib.rs b/kernel/comps/systree/src/lib.rs index f8d54f944..ac1bdb8c9 100644 --- a/kernel/comps/systree/src/lib.rs +++ b/kernel/comps/systree/src/lib.rs @@ -31,6 +31,7 @@ mod utils; use alloc::{borrow::Cow, sync::Arc}; +use aster_util::printer::VmPrinterError; use component::{init_component, ComponentInitError}; use spin::Once; @@ -101,3 +102,11 @@ impl core::fmt::Display for Error { } } } + +impl From for Error { + fn from(value: VmPrinterError) -> Self { + match value { + VmPrinterError::PageFault => Error::PageFault, + } + } +} diff --git a/kernel/comps/systree/src/test.rs b/kernel/comps/systree/src/test.rs index bc098d264..8fd5314da 100644 --- a/kernel/comps/systree/src/test.rs +++ b/kernel/comps/systree/src/test.rs @@ -3,9 +3,10 @@ use alloc::{borrow::Cow, string::ToString, sync::Arc, vec::Vec}; use core::fmt::Debug; +use aster_util::printer::VmPrinter; use inherit_methods_macro::inherit_methods; use ostd::{ - mm::{FallibleVmRead, FallibleVmWrite, VmReader, VmWriter}, + mm::{FallibleVmRead, VmReader, VmWriter}, prelude::ktest, }; @@ -43,7 +44,7 @@ impl DeviceNode { } inherit_sys_branch_node!(DeviceNode, fields, { - fn read_attr(&self, name: &str, writer: &mut VmWriter) -> Result { + fn read_attr_at(&self, name: &str, offset: usize, writer: &mut VmWriter) -> Result { // Check if attribute exists if !self.fields.attr_set().contains(name) { return Err(Error::NotFound); @@ -61,10 +62,11 @@ inherit_sys_branch_node!(DeviceNode, fields, { _ => "", }; + let mut printer = VmPrinter::new_skip(writer, offset); // Write the value to the provided writer - writer - .write_fallible(&mut (value.as_bytes()).into()) - .map_err(|_| Error::AttributeError) + write!(printer, "{}", value)?; + + Ok(printer.bytes_written()) } fn write_attr(&self, name: &str, reader: &mut VmReader) -> Result { diff --git a/kernel/comps/systree/src/utils.rs b/kernel/comps/systree/src/utils.rs index f4388658a..116391100 100644 --- a/kernel/comps/systree/src/utils.rs +++ b/kernel/comps/systree/src/utils.rs @@ -6,12 +6,11 @@ use alloc::{ collections::BTreeMap, string::String, sync::{Arc, Weak}, - vec, }; use inherit_methods_macro::inherit_methods; use ostd::{ - mm::{FallibleVmWrite, VmReader, VmWriter}, + mm::{VmReader, VmWriter}, sync::RwLock, }; use spin::Once; @@ -269,30 +268,6 @@ impl SymlinkNodeFields { } } -macro_rules! impl_default_read_attr_at { - () => { - fn read_attr_at(&self, name: &str, offset: usize, writer: &mut VmWriter) -> Result { - let (attr_buffer, attr_len) = { - let attr_buffer_len = writer.avail().checked_add(offset).ok_or(Error::Overflow)?; - let mut buffer = vec![0; attr_buffer_len]; - let len = self.read_attr( - name, - &mut VmWriter::from(buffer.as_mut_slice()).to_fallible(), - )?; - (buffer, len) - }; - - if attr_len <= offset { - return Ok(0); - } - - writer - .write_fallible(VmReader::from(attr_buffer.as_slice()).skip(offset)) - .map_err(|_| Error::AttributeError) - } - }; -} - #[doc(hidden)] #[macro_export] macro_rules! _inner_impl_sys_node { @@ -356,15 +331,17 @@ pub trait _InheritSysLeafNode { self.field().init_parent(parent); } - fn read_attr(&self, _name: &str, _writer: &mut VmWriter) -> Result { - Err(Error::AttributeError) + fn read_attr(&self, name: &str, writer: &mut VmWriter) -> Result { + self.read_attr_at(name, 0, writer) } fn write_attr(&self, _name: &str, _reader: &mut VmReader) -> Result { Err(Error::AttributeError) } - impl_default_read_attr_at!(); + fn read_attr_at(&self, _name: &str, _offset: usize, _writer: &mut VmWriter) -> Result { + Err(Error::AttributeError) + } fn write_attr_at(&self, name: &str, _offset: usize, reader: &mut VmReader) -> Result { // In general, the `offset` for attribute write operations is ignored directly. @@ -404,6 +381,12 @@ pub trait _InheritSysLeafNode { /// method with the same name from the target `field` or, in the absence of a method with the same /// name, the default implementation provided by the trait. /// +/// Note that for the `SysNode` trait, `read_attr` can be automatically implemented in terms of `read_attr_at`, +/// and `write_attr_at` can be automatically implemented in terms of `write_attr` since most +/// sysfs attributes do not support partial writes and will ignore the `offset`. Therefore, it is **recommended** +/// to override the `read_attr_at` and `write_attr` methods. In addition, users can use +/// [`aster_util::printer::VmPrinter`] to easily handle the `offset` when overriding the `read_attr_at` method. +/// /// ## Examples /// /// ```rust @@ -488,15 +471,17 @@ pub trait _InheritSysBranchNode { self.field().init_parent(parent); } - fn read_attr(&self, _name: &str, _writer: &mut VmWriter) -> Result { - Err(Error::AttributeError) + fn read_attr(&self, name: &str, writer: &mut VmWriter) -> Result { + self.read_attr_at(name, 0, writer) } fn write_attr(&self, _name: &str, _reader: &mut VmReader) -> Result { Err(Error::AttributeError) } - impl_default_read_attr_at!(); + fn read_attr_at(&self, _name: &str, _offset: usize, _writer: &mut VmWriter) -> Result { + Err(Error::AttributeError) + } fn write_attr_at(&self, name: &str, _offset: usize, reader: &mut VmReader) -> Result { // In general, the `offset` for attribute write operations is ignored directly. diff --git a/kernel/libs/aster-util/src/lib.rs b/kernel/libs/aster-util/src/lib.rs index 6c03f1a0f..c937ada16 100644 --- a/kernel/libs/aster-util/src/lib.rs +++ b/kernel/libs/aster-util/src/lib.rs @@ -10,6 +10,7 @@ extern crate alloc; pub mod coeff; pub mod dup; pub mod mem_obj_slice; +pub mod printer; pub mod safe_ptr; pub mod slot_vec; pub mod union_read_ptr; diff --git a/kernel/libs/aster-util/src/printer.rs b/kernel/libs/aster-util/src/printer.rs new file mode 100644 index 000000000..46a616743 --- /dev/null +++ b/kernel/libs/aster-util/src/printer.rs @@ -0,0 +1,158 @@ +// SPDX-License-Identifier: MPL-2.0 + +use core::fmt::{Arguments, Write}; + +use ostd::mm::{FallibleVmWrite, VmReader, VmWriter}; + +/// A specialized printer for formatted text output. +/// +/// `VmPrinter` is designed to handle the common pattern in kernel where kernel +/// code needs to generate formatted text output (like status information, statistics, +/// or configuration data) and write it to user space with proper offset handling. +/// +/// # Examples +/// +/// ```rust,ignore +/// use aster_util::printer::VmPrinter; +/// use ostd::{mm::VmWriter, Pod}; +/// +/// let mut buf = [0u8; 3]; +/// let mut writer = VmWriter::from(buf.as_bytes_mut()).to_fallible(); +/// let mut printer = VmPrinter::new_skip(&mut writer, 3); +/// +/// let res = writeln!(printer, "val: {}", 123); +/// assert!(res.is_ok()); +/// +/// assert_eq!(printer.bytes_written(), 3); +/// assert_eq!(&buf, b": 1"); +/// ``` +pub struct VmPrinter<'a, 'b> { + /// The underlying [`VmWriter`] to write the final output. + writer: &'a mut VmWriter<'b>, + /// Number of bytes to skip from the beginning of the output. + /// + /// When content is written through this writer, the first `bytes_to_skip` + /// bytes will be discarded, and only subsequent bytes will be written + /// to the underlying `VmWriter`. + bytes_to_skip: usize, + /// Total number of bytes written to the underlying writer. + bytes_written: usize, +} + +impl<'a, 'b> VmPrinter<'a, 'b> { + /// Creates a new `VmPrinter` that prints to `writer`. + fn new(writer: &'a mut VmWriter<'b>) -> Self { + Self { + writer, + bytes_to_skip: 0, + bytes_written: 0, + } + } + + /// Creates a new `VmPrinter` that skips the first `bytes_to_skip` bytes and prints the + /// remaining bytes to `writer`. + pub fn new_skip(writer: &'a mut VmWriter<'b>, bytes_to_skip: usize) -> Self { + Self { + writer, + bytes_to_skip, + bytes_written: 0, + } + } + + /// Returns the total number of bytes written to the underlying writer. + pub fn bytes_written(&self) -> usize { + self.bytes_written + } + + /// Writes formatted content to the underlying writer. + pub fn write_fmt(&mut self, args: Arguments<'_>) -> Result<(), VmPrinterError> { + Write::write_fmt(self, args).map_err(|_| VmPrinterError::PageFault) + } + + fn write_bytes(&mut self, bytes: &[u8]) -> core::fmt::Result { + if self.bytes_to_skip >= bytes.len() { + self.bytes_to_skip -= bytes.len(); + return Ok(()); + } + + let bytes_to_write = &bytes[self.bytes_to_skip..]; + if self.bytes_to_skip > 0 { + self.bytes_to_skip = 0; + } + + let mut reader = VmReader::from(bytes_to_write); + let written_len = self + .writer + .write_fallible(&mut reader) + .map_err(|_| core::fmt::Error)?; + + self.bytes_written += written_len; + + Ok(()) + } +} + +impl Write for VmPrinter<'_, '_> { + fn write_str(&mut self, s: &str) -> core::fmt::Result { + self.write_bytes(s.as_bytes()) + } +} + +impl<'a, 'b> From<&'a mut VmWriter<'b>> for VmPrinter<'a, 'b> { + fn from(writer: &'a mut VmWriter<'b>) -> Self { + Self::new(writer) + } +} + +/// An error returned by [`VmPrinter::write_fmt`]. +pub enum VmPrinterError { + /// Page fault occurred. + PageFault, +} + +#[cfg(ktest)] +mod test { + use ostd::{mm::VmWriter, prelude::*, Pod}; + + use super::*; + + #[ktest] + fn basic_write() { + let mut buf = [0u8; 64]; + let mut writer = VmWriter::from(buf.as_bytes_mut()).to_fallible(); + let mut printer = VmPrinter::from(&mut writer); + + let res = writeln!(printer, "test"); + assert!(res.is_ok()); + + assert_eq!(printer.bytes_written(), 5); + assert_eq!(&buf[..5], b"test\n"); + } + + #[ktest] + fn write_with_skip() { + let mut buf = [0u8; 3]; + let mut writer = VmWriter::from(buf.as_bytes_mut()).to_fallible(); + let mut printer = VmPrinter::new_skip(&mut writer, 3); + + let res = writeln!(printer, "val: {}", 123); + assert!(res.is_ok()); + + assert_eq!(printer.bytes_written(), 3); + assert_eq!(&buf, b": 1"); + } + + #[ktest] + fn skip_all_content() { + let mut buf = [0u8; 64]; + let mut writer = VmWriter::from(buf.as_bytes_mut()).to_fallible(); + let mut printer = VmPrinter::new_skip(&mut writer, 100); + + let res = writeln!(printer, "short message"); + assert!(res.is_ok()); + + // Nothing should be written + assert_eq!(printer.bytes_written(), 0); + assert_eq!(buf[0], 0); + } +} diff --git a/kernel/src/fs/cgroupfs/systree_node.rs b/kernel/src/fs/cgroupfs/systree_node.rs index 4e630a880..f3f24b954 100644 --- a/kernel/src/fs/cgroupfs/systree_node.rs +++ b/kernel/src/fs/cgroupfs/systree_node.rs @@ -125,7 +125,7 @@ inherit_sys_branch_node!(CgroupSystem, fields, { // This method should be a no-op for `RootNode`. } - fn read_attr(&self, _name: &str, _writer: &mut VmWriter) -> Result { + fn read_attr_at(&self, _name: &str, _offset: usize, _writer: &mut VmWriter) -> Result { // TODO: Add support for reading attributes. Err(Error::AttributeError) } @@ -147,7 +147,7 @@ inherit_sys_branch_node!(CgroupSystem, fields, { }); inherit_sys_branch_node!(CgroupNode, fields, { - fn read_attr(&self, _name: &str, _writer: &mut VmWriter) -> Result { + fn read_attr_at(&self, _name: &str, _offset: usize, _writer: &mut VmWriter) -> Result { // TODO: Add support for reading attributes. Err(Error::AttributeError) } diff --git a/kernel/src/fs/sysfs/test.rs b/kernel/src/fs/sysfs/test.rs index 5cbc748f6..d7cf25f68 100644 --- a/kernel/src/fs/sysfs/test.rs +++ b/kernel/src/fs/sysfs/test.rs @@ -17,8 +17,9 @@ use aster_systree::{ Result as SysTreeResult, SymlinkNodeFields, SysAttrSetBuilder, SysObj, SysPerms, SysStr, SysTree, }; +use aster_util::printer::VmPrinter; use ostd::{ - mm::{FallibleVmRead, FallibleVmWrite, VmReader, VmWriter}, + mm::{FallibleVmRead, VmReader, VmWriter}, prelude::ktest, sync::RwLock, }; @@ -75,7 +76,12 @@ impl MockLeafNode { } inherit_sys_leaf_node!(MockLeafNode, fields, { - fn read_attr(&self, name: &str, writer: &mut VmWriter) -> SysTreeResult { + fn read_attr_at( + &self, + name: &str, + offset: usize, + writer: &mut VmWriter, + ) -> SysTreeResult { let attr = self .fields .attr_set() @@ -86,6 +92,11 @@ inherit_sys_leaf_node!(MockLeafNode, fields, { } let data = self.data.read(); let value = data.get(name).ok_or(SysTreeError::NotFound)?; // Should exist if in attrs + + let mut printer = VmPrinter::new_skip(writer, offset); + write!(printer, "{}", value)?; + + Ok(printer.bytes_written()) } fn write_attr(&self, name: &str, reader: &mut VmReader) -> SysTreeResult { @@ -102,7 +113,7 @@ inherit_sys_leaf_node!(MockLeafNode, fields, { let mut writer = VmWriter::from(&mut buffer[..]); let read_len = reader .read_fallible(&mut writer) - .map_err(|_| SysTreeError::AttributeError)?; + .map_err(|_| SysTreeError::PageFault)?; let new_value = String::from_utf8_lossy(&buffer[..read_len]).to_string(); @@ -148,7 +159,12 @@ impl MockBranchNode { } inherit_sys_branch_node!(MockBranchNode, fields, { - fn read_attr(&self, name: &str, writer: &mut VmWriter) -> SysTreeResult { + fn read_attr_at( + &self, + name: &str, + offset: usize, + writer: &mut VmWriter, + ) -> SysTreeResult { let attr = self .fields .attr_set() @@ -161,10 +177,11 @@ inherit_sys_branch_node!(MockBranchNode, fields, { "branch_attr" => "branch_value", _ => return Err(SysTreeError::NotFound), }; - let bytes = value.as_bytes(); - writer - .write_fallible(&mut bytes.into()) - .map_err(|_| SysTreeError::AttributeError) + + let mut printer = VmPrinter::new_skip(writer, offset); + write!(printer, "{}", value)?; + + Ok(printer.bytes_written()) } fn write_attr(&self, name: &str, _reader: &mut VmReader) -> SysTreeResult { diff --git a/kernel/src/fs/utils/systree_inode.rs b/kernel/src/fs/utils/systree_inode.rs index cc514745a..48db2be50 100644 --- a/kernel/src/fs/utils/systree_inode.rs +++ b/kernel/src/fs/utils/systree_inode.rs @@ -361,13 +361,7 @@ impl Inode for KInode { return Err(Error::new(Errno::EINVAL)); }; - let len = if offset == 0 { - leaf.read_attr(attr.name(), buf)? - } else { - // The `read_attr_at` method is more general than `read_attr`, - // but it could be less efficient. So we only use the more general form when necessary. - leaf.read_attr_at(attr.name(), offset, buf)? - }; + let len = leaf.read_attr_at(attr.name(), offset, buf)?; Ok(len) }