Remove `VmMapping::inode`

This commit is contained in:
Ruihan Li 2026-01-30 11:01:10 +08:00 committed by Tate, Hongliang Tian
parent 894f6ba9b4
commit 8297da7247
4 changed files with 52 additions and 75 deletions

View File

@ -12,12 +12,13 @@ use super::{inode_handle::InodeHandle, path::Path};
use crate::{ use crate::{
fs::{ fs::{
file_table::FdFlags, file_table::FdFlags,
utils::{AccessMode, FallocMode, Inode, SeekFrom, StatusFlags}, utils::{AccessMode, FallocMode, SeekFrom, StatusFlags},
}, },
net::socket::Socket, net::socket::Socket,
prelude::*, prelude::*,
process::signal::Pollable, process::signal::Pollable,
util::ioctl::RawIoctl, util::ioctl::RawIoctl,
vm::vmo::Vmo,
}; };
/// The basic operations defined on a file /// 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. /// An object that may be memory mapped into the user address space.
#[derive(Debug, Clone)] #[derive(Debug, Clone)]
pub enum Mappable { pub enum Mappable {
/// An inode object. /// A VMO (i.e., page cache).
Inode(Arc<dyn Inode>), Vmo(Arc<Vmo>),
/// An MMIO region. /// An MMIO region.
IoMem(IoMem), IoMem(IoMem),
} }

View File

