From 258fce50e0f84b6a980f750f7540802e821bebda Mon Sep 17 00:00:00 2001 From: Jim906 Date: Sat, 5 Jul 2025 12:40:59 -0400 Subject: [PATCH] nfs4: Check for stale nodes when reload dir cache * When the DirectoryCache is updated, compare the new contents with the old. If any of the old entries are obsolete, treat the respective node as stale. * Create FileSystem::TrashStaleNode() to avoid duplicating code. * Possible fix for #19656. This helps the client keep current with changes made to server files by other users. While the comparison will be slow for a large directory, it won't be executed unless another user has changed the contents of a directory since the last time the Haiku client read the directory. Change-Id: I03e65aaaad5027c66b10265f2b4a21ed9d2b6744 Reviewed-on: https://review.haiku-os.org/c/haiku/+/9445 Tested-by: Commit checker robot Reviewed-by: waddlesplash --- .../file_systems/nfs4/DirectoryCache.cpp | 28 ++++++++- .../kernel/file_systems/nfs4/FileSystem.cpp | 59 ++++++++++++------- .../kernel/file_systems/nfs4/FileSystem.h | 1 + 3 files changed, 63 insertions(+), 25 deletions(-) diff --git a/src/add-ons/kernel/file_systems/nfs4/DirectoryCache.cpp b/src/add-ons/kernel/file_systems/nfs4/DirectoryCache.cpp index c3c3ae6336..016d71ed11 100644 --- a/src/add-ons/kernel/file_systems/nfs4/DirectoryCache.cpp +++ b/src/add-ons/kernel/file_systems/nfs4/DirectoryCache.cpp @@ -206,9 +206,6 @@ DirectoryCache::_LoadSnapshot(bool trash) if (oldSnapshot != NULL) oldSnapshot->AcquireReference(); - if (trash) - Trash(); - DirectoryCacheSnapshot* newSnapshot; status_t result = fInode->GetDirSnapshot(&newSnapshot, NULL, &fChange, fAttrDir); @@ -222,6 +219,31 @@ DirectoryCache::_LoadSnapshot(bool trash) _SetSnapshot(newSnapshot); fExpireTime = system_time() + fExpirationTime; + if (trash) { + // Clear fNameCache, while checking for mismatches between its entries and the newly + // obtained snapshot that might indicate stale nodes. + while (!fNameCache.IsEmpty()) { + NameCacheEntry* current = fNameCache.RemoveHead(); + bool nodeFound = false; + for (SinglyLinkedList::ConstIterator it + = newSnapshot->fEntries.GetIterator(); + NameCacheEntry* snapshotEntry = it.Next();) { + if (current->fNode == snapshotEntry->fNode + && strcmp(current->fName, snapshotEntry->fName) == 0) { + nodeFound = true; + break; + } + } + if (!nodeFound) { + // The inode-name association that was cached in 'current' is no longer valid. + result = fInode->GetFileSystem()->TrashStaleNode(current->fNode); + if (result != B_OK) + INFORM("_LoadSnapshot: Couldn't free stale node.\n"); + } + delete current; + } + } + fTrashed = false; if (oldSnapshot != NULL) diff --git a/src/add-ons/kernel/file_systems/nfs4/FileSystem.cpp b/src/add-ons/kernel/file_systems/nfs4/FileSystem.cpp index b8f088e4d4..e44098a3f7 100644 --- a/src/add-ons/kernel/file_systems/nfs4/FileSystem.cpp +++ b/src/add-ons/kernel/file_systems/nfs4/FileSystem.cpp @@ -469,6 +469,41 @@ FileSystem::GetDelegation(const FileHandle& handle) return it.Current(); } +/*! If a node object with this inode number exists, free it at the FS and VFS levels to + ensure that no operation will try to use it. + +*/ +status_t +FileSystem::TrashStaleNode(ino_t ino) +{ + status_t result = B_OK; + if (acquire_vnode(fFsVolume, ino) == B_OK) { + // we still have a vnode for the stale file present in memory + + // mark it as stale + VnodeToInode* vti; + result = get_vnode(fFsVolume, ino, reinterpret_cast(&vti)); + ASSERT(result == B_OK); + Inode* inode = vti->GetPointer(); + if (inode != NULL) + inode->SetStale(); + put_vnode(fFsVolume, ino); + + // delete it + result = remove_vnode(fFsVolume, ino); + ASSERT(result == B_OK); + put_vnode(fFsVolume, ino); + + // verify it is gone + if (acquire_vnode(fFsVolume, ino) == B_OK) + result = B_ERROR; + else + result = B_OK; + } + + return result; +} + /*! Used when creating a file to check for a stale node with the same ino. If it exists, the stale node is deleted. @@ -488,28 +523,8 @@ FileSystem::EnsureNoCollision(ino_t newID, const FileHandle& handle) // We are already using this file ID for a previously existing file. If the server has // assigned that ID to the file that we are now creating, it means someone else must have // deleted the other file from the server, and the server is recycling the file ID. - result = acquire_vnode(fFsVolume, newID); - if (result == B_OK) { - // we still have a vnode for the stale file present in memory - - // mark it as stale - VnodeToInode* vti; - result = get_vnode(fFsVolume, newID, reinterpret_cast(&vti)); - ASSERT(result == B_OK); - Inode* inode = vti->GetPointer(); - if (inode != NULL) - inode->SetStale(); - put_vnode(fFsVolume, newID); - - // delete it - result = remove_vnode(fFsVolume, newID); - ASSERT(result == B_OK); - put_vnode(fFsVolume, newID); - - // verify it is gone - result = acquire_vnode(fFsVolume, newID); - ASSERT(result != B_OK); - } + result = TrashStaleNode(newID); + ASSERT(result == B_OK); } return; diff --git a/src/add-ons/kernel/file_systems/nfs4/FileSystem.h b/src/add-ons/kernel/file_systems/nfs4/FileSystem.h index 9dc658311e..e0d6db7869 100644 --- a/src/add-ons/kernel/file_systems/nfs4/FileSystem.h +++ b/src/add-ons/kernel/file_systems/nfs4/FileSystem.h @@ -84,6 +84,7 @@ public: inline mutex& CreateFileLock(); + status_t TrashStaleNode(ino_t ino); void EnsureNoCollision(ino_t newID, const FileHandle& handle); void Dump(void (*xprintf)(const char*, ...) = dprintf);