Eliminated unnecessary uses of owned locks inside LocalFs.

crates/btfproto/src/local_fs.rs

@@ -31,7 +31,10 @@ use std::{
     },
     time::Duration,
 };
-use tokio::sync::{Mutex, OwnedMutexGuard, OwnedRwLockReadGuard, OwnedRwLockWriteGuard, RwLock};
+use tokio::sync::{
+    Mutex, MutexGuard, OwnedMutexGuard, OwnedRwLockReadGuard, RwLock, RwLockReadGuard,
+    RwLockWriteGuard,
+};
 
 pub use private::{Authorizer, AuthzContext, Error, LocalFs, ModeAuthorizer};
 
@@ -168,24 +171,18 @@ mod private {
         }
     }
 
-    type HandleValueParts<'a> = (
-        &'a Arc<Mutex<Option<Accessor<&'static [u8]>>>>,
-        &'a Arc<BlockPath>,
-        Flags,
-    );
+    type EmptyAccessor = Option<Accessor<&'static [u8]>>;
+
+    type HandleValueParts<'a> = (&'a Arc<Mutex<EmptyAccessor>>, &'a Arc<BlockPath>, Flags);
 
-    struct HandleGuard<B: Size> {
-        guard: OwnedMutexGuard<Option<Accessor<&'static [u8]>>>,
+    struct HandleGuard<B: Size, G: DerefMut<Target = EmptyAccessor>> {
+        guard: G,
         accessor: Option<Accessor<B>>,
         flags: Flags,
     }
 
-    impl<B: Size> HandleGuard<B> {
-        fn new(
-            flags: Flags,
-            mut guard: OwnedMutexGuard<Option<Accessor<&'static [u8]>>>,
-            block: B,
-        ) -> Self {
+    impl<B: Size, G: DerefMut<Target = EmptyAccessor>> HandleGuard<B, G> {
+        fn new(flags: Flags, mut guard: G, block: B) -> Self {
             let accessor = guard
                 .take()
                 .map(move |accessor| Accessor::combine(accessor, block));
@@ -197,7 +194,7 @@ mod private {
         }
     }
 
-    impl<B: Size> Drop for HandleGuard<B> {
+    impl<B: Size, G: DerefMut<Target = EmptyAccessor>> Drop for HandleGuard<B, G> {
         fn drop(&mut self) {
             *self.guard = self.accessor.take().map(|accessor| {
                 let (accessor, _) = accessor.split();
@@ -206,14 +203,14 @@ mod private {
         }
     }
 
-    impl<B: Size> Deref for HandleGuard<B> {
+    impl<B: Size, G: DerefMut<Target = EmptyAccessor>> Deref for HandleGuard<B, G> {
         type Target = Accessor<B>;
         fn deref(&self) -> &Self::Target {
             self.accessor.as_ref().unwrap()
         }
     }
 
-    impl<B: Size> DerefMut for HandleGuard<B> {
+    impl<B: Size, G: DerefMut<Target = EmptyAccessor>> DerefMut for HandleGuard<B, G> {
         fn deref_mut(&mut self) -> &mut Self::Target {
             self.accessor.as_mut().unwrap()
         }
@@ -221,12 +218,12 @@ mod private {
 
     enum HandleValue {
         File {
-            accessor: Arc<Mutex<Option<Accessor<&'static [u8]>>>>,
+            accessor: Arc<Mutex<EmptyAccessor>>,
             owner: Arc<BlockPath>,
             flags: Flags,
         },
         Directory {
-            accessor: Arc<Mutex<Option<Accessor<&'static [u8]>>>>,
+            accessor: Arc<Mutex<EmptyAccessor>>,
             owner: Arc<BlockPath>,
             flags: Flags,
             dir: Directory,
@@ -310,10 +307,7 @@ mod private {
             }
         }
 
-        async fn lock(
-            &self,
-            from: &BlockPath,
-        ) -> Result<(Flags, OwnedMutexGuard<Option<Accessor<&'static [u8]>>>)> {
+        async fn lock(&self, from: &BlockPath) -> Result<(Flags, OwnedMutexGuard<EmptyAccessor>)> {
             let (mutex, owner, flags) = self.parts();
             owner.assert_eq(from)?;
             Ok((flags, mutex.clone().lock_owned().await))
@@ -323,8 +317,10 @@ mod private {
             &'a self,
             from: &BlockPath,
             block: B,
-        ) -> Result<HandleGuard<B>> {
-            let (flags, guard) = self.lock(from).await?;
+        ) -> Result<HandleGuard<B, MutexGuard<'a, EmptyAccessor>>> {
+            let (mutex, owner, flags) = self.parts();
+            owner.assert_eq(from)?;
+            let guard = mutex.lock().await;
             Ok(HandleGuard::new(flags, guard, block))
         }
 
@@ -431,7 +427,7 @@ mod private {
             &'a self,
             from: &BlockPath,
             handle: Handle,
-        ) -> Result<HandleGuard<&FileBlock<C>>> {
+        ) -> Result<HandleGuard<&FileBlock<C>, MutexGuard<'a, EmptyAccessor>>> {
             let value = self.value(handle)?;
             let block = self.block();
             value.guard(from, block).await
@@ -441,7 +437,9 @@ mod private {
             guard: OwnedRwLockReadGuard<Self>,
             from: &BlockPath,
             handle: Handle,
-        ) -> Result<HandleGuard<BlockGuard<OwnedRwLockReadGuard<Self>>>> {
+        ) -> Result<
+            HandleGuard<BlockGuard<OwnedRwLockReadGuard<Self>>, OwnedMutexGuard<EmptyAccessor>>,
+        > {
             let value = guard.value(handle)?;
             let (flags, mutex_guard) = value.lock(from).await?;
             let guard = BlockGuard::new(guard);
@@ -452,7 +450,7 @@ mod private {
             &'a mut self,
             from: &BlockPath,
             handle: Handle,
-        ) -> Result<HandleGuard<&mut FileBlock<C>>> {
+        ) -> Result<HandleGuard<&mut FileBlock<C>, MutexGuard<'a, EmptyAccessor>>> {
             let value = self
                 .handle_values
                 .get(&handle)
@@ -532,31 +530,51 @@ mod private {
     }
 
     type InodeTable<C> = HashMap<Inode, Arc<RwLock<InodeTableValue<C>>>>;
+    type OwnedTableLock<C> = OwnedRwLockReadGuard<InodeTable<C>>;
+    type TableLock<'a, C> = RwLockReadGuard<'a, InodeTable<C>>;
 
-    struct TableGuard<C> {
-        table_guard: OwnedRwLockReadGuard<InodeTable<C>>,
+    struct TableGuard<G> {
+        table_guard: G,
     }
 
-    impl<C> TableGuard<C> {
-        async fn new(table: Arc<RwLock<InodeTable<C>>>) -> TableGuard<C> {
+    impl<C> TableGuard<OwnedRwLockReadGuard<C>> {
+        async fn new_owned(table: Arc<RwLock<InodeTable<C>>>) -> TableGuard<OwnedTableLock<C>> {
             let table_guard = table.read_owned().await;
             TableGuard { table_guard }
         }
+    }
 
-        async fn read(&self, inode: Inode) -> Result<OwnedRwLockReadGuard<InodeTableValue<C>>> {
-            let value = self
-                .table_guard
-                .get(&inode)
-                .ok_or_else(|| bterr!(Error::NotOpen(inode)))?;
-            Ok(value.clone().read_owned().await)
+    impl<'a, C> TableGuard<TableLock<'a, C>> {
+        async fn new(table: &'a RwLock<InodeTable<C>>) -> TableGuard<TableLock<'a, C>> {
+            let table_guard = table.read().await;
+            TableGuard { table_guard }
         }
+    }
 
-        async fn write(&self, inode: Inode) -> Result<OwnedRwLockWriteGuard<InodeTableValue<C>>> {
-            let value = self
-                .table_guard
+    impl<C, G: Deref<Target = InodeTable<C>>> TableGuard<G> {
+        fn get_value(&self, inode: Inode) -> Result<&Arc<RwLock<InodeTableValue<C>>>> {
+            self.table_guard
                 .get(&inode)
-                .ok_or_else(|| bterr!(Error::NotOpen(inode)))?;
-            Ok(value.clone().write_owned().await)
+                .ok_or_else(|| bterr!(Error::NotOpen(inode)))
+        }
+
+        async fn read<'a>(&'a self, inode: Inode) -> Result<RwLockReadGuard<'a, InodeTableValue<C>>>
+        where
+            C: 'a,
+        {
+            let value = self.get_value(inode)?;
+            Ok(value.read().await)
+        }
+
+        async fn write<'a>(
+            &'a self,
+            inode: Inode,
+        ) -> Result<RwLockWriteGuard<'a, InodeTableValue<C>>>
+        where
+            C: 'a,
+        {
+            let value = self.get_value(inode)?;
+            Ok(value.write().await)
         }
     }
 
@@ -777,8 +795,8 @@ mod private {
             Ok(block)
         }
 
-        async fn table_guard(&self) -> TableGuard<C> {
-            TableGuard::new(self.inodes.clone()).await
+        async fn table_guard(&self) -> TableGuard<TableLock<'_, C>> {
+            TableGuard::new(&self.inodes).await
         }
 
         async fn open_value(
@@ -817,7 +835,7 @@ mod private {
             inode: Inode,
             block_path: BlockPath,
             parent_key: Option<SymKey>,
-        ) -> Result<TableGuard<C>> {
+        ) -> Result<TableGuard<OwnedTableLock<C>>> {
             {
                 let table_guard = self.inodes.clone().read_owned().await;
                 if table_guard.contains_key(&inode) {
@@ -826,7 +844,7 @@ mod private {
             }
             self.open_value(from.clone(), inode, block_path, parent_key)
                 .await?;
-            Ok(TableGuard::new(self.inodes.clone()).await)
+            Ok(TableGuard::new_owned(self.inodes.clone()).await)
         }
 
         async fn inode_forget<'a>(
@@ -985,8 +1003,8 @@ mod private {
             Ok(())
         }
 
-        async fn lookup_inode_in(
-            table_guard: &TableGuard<C>,
+        async fn lookup_inode_in<'a>(
+            table_guard: &'a TableGuard<TableLock<'a, C>>,
             parent: Inode,
             name: &str,
         ) -> Result<Inode> {
@@ -999,8 +1017,8 @@ mod private {
 
         /// Returns a pair of inodes, where the first inode is the inode referred to by the given
         /// path, and the second is the parent inode.
-        async fn lookup_inode<'a, I: Iterator<Item = &'a str>>(
-            table_guard: &TableGuard<C>,
+        async fn lookup_inode<'a, 'b, I: Iterator<Item = &'a str>>(
+            table_guard: &'b TableGuard<TableLock<'b, C>>,
             components: I,
         ) -> Result<(Inode, Option<Inode>)> {
             const ROOT: Inode = SpecInodes::RootDir as Inode;
@@ -1068,8 +1086,11 @@ mod private {
         size: u64,
         // Note that handle must come before _table to ensure the guards are dropped in the correct
         // order.
-        handle: HandleGuard<BlockGuard<OwnedRwLockReadGuard<InodeTableValue<C>>>>,
-        _table: OwnedRwLockReadGuard<InodeTable<C>>,
+        handle: HandleGuard<
+            BlockGuard<OwnedRwLockReadGuard<InodeTableValue<C>>>,
+            OwnedMutexGuard<EmptyAccessor>,
+        >,
+        _table: OwnedTableLock<C>,
     }
 
     impl<C: 'static + Principaled + Signer + Decrypter> BufGuard<C> {