@ -328,10 +328,10 @@ impl FileLike for InodeHandle {
} }
let inode = self.path.inode(); 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 // If the inode has a page cache, it is a file-backed mapping and
// we directly return the corresponding inode. // we return the VMO as the mappable object.
Ok(Mappable::Inode(inode.clone())) Ok(Mappable::Vmo(vmo.clone()))
} else if let Some(ref file_io) = self.file_io { } else if let Some(ref file_io) = self.file_io {
// Otherwise, it is a special file (e.g. device file) and we should // Otherwise, it is a special file (e.g. device file) and we should
// return the file-specific mappable object. // return the file-specific mappable object.

View File

@ -68,13 +68,10 @@ pub struct VmMapping {
/// The start of the virtual address maps to the start of the range /// The start of the virtual address maps to the start of the range
/// specified in the mapped object. /// specified in the mapped object.
mapped_mem: MappedMemory, 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<Arc<dyn Inode>>,
/// The path of the file that backs the mapping. /// 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<Path>, path: Option<Path>,
/// Whether the mapping is shared. /// Whether the mapping is shared.
/// ///
@ -99,12 +96,10 @@ impl Interval<Vaddr> for VmMapping {
/***************************** Basic methods *********************************/ /***************************** Basic methods *********************************/
impl VmMapping { impl VmMapping {
#[expect(clippy::too_many_arguments)]
pub(super) fn new( pub(super) fn new(
map_size: NonZeroUsize, map_size: NonZeroUsize,
map_to_addr: Vaddr, map_to_addr: Vaddr,
mapped_mem: MappedMemory, mapped_mem: MappedMemory,
inode: Option<Arc<dyn Inode>>,
path: Option<Path>, path: Option<Path>,
is_shared: bool, is_shared: bool,
handle_page_faults_around: bool, handle_page_faults_around: bool,
@ -114,7 +109,6 @@ impl VmMapping {
map_size, map_size,
map_to_addr, map_to_addr,
mapped_mem, mapped_mem,
inode,
path, path,
is_shared, is_shared,
handle_page_faults_around, handle_page_faults_around,
@ -125,7 +119,6 @@ impl VmMapping {
pub(super) fn new_fork(&self) -> VmMapping { pub(super) fn new_fork(&self) -> VmMapping {
VmMapping { VmMapping {
mapped_mem: self.mapped_mem.dup(), mapped_mem: self.mapped_mem.dup(),
inode: self.inode.clone(),
path: self.path.clone(), path: self.path.clone(),
..*self ..*self
} }
@ -159,7 +152,7 @@ impl VmMapping {
/// Returns the inode of the file that backs the mapping. /// Returns the inode of the file that backs the mapping.
pub fn inode(&self) -> Option<&Arc<dyn Inode>> { pub fn inode(&self) -> Option<&Arc<dyn Inode>> {
self.inode.as_ref() self.path.as_ref().map(|path| path.inode())
} }
/// Returns a reference to the VMO if this mapping is VMO-backed. /// 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_to_addr: self.map_to_addr,
map_size: NonZeroUsize::new(left_size).unwrap(), map_size: NonZeroUsize::new(left_size).unwrap(),
mapped_mem: l_mapped_mem, mapped_mem: l_mapped_mem,
inode: self.inode.clone(),
path: self.path.clone(), path: self.path.clone(),
..self ..self
}; };
@ -964,7 +956,6 @@ fn try_merge(left: &VmMapping, right: &VmMapping) -> Option<VmMapping> {
Some(VmMapping { Some(VmMapping {
map_size, map_size,
mapped_mem, mapped_mem,
inode: left.inode.clone(),
path: left.path.clone(), path: left.path.clone(),
..*left ..*left
}) })

View File

@ -52,7 +52,6 @@ impl Vmar {
/// to overlap with any existing mapping, either. /// to overlap with any existing mapping, either.
pub struct VmarMapOptions<'a> { pub struct VmarMapOptions<'a> {
parent: &'a Vmar, parent: &'a Vmar,
vmo: Option<Arc<Vmo>>,
mappable: Option<Mappable>, mappable: Option<Mappable>,
path: Option<Path>, path: Option<Path>,
perms: VmPerms, perms: VmPerms,
@ -74,7 +73,6 @@ impl<'a> VmarMapOptions<'a> {
pub fn new(parent: &'a Vmar, size: usize, perms: VmPerms) -> Self { pub fn new(parent: &'a Vmar, size: usize, perms: VmPerms) -> Self {
Self { Self {
parent, parent,
vmo: None,
mappable: None, mappable: None,
path: None, path: None,
perms, perms,
@ -116,30 +114,37 @@ impl<'a> VmarMapOptions<'a> {
/// oversized mappings can reserve space for future expansions. /// oversized mappings can reserve space for future expansions.
/// ///
/// The [`Vmo`] of a mapping will be implicitly set if [`Self::mappable`] is /// The [`Vmo`] of a mapping will be implicitly set if [`Self::mappable`] is
/// set with a [`Mappable::Inode`]. /// set with a [`Mappable::Vmo`].
/// ///
/// # Panics /// # 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<Vmo>) -> Self { pub fn vmo(mut self, vmo: Arc<Vmo>) -> Self {
if self.mappable.is_some() { if self.mappable.is_some() {
panic!("Cannot set `vmo` when `mappable` is already set"); panic!("Cannot set `vmo` when `mappable` is already set");
} }
self.vmo = Some(vmo); self.mappable = Some(Mappable::Vmo(vmo));
self self
} }
/// Sets the [`Path`] of the mapping. /// 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 /// # 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 { pub fn path(mut self, path: Path) -> Self {
if self.mappable.is_some() { if self.path.is_some() {
panic!("Cannot set `path` when `mappable` is already set"); panic!("Cannot set `path` when `path` is already set");
} }
self.path = Some(path); self.path = Some(path);
self self
} }
@ -206,37 +211,30 @@ impl<'a> VmarMapOptions<'a> {
self 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) /// This method accepts file-specific details, like a page cache (inode)
/// or I/O memory, but not both simultaneously. /// or I/O memory, but not both simultaneously.
/// ///
/// # Panics /// # 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 /// # Errors
/// ///
/// This function returns an error if the file does not have a corresponding /// 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<Self> { pub fn mappable(mut self, file: &dyn FileLike) -> Result<Self> {
if self.vmo.is_some() { if self.mappable.is_some() {
panic!("Cannot set `mappable` when `vmo` is already set"); panic!("Cannot set `mappable` when `mappable` is already set");
} }
if self.path.is_some() { if self.path.is_some() {
panic!("Cannot set `mappable` when `path` is already set"); 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()?; 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.mappable = Some(mappable);
self.path = Some(file.path().clone()); self.path = Some(file.path().clone());
@ -252,7 +250,6 @@ impl<'a> VmarMapOptions<'a> {
self.check_options()?; self.check_options()?;
let Self { let Self {
parent, parent,
vmo,
mappable, mappable,
path, path,
perms, perms,
@ -310,40 +307,29 @@ impl<'a> VmarMapOptions<'a> {
}; };
// Parse the `Mappable` and prepare the `MappedMemory`. // Parse the `Mappable` and prepare the `MappedMemory`.
let (mapped_mem, inode, io_mem) = if let Some(mappable) = mappable { let (mapped_mem, io_mem) = match mappable {
// Handle the memory backed by device or page cache. Some(Mappable::Vmo(vmo)) => {
match mappable { if let Some(ref path) = path {
Mappable::Inode(inode) => { debug_assert!(Arc::ptr_eq(&vmo, &path.inode().page_cache().unwrap()));
let is_writable_tracked = if let Some(memfd_inode) =
inode.downcast_ref::<MemfdInode>()
&& 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)
} }
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::<MemfdInode>()
&& 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 { Some(Mappable::IoMem(io_mem)) => (MappedMemory::Device, Some(io_mem)),
( None => (MappedMemory::Anonymous, None),
MappedMemory::Vmo(MappedVmo::new(vmo, vmo_offset, false)?),
None,
None,
)
} else {
(MappedMemory::Anonymous, None, None)
}; };
// Build the mapping. // Build the mapping.
@ -351,7 +337,6 @@ impl<'a> VmarMapOptions<'a> {
NonZeroUsize::new(map_size).unwrap(), NonZeroUsize::new(map_size).unwrap(),
map_to_addr, map_to_addr,
mapped_mem, mapped_mem,
inode,
path, path,
is_shared, is_shared,
handle_page_faults_around, handle_page_faults_around,