From f3a240bf8671a6de85120ee67ad2e84e97834f39 Mon Sep 17 00:00:00 2001 From: Florian Klink Date: Tue, 7 Nov 2023 10:28:49 +0200 Subject: refactor(tvix/store/fs): move inode for store_path lookup to helper 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 --- tvix/store/src/fs/mod.rs | 32 +++++++++++++------------------- 1 file changed, 13 insertions(+), 19 deletions(-) (limited to 'tvix/store') 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 { + 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, -- cgit 1.4.1