diff options
author | Florian Klink <flokli@flokli.de> | 2023-11-07T08·28+0200 |
---|---|---|
committer | flokli <flokli@flokli.de> | 2023-11-07T14·52+0000 |
commit | f3a240bf8671a6de85120ee67ad2e84e97834f39 (patch) | |
tree | 08b0f37eded435a40cad61d716052e2b467e9c1f | |
parent | a8e7f4eadbfe59dd97848588a097837e3adcb849 (diff) |
refactor(tvix/store/fs): move inode for store_path lookup to helper r/6973
This makes it much harder to keep the read lock around for too long, and the code a bit easier to understand. Change-Id: I7d99c85cadd433cad444b8edd34e2c43d7eaf5a8 Reviewed-on: https://cl.tvl.fyi/c/depot/+/9977 Tested-by: BuildkiteCI Reviewed-by: Connor Brewster <cbrewster@hey.com>
-rw-r--r-- | tvix/store/src/fs/mod.rs | 32 |
1 files changed, 13 insertions, 19 deletions
diff --git a/tvix/store/src/fs/mod.rs b/tvix/store/src/fs/mod.rs index 72afec5fc1ea..10636fdd5760 100644 --- a/tvix/store/src/fs/mod.rs +++ b/tvix/store/src/fs/mod.rs @@ -124,6 +124,13 @@ impl TvixStoreFs { } } + /// Retrieves the inode for a given StorePath, if present. + /// This obtains a read lock on self.store_paths. + fn get_inode_for_store_path(&self, store_path: &StorePath) -> Option<u64> { + let store_paths = self.store_paths.read(); + store_paths.get(store_path).cloned() + } + /// This will turn a lookup request for [std::ffi::OsStr] in the root to /// a ino and [InodeData]. /// It will peek in [self.store_paths], and then either look it up from @@ -151,14 +158,7 @@ impl TvixStoreFs { return Ok(None); }; - let ino = { - // This extra scope makes sure we drop the read lock - // immediately after reading, to prevent deadlocks. - let store_paths = self.store_paths.read(); - store_paths.get(&store_path).cloned() - }; - - if let Some(ino) = ino { + if let Some(ino) = self.get_inode_for_store_path(&store_path) { // If we already have that store path, lookup the inode from // self.store_paths and then get the data from [self.inode_tracker], // which in the case of a [InodeData::Directory] will be fully @@ -396,22 +396,16 @@ impl FileSystem for TvixStoreFs { let root_node = path_info.node.unwrap().node.unwrap(); let store_path = StorePath::from_bytes(root_node.get_name()).unwrap(); - let ino = { - // This extra scope makes sure we drop the read lock - // immediately after reading, to prevent deadlocks. - let store_paths = self.store_paths.read(); - store_paths.get(&store_path).cloned() - }; - let ino = match ino { - Some(ino) => ino, - None => { + // obtain the inode, or allocate a new one. + let ino = self + .get_inode_for_store_path(&store_path) + .unwrap_or_else(|| { // insert the (sparse) inode data and register in // self.store_paths. let ino = self.inode_tracker.write().put((&root_node).into()); self.store_paths.write().insert(store_path.clone(), ino); ino - } - }; + }); let ty = match root_node { Node::Directory(_) => libc::S_IFDIR, |