nfs4: Remove stale nodes when their ino is reused

* Add FileSystem::EnsureNoCollision to check for a stale node with
  the given inode number, and remove it if it exists.
* Call EnsureNoCollision from Inode::CreateObject and
  Inode::CreateState to cover creation of directory files and regular
  files, respectively.
* Stop using the entry cache.  Instead, make use of the driver's
  DirectoryCache in the lookup hook.
* Fixes #16924.

When a shared file is deleted, and no clients have the file open, the
server considers the file ID number of the deleted file to be eligible
for reassignment to new files.  If the deletion was performed by the
server or by some other client, the Haiku client does't know the file
was deleted, and still may hold a vnode for it.  This interferes with
the creation of a new file by the client, when the file ID / ino of
the new file is the same as that of the stale node.

The reason for disabling the entry cache is to give the driver a
chance to check the server for changes in name-to-ino mapping when the
VFS function lookup_dir_entry is called.  At least in its current
form, the driver isn't able to deal with outdated entry cache
information.  For example, if the Haiku client has added temp.txt to
its entry cache, and then temp.txt is deleted by the server, running
'nano temp.txt' on the client will attempt to open the (non-existent)
file instead of creating it.

Change-Id: Ic3ec21d18c8616fcfeb9234aafb0e647746d23cd
Reviewed-on: https://review.haiku-os.org/c/haiku/+/9068
Reviewed-by: waddlesplash <[email protected]>

src/add-ons/kernel/file_systems/nfs4/DirectoryCache.cpp

@@ -110,8 +110,6 @@ DirectoryCache::Trash()
 {
 	while (!fNameCache.IsEmpty()) {
 		NameCacheEntry* current = fNameCache.RemoveHead();
-		entry_cache_remove(fInode->GetFileSystem()->DevId(), fInode->ID(),
-			current->fName);
 		delete current;
 	}
 
@@ -149,11 +147,6 @@ DirectoryCache::AddEntry(const char* name, ino_t node, bool created)
 		fDirectoryCache->fEntries.Add(entry);
 	}
 
-	if (!fAttrDir) {
-		return entry_cache_add(fInode->GetFileSystem()->DevId(), fInode->ID(),
-			name, node);
-	}
-
 	return B_OK;
 }
 
@@ -194,11 +187,6 @@ DirectoryCache::RemoveEntry(const char* name)
 			current = iterator.Next();
 		}
 	}
-
-	if (!fAttrDir) {
-		entry_cache_remove(fInode->GetFileSystem()->DevId(), fInode->ID(),
-			name);
-	}
 }
 
 

src/add-ons/kernel/file_systems/nfs4/FileSystem.cpp

@@ -17,6 +17,7 @@
 
 #include "Request.h"
 #include "RootInode.h"
+#include "VnodeToInode.h"
 
 
 #define ERROR(x...) dprintf("nfs4: " x)
@@ -142,8 +143,8 @@ GetInodeNames(const char** root, const char* _path)
 
 
 status_t
