Track inode reference count from handles
With this change, we start to track all inodes' reference count by the
handles. The reference count will be used when unlink/rmdir is
implemented, but at this moment, there should be no behavior change /
feature differences.
The `inode_table` now stores `InodeState`, a new struct that contains
the original `AuthFsEntry` and now the handle reference count. It
will/can also be used to store metadata such as the file modes, xattrs
and uid/gid (though some may not make sense on AuthFS).
Bug: 208892249
Test: atest AuthFsHostTest ComposHostTestCases
Change-Id: Ic48be15a8443c7cda0661ec0202151a3339a0319
diff --git a/authfs/src/fusefs.rs b/authfs/src/fusefs.rs
index 8ca82f8..6fdcd62 100644
--- a/authfs/src/fusefs.rs
+++ b/authfs/src/fusefs.rs
@@ -17,7 +17,7 @@
mod mount;
use anyhow::{anyhow, bail, Result};
-use log::{debug, warn};
+use log::{debug, error, warn};
use std::collections::{btree_map, BTreeMap};
use std::convert::TryFrom;
use std::ffi::{CStr, OsStr};
@@ -72,11 +72,36 @@
VerifiedNewDirectory { dir: RemoteDirEditor },
}
+struct InodeState {
+ /// Actual inode entry.
+ entry: AuthFsEntry,
+
+ /// Number of `Handle`s (i.e. file descriptors) that are currently referring to the this inode.
+ ///
+ /// Technically, this does not matter to readonly entries, since they live forever. The
+ /// reference count is only needed for manageing lifetime of writable entries like `VerifiedNew`
+ /// and `VerifiedNewDirectory`. That is, when an entry is deleted, the actual entry needs to
+ /// stay alive until the reference count reaches zero.
+ ///
+ /// Note: This is not to be confused with hardlinks, which AuthFS doesn't currently implement.
+ handle_ref_count: u64,
+}
+
+impl InodeState {
+ fn new(entry: AuthFsEntry) -> Self {
+ InodeState { entry, handle_ref_count: 0 }
+ }
+
+ fn new_with_ref_count(entry: AuthFsEntry, handle_ref_count: u64) -> Self {
+ InodeState { entry, handle_ref_count }
+ }
+}
+
// AuthFS needs to be `Sync` to be accepted by fuse::worker::start_message_loop as a `FileSystem`.
pub struct AuthFs {
- /// Table for `Inode` to `AuthFsEntry` lookup. This needs to be `Sync` to be used in
+ /// Table for `Inode` to `InodeState` lookup. This needs to be `Sync` to be used in
/// `fuse::worker::start_message_loop`.
- inode_table: Mutex<BTreeMap<Inode, AuthFsEntry>>,
+ inode_table: Mutex<BTreeMap<Inode, InodeState>>,
/// The next available inode number.
next_inode: AtomicU64,
@@ -92,7 +117,10 @@
impl AuthFs {
pub fn new(remote_fs_stats_reader: RemoteFsStatsReader) -> AuthFs {
let mut inode_table = BTreeMap::new();
- inode_table.insert(ROOT_INODE, AuthFsEntry::ReadonlyDirectory { dir: InMemoryDir::new() });
+ inode_table.insert(
+ ROOT_INODE,
+ InodeState::new(AuthFsEntry::ReadonlyDirectory { dir: InMemoryDir::new() }),
+ );
AuthFs {
inode_table: Mutex::new(inode_table),
@@ -130,10 +158,12 @@
Component::Normal(name) => {
let inode_table = self.inode_table.get_mut().unwrap();
// Locate the internal directory structure.
- let current_dir_entry =
- inode_table.get_mut(¤t_dir_inode).ok_or_else(|| {
+ let current_dir_entry = &mut inode_table
+ .get_mut(¤t_dir_inode)
+ .ok_or_else(|| {
anyhow!("Unknown directory inode {}", current_dir_inode)
- })?;
+ })?
+ .entry;
let dir = match current_dir_entry {
AuthFsEntry::ReadonlyDirectory { dir } => dir,
_ => unreachable!("Not a ReadonlyDirectory"),
@@ -148,7 +178,10 @@
// Actually update the tables.
dir.add_entry(name.as_ref(), new_inode)?;
- if inode_table.insert(new_inode, new_dir_entry).is_some() {
+ if inode_table
+ .insert(new_inode, InodeState::new(new_dir_entry))
+ .is_some()
+ {
bail!("Unexpected to find a duplicated inode");
}
Ok(new_inode)
@@ -160,7 +193,8 @@
// 2. Insert the entry to the parent directory, as well as the inode table.
let inode_table = self.inode_table.get_mut().unwrap();
- match inode_table.get_mut(&parent_inode).expect("previously returned inode") {
+ let inode_state = inode_table.get_mut(&parent_inode).expect("previously returned inode");
+ match &mut inode_state.entry {
AuthFsEntry::ReadonlyDirectory { dir } => {
let basename =
path.file_name().ok_or_else(|| anyhow!("Bad file name: {:?}", path))?;
@@ -168,7 +202,7 @@
// Actually update the tables.
dir.add_entry(basename.as_ref(), new_inode)?;
- if inode_table.insert(new_inode, entry).is_some() {
+ if inode_table.insert(new_inode, InodeState::new(entry)).is_some() {
bail!("Unexpected to find a duplicated inode");
}
Ok(new_inode)
@@ -187,12 +221,11 @@
F: FnOnce(&AuthFsEntry) -> io::Result<R>,
{
let inode_table = self.inode_table.lock().unwrap();
- let entry =
- inode_table.get(inode).ok_or_else(|| io::Error::from_raw_os_error(libc::ENOENT))?;
- handle_fn(entry)
+ handle_inode_locked(&inode_table, inode, |inode_state| handle_fn(&inode_state.entry))
}
- /// Adds a new entry `name` created by `create_fn` at `parent_inode`.
+ /// Adds a new entry `name` created by `create_fn` at `parent_inode`, with an initial ref count
+ /// of one.
///
/// The operation involves two updates: adding the name with a new allocated inode to the
/// parent directory, and insert the new inode and the actual `AuthFsEntry` to the global inode
@@ -200,7 +233,7 @@
///
/// `create_fn` receives the parent directory, through which it can create the new entry at and
/// register the new inode to. Its returned entry is then added to the inode table.
- fn create_new_entry<F>(
+ fn create_new_entry_with_ref_count<F>(
&self,
parent_inode: Inode,
name: &CStr,
@@ -210,15 +243,19 @@
F: FnOnce(&mut AuthFsEntry, &Path, Inode) -> io::Result<AuthFsEntry>,
{
let mut inode_table = self.inode_table.lock().unwrap();
- let parent_entry = inode_table
- .get_mut(&parent_inode)
- .ok_or_else(|| io::Error::from_raw_os_error(libc::ENOENT))?;
+ let (new_inode, new_file_entry) = handle_inode_mut_locked(
+ &mut inode_table,
+ &parent_inode,
+ |InodeState { entry, .. }| {
+ let new_inode = self.next_inode.fetch_add(1, Ordering::Relaxed);
+ let basename: &Path = cstr_to_path(name);
+ let new_file_entry = create_fn(entry, basename, new_inode)?;
+ Ok((new_inode, new_file_entry))
+ },
+ )?;
- let new_inode = self.next_inode.fetch_add(1, Ordering::Relaxed);
- let basename: &Path = cstr_to_path(name);
- let new_file_entry = create_fn(parent_entry, basename, new_inode)?;
if let btree_map::Entry::Vacant(entry) = inode_table.entry(new_inode) {
- entry.insert(new_file_entry);
+ entry.insert(InodeState::new_with_ref_count(new_file_entry, 1));
Ok(new_inode)
} else {
unreachable!("Unexpected duplication of inode {}", new_inode);
@@ -363,39 +400,47 @@
}
fn lookup(&self, _ctx: Context, parent: Inode, name: &CStr) -> io::Result<Entry> {
- // Look up the entry's inode number in parent directory.
- let inode = self.handle_inode(&parent, |parent_entry| match parent_entry {
- AuthFsEntry::ReadonlyDirectory { dir } => {
- let path = cstr_to_path(name);
- dir.lookup_inode(path).ok_or_else(|| io::Error::from_raw_os_error(libc::ENOENT))
- }
- AuthFsEntry::VerifiedNewDirectory { dir } => {
- let path = cstr_to_path(name);
- dir.find_inode(path).ok_or_else(|| io::Error::from_raw_os_error(libc::ENOENT))
- }
- _ => Err(io::Error::from_raw_os_error(libc::ENOTDIR)),
- })?;
+ let mut inode_table = self.inode_table.lock().unwrap();
- // Normally, `lookup` is required to increase a reference count for the inode (while
- // `forget` will decrease it). It is not yet necessary until we start to support
- // deletion (only for `VerifiedNewDirectory`).
+ // Look up the entry's inode number in parent directory.
+ let inode =
+ handle_inode_locked(&inode_table, &parent, |inode_state| match &inode_state.entry {
+ AuthFsEntry::ReadonlyDirectory { dir } => {
+ let path = cstr_to_path(name);
+ dir.lookup_inode(path).ok_or_else(|| io::Error::from_raw_os_error(libc::ENOENT))
+ }
+ AuthFsEntry::VerifiedNewDirectory { dir } => {
+ let path = cstr_to_path(name);
+ dir.find_inode(path).ok_or_else(|| io::Error::from_raw_os_error(libc::ENOENT))
+ }
+ _ => Err(io::Error::from_raw_os_error(libc::ENOTDIR)),
+ })?;
// Create the entry's stat if found.
- let st = self.handle_inode(&inode, |entry| match entry {
- AuthFsEntry::ReadonlyDirectory { dir } => {
- create_dir_stat(inode, dir.number_of_entries())
- }
- AuthFsEntry::UnverifiedReadonly { file_size, .. }
- | AuthFsEntry::VerifiedReadonly { file_size, .. } => {
- create_stat(inode, *file_size, AccessMode::ReadOnly)
- }
- AuthFsEntry::VerifiedNew { editor } => {
- create_stat(inode, editor.size(), AccessMode::ReadWrite)
- }
- AuthFsEntry::VerifiedNewDirectory { dir } => {
- create_dir_stat(inode, dir.number_of_entries())
- }
- })?;
+ let st = handle_inode_mut_locked(
+ &mut inode_table,
+ &inode,
+ |InodeState { entry, handle_ref_count, .. }| {
+ let st = match entry {
+ AuthFsEntry::ReadonlyDirectory { dir } => {
+ create_dir_stat(inode, dir.number_of_entries())
+ }
+ AuthFsEntry::UnverifiedReadonly { file_size, .. }
+ | AuthFsEntry::VerifiedReadonly { file_size, .. } => {
+ create_stat(inode, *file_size, AccessMode::ReadOnly)
+ }
+ AuthFsEntry::VerifiedNew { editor } => {
+ create_stat(inode, editor.size(), AccessMode::ReadWrite)
+ }
+ AuthFsEntry::VerifiedNewDirectory { dir } => {
+ create_dir_stat(inode, dir.number_of_entries())
+ }
+ }?;
+ *handle_ref_count += 1;
+ Ok(st)
+ },
+ )?;
+
Ok(Entry {
inode,
generation: 0,
@@ -405,6 +450,27 @@
})
}
+ fn forget(&self, _ctx: Context, inode: Self::Inode, count: u64) {
+ let mut inode_table = self.inode_table.lock().unwrap();
+ let _ = handle_inode_mut_locked(
+ &mut inode_table,
+ &inode,
+ |InodeState { handle_ref_count, .. }| {
+ if count > *handle_ref_count {
+ error!(
+ "Trying to decrease refcount of inode {} by {} (> current {})",
+ inode, count, *handle_ref_count
+ );
+ panic!(); // log to logcat with error!
+ }
+ *handle_ref_count = handle_ref_count.saturating_sub(count);
+ // TODO(208892249): Remove the inode with zero ref count from inode_table, if the
+ // inode is marked for deletion.
+ Ok(())
+ },
+ );
+ }
+
fn getattr(
&self,
_ctx: Context,
@@ -473,19 +539,20 @@
) -> io::Result<(Entry, Option<Self::Handle>, fuse::sys::OpenOptions)> {
// TODO(205169366): Implement mode properly.
// TODO(205172873): handle O_TRUNC and O_EXCL properly.
- let new_inode =
- self.create_new_entry(parent, name, |parent_entry, basename, new_inode| {
- match parent_entry {
- AuthFsEntry::VerifiedNewDirectory { dir } => {
- if dir.find_inode(basename).is_some() {
- return Err(io::Error::from_raw_os_error(libc::EEXIST));
- }
- let new_file = dir.create_file(basename, new_inode)?;
- Ok(AuthFsEntry::VerifiedNew { editor: new_file })
+ let new_inode = self.create_new_entry_with_ref_count(
+ parent,
+ name,
+ |parent_entry, basename, new_inode| match parent_entry {
+ AuthFsEntry::VerifiedNewDirectory { dir } => {
+ if dir.find_inode(basename).is_some() {
+ return Err(io::Error::from_raw_os_error(libc::EEXIST));
}
- _ => Err(io::Error::from_raw_os_error(libc::EBADF)),
+ let new_file = dir.create_file(basename, new_inode)?;
+ Ok(AuthFsEntry::VerifiedNew { editor: new_file })
}
- })?;
+ _ => Err(io::Error::from_raw_os_error(libc::EBADF)),
+ },
+ )?;
Ok((
Entry {
@@ -651,22 +718,23 @@
_umask: u32,
) -> io::Result<Entry> {
// TODO(205169366): Implement mode properly.
- let new_inode =
- self.create_new_entry(parent, name, |parent_entry, basename, new_inode| {
- match parent_entry {
- AuthFsEntry::VerifiedNewDirectory { dir } => {
- if dir.find_inode(basename).is_some() {
- return Err(io::Error::from_raw_os_error(libc::EEXIST));
- }
- let new_dir = dir.mkdir(basename, new_inode)?;
- Ok(AuthFsEntry::VerifiedNewDirectory { dir: new_dir })
+ let new_inode = self.create_new_entry_with_ref_count(
+ parent,
+ name,
+ |parent_entry, basename, new_inode| match parent_entry {
+ AuthFsEntry::VerifiedNewDirectory { dir } => {
+ if dir.find_inode(basename).is_some() {
+ return Err(io::Error::from_raw_os_error(libc::EEXIST));
}
- AuthFsEntry::ReadonlyDirectory { .. } => {
- Err(io::Error::from_raw_os_error(libc::EACCES))
- }
- _ => Err(io::Error::from_raw_os_error(libc::EBADF)),
+ let new_dir = dir.mkdir(basename, new_inode)?;
+ Ok(AuthFsEntry::VerifiedNewDirectory { dir: new_dir })
}
- })?;
+ AuthFsEntry::ReadonlyDirectory { .. } => {
+ Err(io::Error::from_raw_os_error(libc::EACCES))
+ }
+ _ => Err(io::Error::from_raw_os_error(libc::EBADF)),
+ },
+ )?;
Ok(Entry {
inode: new_inode,
@@ -704,6 +772,36 @@
}
}
+fn handle_inode_locked<F, R>(
+ inode_table: &BTreeMap<Inode, InodeState>,
+ inode: &Inode,
+ handle_fn: F,
+) -> io::Result<R>
+where
+ F: FnOnce(&InodeState) -> io::Result<R>,
+{
+ if let Some(inode_state) = inode_table.get(inode) {
+ handle_fn(inode_state)
+ } else {
+ Err(io::Error::from_raw_os_error(libc::ENOENT))
+ }
+}
+
+fn handle_inode_mut_locked<F, R>(
+ inode_table: &mut BTreeMap<Inode, InodeState>,
+ inode: &Inode,
+ handle_fn: F,
+) -> io::Result<R>
+where
+ F: FnOnce(&mut InodeState) -> io::Result<R>,
+{
+ if let Some(inode_state) = inode_table.get_mut(inode) {
+ handle_fn(inode_state)
+ } else {
+ Err(io::Error::from_raw_os_error(libc::ENOENT))
+ }
+}
+
fn cstr_to_path(cstr: &CStr) -> &Path {
OsStr::from_bytes(cstr.to_bytes()).as_ref()
}