From 8297da7247fe44ed6bbd59ee65308ffa0c3e3a65 Mon Sep 17 00:00:00 2001 From: Ruihan Li Date: Fri, 30 Jan 2026 11:01:10 +0800 Subject: [PATCH] Remove `VmMapping::inode` --- kernel/src/fs/file_handle.rs | 7 +- kernel/src/fs/inode_handle.rs | 6 +- kernel/src/vm/vmar/vm_mapping.rs | 17 ++--- kernel/src/vm/vmar/vmar_impls/map.rs | 97 ++++++++++++---------------- 4 files changed, 52 insertions(+), 75 deletions(-) diff --git a/kernel/src/fs/file_handle.rs b/kernel/src/fs/file_handle.rs index 6c2712ad4..fbfeaa4f6 100644 --- a/kernel/src/fs/file_handle.rs +++ b/kernel/src/fs/file_handle.rs @@ -12,12 +12,13 @@ use super::{inode_handle::InodeHandle, path::Path}; use crate::{ fs::{ file_table::FdFlags, - utils::{AccessMode, FallocMode, Inode, SeekFrom, StatusFlags}, + utils::{AccessMode, FallocMode, SeekFrom, StatusFlags}, }, net::socket::Socket, prelude::*, process::signal::Pollable, util::ioctl::RawIoctl, + vm::vmo::Vmo, }; /// The basic operations defined on a file @@ -154,8 +155,8 @@ impl dyn FileLike { /// An object that may be memory mapped into the user address space. #[derive(Debug, Clone)] pub enum Mappable { - /// An inode object. - Inode(Arc), + /// A VMO (i.e., page cache). + Vmo(Arc), /// An MMIO region. IoMem(IoMem), } diff --git a/kernel/src/fs/inode_handle.rs b/kernel/src/fs/inode_handle.rs index 81b6ec5a3..db694f36e 100644 --- a/kernel/src/fs/inode_handle.rs +++ b/kernel/src/fs/inode_handle.rs @@ -328,10 +328,10 @@ impl FileLike for InodeHandle { } let inode = self.path.inode(); - if inode.page_cache().is_some() { + if let Some(ref vmo) = inode.page_cache() { // If the inode has a page cache, it is a file-backed mapping and - // we directly return the corresponding inode. - Ok(Mappable::Inode(inode.clone())) + // we return the VMO as the mappable object. + Ok(Mappable::Vmo(vmo.clone())) } else if let Some(ref file_io) = self.file_io { // Otherwise, it is a special file (e.g. device file) and we should // return the file-specific mappable object. diff --git a/kernel/src/vm/vmar/vm_mapping.rs b/kernel/src/vm/vmar/vm_mapping.rs index d08b6fb65..5fb46435f 100644 --- a/kernel/src/vm/vmar/vm_mapping.rs +++ b/kernel/src/vm/vmar/vm_mapping.rs @@ -68,13 +68,10 @@ pub struct VmMapping { /// The start of the virtual address maps to the start of the range /// specified in the mapped object. mapped_mem: MappedMemory, - /// The inode of the file that backs the mapping. - /// - /// If the inode is `Some`, it means that the mapping is file-backed. - /// And the `mapped_mem` field must be the page cache of the inode, i.e. - /// [`MappedMemory::Vmo`]. - inode: Option>, /// The path of the file that backs the mapping. + /// + /// If the mapping is VMO-backed, the `mapped_mem` field should be the + /// page cache of the inode in the path. path: Option, /// Whether the mapping is shared. /// @@ -99,12 +96,10 @@ impl Interval for VmMapping { /***************************** Basic methods *********************************/ impl VmMapping { - #[expect(clippy::too_many_arguments)] pub(super) fn new( map_size: NonZeroUsize, map_to_addr: Vaddr, mapped_mem: MappedMemory, - inode: Option>, path: Option, is_shared: bool, handle_page_faults_around: bool, @@ -114,7 +109,6 @@ impl VmMapping { map_size, map_to_addr, mapped_mem, - inode, path, is_shared, handle_page_faults_around, @@ -125,7 +119,6 @@ impl VmMapping { pub(super) fn new_fork(&self) -> VmMapping { VmMapping { mapped_mem: self.mapped_mem.dup(), - inode: self.inode.clone(), path: self.path.clone(), ..*self } @@ -159,7 +152,7 @@ impl VmMapping { /// Returns the inode of the file that backs the mapping. pub fn inode(&self) -> Option<&Arc> { - self.inode.as_ref() + self.path.as_ref().map(|path| path.inode()) } /// Returns a reference to the VMO if this mapping is VMO-backed. @@ -641,7 +634,6 @@ impl VmMapping { map_to_addr: self.map_to_addr, map_size: NonZeroUsize::new(left_size).unwrap(), mapped_mem: l_mapped_mem, - inode: self.inode.clone(), path: self.path.clone(), ..self }; @@ -964,7 +956,6 @@ fn try_merge(left: &VmMapping, right: &VmMapping) -> Option { Some(VmMapping { map_size, mapped_mem, - inode: left.inode.clone(), path: left.path.clone(), ..*left }) diff --git a/kernel/src/vm/vmar/vmar_impls/map.rs b/kernel/src/vm/vmar/vmar_impls/map.rs index 8f9dd1beb..7cf350f50 100644 --- a/kernel/src/vm/vmar/vmar_impls/map.rs +++ b/kernel/src/vm/vmar/vmar_impls/map.rs @@ -52,7 +52,6 @@ impl Vmar { /// to overlap with any existing mapping, either. pub struct VmarMapOptions<'a> { parent: &'a Vmar, - vmo: Option>, mappable: Option, path: Option, perms: VmPerms, @@ -74,7 +73,6 @@ impl<'a> VmarMapOptions<'a> { pub fn new(parent: &'a Vmar, size: usize, perms: VmPerms) -> Self { Self { parent, - vmo: None, mappable: None, path: None, perms, @@ -116,30 +114,37 @@ impl<'a> VmarMapOptions<'a> { /// oversized mappings can reserve space for future expansions. /// /// The [`Vmo`] of a mapping will be implicitly set if [`Self::mappable`] is - /// set with a [`Mappable::Inode`]. + /// set with a [`Mappable::Vmo`]. /// /// # Panics /// - /// This function panics if a [`Mappable`] is already provided. + /// This function panics if a [`Vmo`] or [`Mappable`] is already provided. pub fn vmo(mut self, vmo: Arc) -> Self { if self.mappable.is_some() { panic!("Cannot set `vmo` when `mappable` is already set"); } - self.vmo = Some(vmo); + self.mappable = Some(Mappable::Vmo(vmo)); self } /// Sets the [`Path`] of the mapping. /// + /// If a [`Vmo`] is specified, the inode behind the [`Path`] must have + /// the [`Vmo`] as the page cache. + /// + /// The [`Path`] of a mapping will be implicitly set if [`Self::mappable`] + /// is set. + /// /// # Panics /// - /// This function panics if a [`Mappable`] is already provided. + /// This function panics if a [`Path`] is already provided. pub fn path(mut self, path: Path) -> Self { - if self.mappable.is_some() { - panic!("Cannot set `path` when `mappable` is already set"); + if self.path.is_some() { + panic!("Cannot set `path` when `path` is already set"); } self.path = Some(path); + self } @@ -206,37 +211,30 @@ impl<'a> VmarMapOptions<'a> { self } - /// Binds memory to map based on the [`Mappable`] enum. + /// Binds the file's [`Mappable`] object to the mapping and sets the + /// [`Path`] of the mapping. /// /// This method accepts file-specific details, like a page cache (inode) /// or I/O memory, but not both simultaneously. /// /// # Panics /// - /// This function panics if a [`Vmo`], [`Path`] or [`Mappable`] is already provided. + /// This function panics if a [`Vmo`], [`Mappable`], or [`Path`] is already + /// provided. /// /// # Errors /// /// This function returns an error if the file does not have a corresponding - /// mappable object of [`crate::fs::file_handle::Mappable`]. + /// mappable object of [`Mappable`]. pub fn mappable(mut self, file: &dyn FileLike) -> Result { - if self.vmo.is_some() { - panic!("Cannot set `mappable` when `vmo` is already set"); + if self.mappable.is_some() { + panic!("Cannot set `mappable` when `mappable` is already set"); } if self.path.is_some() { panic!("Cannot set `mappable` when `path` is already set"); } - if self.mappable.is_some() { - panic!("Cannot set `mappable` when `mappable` is already set"); - } let mappable = file.mappable()?; - - // Verify whether the page cache inode is valid. - if let Mappable::Inode(ref inode) = mappable { - self.vmo = Some(inode.page_cache().expect("Map an inode without page cache")); - } - self.mappable = Some(mappable); self.path = Some(file.path().clone()); @@ -252,7 +250,6 @@ impl<'a> VmarMapOptions<'a> { self.check_options()?; let Self { parent, - vmo, mappable, path, perms, @@ -310,40 +307,29 @@ impl<'a> VmarMapOptions<'a> { }; // Parse the `Mappable` and prepare the `MappedMemory`. - let (mapped_mem, inode, io_mem) = if let Some(mappable) = mappable { - // Handle the memory backed by device or page cache. - match mappable { - Mappable::Inode(inode) => { - let is_writable_tracked = if let Some(memfd_inode) = - inode.downcast_ref::() - && is_shared - && may_perms.contains(VmPerms::MAY_WRITE) - { - memfd_inode.check_writable(perms, &mut may_perms)?; - true - } else { - false - }; - - // Since `Mappable::Inode` is provided, it is - // reasonable to assume that the VMO is provided. - let mapped_mem = MappedMemory::Vmo(MappedVmo::new( - vmo.unwrap(), - vmo_offset, - is_writable_tracked, - )?); - (mapped_mem, Some(inode), None) + let (mapped_mem, io_mem) = match mappable { + Some(Mappable::Vmo(vmo)) => { + if let Some(ref path) = path { + debug_assert!(Arc::ptr_eq(&vmo, &path.inode().page_cache().unwrap())); } - Mappable::IoMem(iomem) => (MappedMemory::Device, None, Some(iomem)), + + let is_writable_tracked = if let Some(ref path) = path + && let Some(memfd_inode) = path.inode().downcast_ref::() + && is_shared + && may_perms.contains(VmPerms::MAY_WRITE) + { + memfd_inode.check_writable(perms, &mut may_perms)?; + true + } else { + false + }; + + let mapped_mem = + MappedMemory::Vmo(MappedVmo::new(vmo, vmo_offset, is_writable_tracked)?); + (mapped_mem, None) } - } else if let Some(vmo) = vmo { - ( - MappedMemory::Vmo(MappedVmo::new(vmo, vmo_offset, false)?), - None, - None, - ) - } else { - (MappedMemory::Anonymous, None, None) + Some(Mappable::IoMem(io_mem)) => (MappedMemory::Device, Some(io_mem)), + None => (MappedMemory::Anonymous, None), }; // Build the mapping. @@ -351,7 +337,6 @@ impl<'a> VmarMapOptions<'a> { NonZeroUsize::new(map_size).unwrap(), map_to_addr, mapped_mem, - inode, path, is_shared, handle_page_faults_around,