From 4b7f959f45c4d87aef97cda38ac1fb9e49b4c8a4 Mon Sep 17 00:00:00 2001
From: Victor Hsieh <victorhsieh@google.com>
Date: Tue, 8 Mar 2022 20:13:20 +0000
Subject: [PATCH] Use AtomicU64 for handle_ref_count

This allows us to use a read lock instead of a write on `lookup`.

Intriguingly, the corresponding call to decrease the count in `forget`
does not need a lock, thanks to Rust. So this should be a net
performance gain.

Also, panic is a handle's reference count is about to overflow.

Bug: 220386264
Test: atest AuthFsHostTest
Change-Id: Icc3ba1a3fe92431837d6cffb470688ae0883a355
---
 authfs/src/fusefs.rs | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/authfs/src/fusefs.rs b/authfs/src/fusefs.rs
index 82b85017..511db684 100644
--- a/authfs/src/fusefs.rs
+++ b/authfs/src/fusefs.rs
@@ -104,7 +104,7 @@ struct InodeState {
     /// 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,
+    handle_ref_count: AtomicU64,
 
     /// Whether the inode is already unlinked, i.e. should be removed, once `handle_ref_count` is
     /// down to zero.
@@ -113,11 +113,11 @@ struct InodeState {
 
 impl InodeState {
     fn new(entry: AuthFsEntry) -> Self {
-        InodeState { entry, handle_ref_count: 0, unlinked: false }
+        InodeState { entry, handle_ref_count: AtomicU64::new(0), unlinked: false }
     }
 
     fn new_with_ref_count(entry: AuthFsEntry, handle_ref_count: u64) -> Self {
-        InodeState { entry, handle_ref_count, unlinked: false }
+        InodeState { entry, handle_ref_count: AtomicU64::new(handle_ref_count), unlinked: false }
     }
 }
 
@@ -511,7 +511,7 @@ impl FileSystem for AuthFs {
     }
 
     fn lookup(&self, _ctx: Context, parent: Inode, name: &CStr) -> io::Result<Entry> {
-        let mut inode_table = self.inode_table.write().unwrap();
+        let inode_table = self.inode_table.read().unwrap();
 
         // Look up the entry's inode number in parent directory.
         let inode =
@@ -528,8 +528,8 @@ impl FileSystem for AuthFs {
             })?;
 
         // Create the entry's stat if found.
-        let st = handle_inode_mut_locked(
-            &mut inode_table,
+        let st = handle_inode_locked(
+            &inode_table,
             &inode,
             |InodeState { entry, handle_ref_count, .. }| {
                 let st = match entry {
@@ -551,7 +551,9 @@ impl FileSystem for AuthFs {
                         AccessMode::Variable(attr.mode()),
                     ),
                 }?;
-                *handle_ref_count += 1;
+                if handle_ref_count.fetch_add(1, Ordering::Relaxed) == u64::MAX {
+                    panic!("Handle reference count overflow");
+                }
                 Ok(st)
             },
         )?;
@@ -571,15 +573,16 @@ impl FileSystem for AuthFs {
             &mut inode_table,
             &inode,
             |InodeState { handle_ref_count, unlinked, .. }| {
-                if count > *handle_ref_count {
+                let current = handle_ref_count.get_mut();
+                if count > *current {
                     error!(
                         "Trying to decrease refcount of inode {} by {} (> current {})",
-                        inode, count, *handle_ref_count
+                        inode, count, *current
                     );
                     panic!(); // log to logcat with error!
                 }
-                *handle_ref_count = handle_ref_count.saturating_sub(count);
-                Ok(*unlinked && *handle_ref_count == 0)
+                *current -= count;
+                Ok(*unlinked && *current == 0)
             },
         );