diff --git a/src/add-ons/kernel/file_systems/nfs4/DirectoryCache.cpp b/src/add-ons/kernel/file_systems/nfs4/DirectoryCache.cpp index 70dc8b1e74..b4a8227ef9 100644 --- a/src/add-ons/kernel/file_systems/nfs4/DirectoryCache.cpp +++ b/src/add-ons/kernel/file_systems/nfs4/DirectoryCache.cpp @@ -237,10 +237,8 @@ DirectoryCache::_LoadSnapshot(bool trash) } 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 %" B_PRIdINO "\n", - current->fNode); + fInode->GetFileSystem()->ServerUnlinkCleanup(current->fNode, fInode, + current->fName); } } delete current; diff --git a/src/add-ons/kernel/file_systems/nfs4/FileSystem.cpp b/src/add-ons/kernel/file_systems/nfs4/FileSystem.cpp index 169b4dfa21..4e86159e7d 100644 --- a/src/add-ons/kernel/file_systems/nfs4/FileSystem.cpp +++ b/src/add-ons/kernel/file_systems/nfs4/FileSystem.cpp @@ -63,8 +63,6 @@ FileSystem::~FileSystem() free(const_cast(fPath[i])); } delete[] fPath; - - delete fRoot; } @@ -267,13 +265,6 @@ FileSystem::GetInode(ino_t id, Inode** _inode) FileInfo fi; status_t result = fInoIdMap.GetFileInfo(&fi, id); - if (result == B_ENTRY_NOT_FOUND) { - bool removed = false; - status_t getRemovedResult = get_vnode_removed(fFsVolume, id, &removed); - ASSERT(getRemovedResult == B_OK); - ASSERT(removed == true); - } - if (result != B_OK) return result; @@ -469,37 +460,36 @@ 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. +/*! Mark this node removed and free it to ensure that no operation will try to use it. + @pre We hold a VFS ref to this node. + @post The ref needs to be released by the caller. */ status_t -FileSystem::TrashStaleNode(ino_t ino) +FileSystem::TrashStaleNode(Inode* inode) { - status_t result = B_OK; - if (acquire_vnode(fFsVolume, ino) == B_OK) { - // we still have a vnode for the stale file present in memory + ino_t ino = inode->ID(); + INFORM("TrashStaleNode %p %" B_PRIdINO "\n", inode, ino); - // 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); + inode->SetStale(); - // delete it - result = remove_vnode(fFsVolume, ino); - ASSERT(result == B_OK); - put_vnode(fFsVolume, ino); + status_t result = remove_vnode(fFsVolume, ino); + ASSERT(result == B_OK); - // verify it is gone - if (acquire_vnode(fFsVolume, ino) == B_OK) - result = B_ERROR; - else - result = B_OK; - } + return result; +} + + +/*! Check whether the server node still has any hard links (including links that the client may be + unaware of). If none, mark the client node removed. +*/ +status_t +FileSystem::TrashIfStale(Inode* inode) +{ + struct stat stat; + status_t result = inode->Stat(&stat, NULL, true); + if (result != B_OK) + result = TrashStaleNode(inode); return result; } @@ -515,16 +505,46 @@ FileSystem::TrashStaleNode(ino_t ino) present, and will be replaced when the caller calls fInoIdMap->AddName. */ void -FileSystem::EnsureNoCollision(ino_t newID, const FileHandle& handle) +FileSystem::EnsureNoCollision(ino_t newId, const FileHandle& handle) { - FileInfo existingInfo; - status_t result = fInoIdMap.GetFileInfo(&existingInfo, newID); - if (result == B_OK && existingInfo.fHandle != 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 = TrashStaleNode(newID); - ASSERT(result == B_OK); + VnodeToInode* existingVti = NULL; + status_t result = get_vnode(fFsVolume, newId, reinterpret_cast(&existingVti)); + if (result == B_OK) { + // We haven't finished creating the new node yet, so whatever get_vnode returned must be + // stale since the server just re-issued its inode number. + ASSERT(handle != existingVti->GetPointer()->fInfo.fHandle); + result = TrashStaleNode(existingVti->GetPointer()); + if (result != B_OK) + INFORM("EnsureNoCollision: Couldn't trash stale node %" B_PRIdINO "\n", newId); + put_vnode(fFsVolume, newId); + } + + return; +} + + +/*! Delete a name from a client-side node after someone else has unlinked that name on the server + side. If that was the last name known to the client, call TrashIfStale. + @param missingName A previously cached file name that is no longer valid. +*/ +void +FileSystem::ServerUnlinkCleanup(ino_t id, Inode* parent, const char* missingName) +{ + FileInfo fileInfo; + status_t result = fInoIdMap.GetFileInfo(&fileInfo, id); + + VnodeToInode* vti = NULL; + result = get_vnode(fFsVolume, id, reinterpret_cast(&vti)); + if (result == B_OK) { + bool noRemainingNames = false; + noRemainingNames = fileInfo.fNames->RemoveName(parent->fInfo.fNames, missingName); + if (noRemainingNames) { + // This client knows of no hard links to this node. + result = TrashIfStale(vti->GetPointer()); + if (result != B_OK) + INFORM("ServerUnlinkCleanup: Couldn't trash stale node %" B_PRIdINO "\n", id); + } + put_vnode(fFsVolume, id); } return; diff --git a/src/add-ons/kernel/file_systems/nfs4/FileSystem.h b/src/add-ons/kernel/file_systems/nfs4/FileSystem.h index e0d6db7869..2f2c94cede 100644 --- a/src/add-ons/kernel/file_systems/nfs4/FileSystem.h +++ b/src/add-ons/kernel/file_systems/nfs4/FileSystem.h @@ -84,8 +84,11 @@ public: inline mutex& CreateFileLock(); - status_t TrashStaleNode(ino_t ino); - void EnsureNoCollision(ino_t newID, const FileHandle& handle); + status_t TrashStaleNode(Inode* inode); + status_t TrashIfStale(Inode* inode); + void EnsureNoCollision(ino_t newId, const FileHandle& handle); + void ServerUnlinkCleanup(ino_t id, Inode* parent, + const char* missingName); void Dump(void (*xprintf)(const char*, ...) = dprintf); diff --git a/src/add-ons/kernel/file_systems/nfs4/Inode.cpp b/src/add-ons/kernel/file_systems/nfs4/Inode.cpp index 6a9b7632e8..f430642c8c 100644 --- a/src/add-ons/kernel/file_systems/nfs4/Inode.cpp +++ b/src/add-ons/kernel/file_systems/nfs4/Inode.cpp @@ -533,7 +533,7 @@ Inode::Access(int mode) status_t -Inode::Stat(struct stat* st, OpenAttrCookie* attr) +Inode::Stat(struct stat* st, OpenAttrCookie* attr, bool revalidate) { ASSERT(st != NULL); @@ -541,7 +541,7 @@ Inode::Stat(struct stat* st, OpenAttrCookie* attr) return GetStat(st, attr); bool cache = fFileSystem->GetConfiguration().fCacheMetadata; - if (!cache) + if (!cache || revalidate) return GetStat(st, NULL); status_t result = fMetaCache.GetStat(st); diff --git a/src/add-ons/kernel/file_systems/nfs4/Inode.h b/src/add-ons/kernel/file_systems/nfs4/Inode.h index 9f7297c2e2..55386dcfe6 100644 --- a/src/add-ons/kernel/file_systems/nfs4/Inode.h +++ b/src/add-ons/kernel/file_systems/nfs4/Inode.h @@ -66,7 +66,7 @@ public: ino_t* oldID = NULL); status_t Stat(struct stat* st, - OpenAttrCookie* attr = NULL); + OpenAttrCookie* attr = NULL, bool revalidate = false); status_t WriteStat(const struct stat* st, uint32 mask, OpenAttrCookie* attr = NULL); diff --git a/src/add-ons/kernel/file_systems/nfs4/kernel_interface.cpp b/src/add-ons/kernel/file_systems/nfs4/kernel_interface.cpp index 43b3f1b2a1..9e92e9c209 100644 --- a/src/add-ons/kernel/file_systems/nfs4/kernel_interface.cpp +++ b/src/add-ons/kernel/file_systems/nfs4/kernel_interface.cpp @@ -259,9 +259,12 @@ static status_t nfs4_unmount(fs_volume* volume) { TRACE("volume = %p\n", volume); + FileSystem* fs = reinterpret_cast(volume->private_volume); RPC::Server* server = fs->Server(); + put_vnode(volume, fs->Root()->ID()); + delete fs; gRPCServerManager->Release(server); @@ -599,24 +602,24 @@ nfs4_unlink(fs_volume* volume, fs_vnode* dir, const char* name) return B_ENTRY_NOT_FOUND; ino_t id; - status_t result = inode->Remove(name, NF4REG, &id); - if (result != B_OK) - return result; - locker.Unlock(); - - result = acquire_vnode(volume, id); + inode->LookUp(name, &id); + VnodeToInode* childVti = NULL; + status_t result = get_vnode(volume, id, reinterpret_cast(&childVti)); if (result == B_OK) { - result = get_vnode(volume, id, reinterpret_cast(&vti)); - ASSERT(result == B_OK); - + ino_t removedId; + status_t result = inode->Remove(name, NF4REG, &removedId); + if (result != B_OK) + return result; + ASSERT(removedId == id); + locker.Unlock(); + if (vti->Unlink(inode->fInfo.fNames, name)) remove_vnode(volume, id); put_vnode(volume, id); - put_vnode(volume, id); } - return B_OK; + return result; } @@ -656,14 +659,10 @@ nfs4_rename(fs_volume* volume, fs_vnode* fromDir, const char* fromName, if (oldID != 0) { // we have overriden an inode - result = acquire_vnode(volume, oldID); + result = get_vnode(volume, oldID, reinterpret_cast(&vti)); if (result == B_OK) { - result = get_vnode(volume, oldID, reinterpret_cast(&vti)); - ASSERT(result == B_OK); if (vti->Unlink(toInode->fInfo.fNames, toName)) remove_vnode(volume, oldID); - - put_vnode(volume, oldID); put_vnode(volume, oldID); } } @@ -741,39 +740,6 @@ nfs4_write_stat(fs_volume* volume, fs_vnode* vnode, const struct stat* stat, } -static status_t -get_new_vnode(fs_volume* volume, ino_t id, VnodeToInode** _vti) -{ - FileSystem* fs = reinterpret_cast(volume->private_volume); - Inode* inode; - VnodeToInode* vti; - - status_t result = acquire_vnode(volume, id); - if (result == B_OK) { - ASSERT(get_vnode(volume, id, reinterpret_cast(_vti)) == B_OK); - unremove_vnode(volume, id); - - // Release after acquire - put_vnode(volume, id); - - vti = *_vti; - - if (vti->Get() == NULL) { - result = fs->GetInode(id, &inode); - if (result != B_OK) { - put_vnode(volume, id); - return result; - } - - vti->Replace(inode); - } - return B_OK; - } - - return get_vnode(volume, id, reinterpret_cast(_vti)); -} - - static status_t nfs4_create(fs_volume* volume, fs_vnode* dir, const char* name, int openMode, int perms, void** _cookie, ino_t* _newVnodeID) @@ -807,7 +773,7 @@ nfs4_create(fs_volume* volume, fs_vnode* dir, const char* name, int openMode, return result; } - result = get_new_vnode(volume, *_newVnodeID, &vti); + result = get_vnode(volume, *_newVnodeID, reinterpret_cast(&vti)); if (result != B_OK) { delete cookie; return result; @@ -1031,15 +997,10 @@ nfs4_remove_dir(fs_volume* volume, fs_vnode* parent, const char* name) if (result != B_OK) return result; - result = acquire_vnode(volume, id); + result = get_vnode(volume, id, reinterpret_cast(&vti)); if (result == B_OK) { - result = get_vnode(volume, id, reinterpret_cast(&vti)); - ASSERT(result == B_OK); - if (vti->Unlink(inode->fInfo.fNames, name)) remove_vnode(volume, id); - - put_vnode(volume, id); put_vnode(volume, id); }