-FileSystem::Mount(FileSystem** _fs, RPC::Server* serv, const char* serverName,
-	const char* fsPath, dev_t id, const MountConfiguration& configuration)
+FileSystem::Mount(FileSystem** _fs, RPC::Server* serv, const char* serverName, const char* fsPath,
+	fs_volume* volume, const MountConfiguration& configuration)
 {
 	CALLED();
 
@@ -223,8 +224,9 @@ FileSystem::Mount(FileSystem** _fs, RPC::Server* serv, const char* serverName,
 	FileInfo fi;
 
 	fs->fServer = serv;
-	fs->fDevId = id;
+	fs->fDevId = volume->id;
 	fs->fFsId = *fsid;
+	fs->fFsVolume = volume;
 
 	fi.fHandle = fh;
 
@@ -474,6 +476,52 @@ FileSystem::GetDelegation(const FileHandle& handle)
 }
 
 
+/*! Used when creating a file to check for a stale node with the same ino. If it exists,
+	the stale node is deleted.
+	@param newID The file ID assigned by the server to a file now being created.
+	@param handle The handle assigned by the server to the same file.
+	@pre The caller has not yet updated fInoIdMap with the FileInfo of the file that we are
+	creating. The VFS list of vnodes has also not been updated yet.
+	@post Any stale node object with this ID is gone. Any stale entry in fInoIdMap is still
+	present, and will be replaced when the caller calls fInoIdMap->AddName.
+*/
+void
+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 = 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<void**>(&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);
+		}
+	}
+
+	return;
+}
+
+
 status_t
 FileSystem::_ParsePath(RequestBuilder& req, uint32& count, const char* _path)
 {

src/add-ons/kernel/file_systems/nfs4/FileSystem.h

@@ -9,6 +9,8 @@
 #define FILESYSTEM_H
 
 
+#include <fs_interface.h>
+
 #include "Delegation.h"
 #include "InodeIdMap.h"
 #include "NFS4Defs.h"
@@ -33,7 +35,7 @@ class FileSystem : public DoublyLinkedListLinkImpl<FileSystem> {
 public:
 	static	status_t			Mount(FileSystem** pfs, RPC::Server* serv,
 									const char* serverName, const char* path,
-									dev_t id,
+									fs_volume* volume,
 									const MountConfiguration& configuration);
 								~FileSystem();
 
@@ -78,6 +80,8 @@ public:
 	inline	const MountConfiguration&	GetConfiguration();
 
 	inline	mutex&				CreateFileLock();
+
+			void				EnsureNoCollision(ino_t newID, const FileHandle& handle);
 private:
 								FileSystem(const MountConfiguration& config);
 
@@ -115,6 +119,8 @@ private:
 			InodeIdMap			fInoIdMap;
 
 			MountConfiguration	fConfiguration;
+
+			fs_volume*			fFsVolume;
 };
 
 

src/add-ons/kernel/file_systems/nfs4/Inode.cpp

@@ -32,7 +32,8 @@ Inode::Inode()
 	fOpenState(NULL),
 	fWriteDirty(false),
 	fAIOWait(create_sem(1, NULL)),
-	fAIOCount(0)
+	fAIOCount(0),
+	fStale(false)
 {
 	rw_lock_init(&fDelegationLock, NULL);
 	mutex_init(&fStateLock, NULL);
@@ -195,10 +196,39 @@ Inode::LookUp(const char* name, ino_t* id)
 	if (fType != NF4DIR)
 		return B_NOT_A_DIRECTORY;
 
+	// attempt to get the id from the DirectoryCache
+	fCache->Lock();
+	status_t result = fCache->Revalidate();
+		// At times, this will do a full readdir, in order to sync the DirectoryCache
+		// with server changes.  However, if the directory contents have not changed since
+		// the last readdir, it will either perform no RPC, or an RPC that just checks
+		// FATTR4_CHANGE.
+	if (result == B_OK) {
+		SinglyLinkedList<NameCacheEntry>& entriesList = fCache->EntriesList();
+		NameCacheEntry* entry = entriesList.Head();
+		while (entry != NULL) {
+			if (strcmp(name, entry->fName) == 0)
+				break;
+			entry = entriesList.GetNext(entry);
+		}
+		if (entry != NULL) {
+			// we are skipping ChildAdded(); verify that it is not needed because the InoIdMap
+			// already has this entry
+			FileInfo info;
+			result = fFileSystem->InoIdMap()->GetFileInfo(&info, entry->fNode);
+			ASSERT(result == B_OK);
+
+			*id = entry->fNode;
+			fCache->Unlock();
+			return B_OK;
+		}
+	}
+	fCache->Unlock();
+
 	uint64 change;
 	uint64 fileID;
 	FileHandle handle;
-	status_t result = NFS4Inode::LookUp(name, &change, &fileID, &handle);
+	result = NFS4Inode::LookUp(name, &change, &fileID, &handle);
 	if (result != B_OK)
 		return result;
 
@@ -272,6 +302,7 @@ Inode::Remove(const char* name, FileType type, ino_t* id)
 
 	ChangeInfo changeInfo;
 	uint64 fileID;
+
 	status_t result = NFS4Inode::RemoveObject(name, type, &changeInfo, &fileID);
 	if (result != B_OK)
 		return result;
@@ -418,6 +449,8 @@ Inode::CreateObject(const char* name, const char* path, int mode, FileType type,
 	if (result != B_OK)
 		return result;
 
+	fFileSystem->EnsureNoCollision(FileIdToInoT(fileID), handle);
+
 	fFileSystem->Root()->MakeInfoInvalid();
 
 	result = ChildAdded(name, fileID, handle);

src/add-ons/kernel/file_systems/nfs4/Inode.h

@@ -118,6 +118,9 @@ public:
 					void		BeginAIOOp();
 					void		EndAIOOp();
 	inline			void		WaitAIOComplete();
+
+	inline			void		SetStale(bool stale = true);
+	inline			bool		IsStale() const;
 protected:
 								Inode();
 
@@ -168,6 +171,8 @@ private:
 					sem_id		fAIOWait;
 					uint32		fAIOCount;
 					mutex		fAIOLock;
+
+					bool		fStale;
 };
 
 
@@ -255,5 +260,19 @@ Inode::GetOpenState()
 }
 
 
+inline void
+Inode::SetStale(bool stale)
+{
+	fStale = stale;
+}
+
+
+inline bool
+Inode::IsStale() const
+{
+	return fStale;
+}
+
+
 #endif	// INODE_H
 

src/add-ons/kernel/file_systems/nfs4/InodeRegular.cpp

@@ -40,6 +40,8 @@ Inode::CreateState(const char* name, int mode, int perms, OpenState* state,
 	fileInfo.fFileId = fileID;
 	fileInfo.fHandle = handle;
 
+	fFileSystem->EnsureNoCollision(FileIdToInoT(fileID), handle);
+
 	fFileSystem->InoIdMap()->AddName(fileInfo, fInfo.fNames, name,
 		FileIdToInoT(fileID));
 

src/add-ons/kernel/file_systems/nfs4/kernel_interface.cpp

@@ -191,10 +191,9 @@ nfs4_mount(fs_volume* volume, const char* device, uint32 flags,
 		ERROR("Unable to Acquire RPCServerManager!\n");
 		return result;
 	}
-	
+
 	FileSystem* fs;
-	result = FileSystem::Mount(&fs, server, serverName, path, volume->id,
-		config);
+	result = FileSystem::Mount(&fs, server, serverName, path, volume, config);
 	if (result != B_OK) {
 		ERROR("Error mounting filesystem: %s\n", strerror(result));
 		gRPCServerManager->Release(server);
@@ -347,9 +346,17 @@ nfs4_remove_vnode(fs_volume* volume, fs_vnode* vnode, bool reenter)
 	VnodeToInode* vti = reinterpret_cast<VnodeToInode*>(vnode->private_node);
 	TRACE("volume = %p, vnode = %" B_PRIi64 "\n", volume, vti->ID());
 
-	if (fs->Root() == vti->GetPointer())
+	Inode* node = vti->GetPointer();
+
+	if (node == fs->Root())
 		return B_OK;
 
+	if (node != NULL && node->IsStale()) {
+		// in the case of a stale node, VnodeToInode::Unlink was never called by the client,
+		// so the Inode hasn't been deleted yet
+		vti->Clear();
+	}
+
 	ASSERT(vti->GetPointer() == NULL);
 	delete vti;