nfs4: Make sure that request are properly sequenced

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

@@ -60,7 +60,7 @@ public:
 
 	inline	uint64				OpenOwner() const;
 	inline	uint32				OpenOwnerSequenceLock();
-	inline	void				OpenOwnerSequenceUnlock(bool increment = true);
+	inline	void				OpenOwnerSequenceUnlock(uint32 sequence);
 
 			FileSystem*			fNext;
 			FileSystem*			fPrev;
@@ -198,10 +198,9 @@ FileSystem::OpenOwnerSequenceLock()
 
 
 inline void
-FileSystem::OpenOwnerSequenceUnlock(bool increment = true)
+FileSystem::OpenOwnerSequenceUnlock(uint32 sequence)
 {
-	if (increment)
-		fOpenOwnerSequence++;
+	fOpenOwnerSequence = sequence;
 	mutex_unlock(&fOpenOwnerLock);
 }
 

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

@@ -454,30 +454,29 @@ NFS4Inode::CreateFile(const char* name, int mode, int perms, OpenState* state,
 
 		result = request.Send();
 		if (result != B_OK) {
-			fFileSystem->OpenOwnerSequenceUnlock(false);
+			fFileSystem->OpenOwnerSequenceUnlock(sequence);
 			return result;
 		}
 
 		ReplyInterpreter& reply = request.Reply();
 
-		if (reply.NFS4Error() == NFS4ERR_BADOWNER) {
-			fFileSystem->OpenOwnerSequenceUnlock();
-			sequence = fFileSystem->OpenOwnerSequenceLock();
+		sequence += IncrementSequence(reply.NFS4Error());
 
+		if (reply.NFS4Error() == NFS4ERR_BADOWNER) {
 			badOwner = true;
 			continue;
 		}
-		if (HandleErrors(reply.NFS4Error(), serv))
+		if (HandleErrors(reply.NFS4Error(), serv, NULL, state, &sequence))
 			continue;
 
-		fFileSystem->OpenOwnerSequenceUnlock();
-
 		reply.PutFH();
 
 		result = reply.Open(state->fStateID, &state->fStateSeq, &confirm,
 			delegation, changeInfo);
-		if (result != B_OK)
+		if (result != B_OK) {
+			fFileSystem->OpenOwnerSequenceUnlock(sequence);
 			return result;
+		}
 
 		reply.GetFH(handle);
 
@@ -485,8 +484,10 @@ NFS4Inode::CreateFile(const char* name, int mode, int perms, OpenState* state,
 			AttrValue* values;
 			uint32 count;
 			result = reply.GetAttr(&values, &count);
-			if (result != B_OK)
+			if (result != B_OK) {
+				fFileSystem->OpenOwnerSequenceUnlock(sequence);
 				return result;
+			}
 
 			*fileID = values[0].fData.fValue64;
 
@@ -500,9 +501,10 @@ NFS4Inode::CreateFile(const char* name, int mode, int perms, OpenState* state,
 	state->fOpened = true;
 
 	if (confirm)
-		return ConfirmOpen(*handle, state);
+		result = ConfirmOpen(*handle, state, &sequence);
 
-	return B_OK;
+	fFileSystem->OpenOwnerSequenceUnlock(sequence);
+	return result;
 }
 
 
@@ -547,7 +549,7 @@ NFS4Inode::OpenFile(OpenState* state, int mode, OpenDelegationData* delegation)
 
 		result = request.Send();
 		if (result != B_OK) {
-			fFileSystem->OpenOwnerSequenceUnlock(false);
+			fFileSystem->OpenOwnerSequenceUnlock(sequence);
 			return result;
 		}
 
@@ -556,7 +558,7 @@ NFS4Inode::OpenFile(OpenState* state, int mode, OpenDelegationData* delegation)
 		if (HandleErrors(reply.NFS4Error(), serv, NULL, state, &sequence))
 			continue;
 
-		fFileSystem->OpenOwnerSequenceUnlock();
+		sequence += IncrementSequence(reply.NFS4Error());
 
 		// Verify if the file we want to open is the file this Inode
 		// represents.
@@ -564,9 +566,17 @@ NFS4Inode::OpenFile(OpenState* state, int mode, OpenDelegationData* delegation)
 			fFileSystem->ExpireType() == FH4_PERSISTENT) {
 			reply.PutFH();
 			result = reply.LookUp();
-			if (result != B_OK)
+			if (result != B_OK) {
+				fFileSystem->OpenOwnerSequenceUnlock(sequence);
 				return result;
+			}
+
 			result = reply.Verify();
+			if (result != B_OK) {
+				fFileSystem->OpenOwnerSequenceUnlock(sequence);
+				return result;
+			}
+
 			if (result != B_OK && reply.NFS4Error() == NFS4ERR_NOT_SAME)
 				return B_ENTRY_NOT_FOUND;
 			else if (result != B_OK)
@@ -580,8 +590,10 @@ NFS4Inode::OpenFile(OpenState* state, int mode, OpenDelegationData* delegation)
 		FileHandle handle;
 		reply.GetFH(&handle);
 
-		if (result != B_OK)
+		if (result != B_OK) {
+			fFileSystem->OpenOwnerSequenceUnlock(sequence);
 			return result;
+		}
 
 		break;
 	} while (true);
@@ -589,9 +601,10 @@ NFS4Inode::OpenFile(OpenState* state, int mode, OpenDelegationData* delegation)
 	state->fOpened = true;
 
 	if (confirm)
-		return ConfirmOpen(fInfo.fHandle, state);
+		result =  ConfirmOpen(fInfo.fHandle, state, &sequence);
 
-	return B_OK;
+	fFileSystem->OpenOwnerSequenceUnlock(sequence);
+	return result;
 }
 
 
@@ -617,7 +630,7 @@ NFS4Inode::OpenAttr(OpenState* state, const char* name, int mode,
 
 		result = request.Send();
 		if (result != B_OK) {
-			fFileSystem->OpenOwnerSequenceUnlock(false);
+			fFileSystem->OpenOwnerSequenceUnlock(sequence);
 			return result;
 		}
 
@@ -626,7 +639,7 @@ NFS4Inode::OpenAttr(OpenState* state, const char* name, int mode,
 		if (HandleErrors(reply.NFS4Error(), serv, NULL, state))
 			continue;
 
-		fFileSystem->OpenOwnerSequenceUnlock();
+		sequence += IncrementSequence(reply.NFS4Error());
 
 		reply.PutFH();
 		result = reply.Open(state->fStateID, &state->fStateSeq, &confirm,
@@ -634,8 +647,10 @@ NFS4Inode::OpenAttr(OpenState* state, const char* name, int mode,
 
 		reply.GetFH(&state->fInfo.fHandle);
 
-		if (result != B_OK)
+		if (result != B_OK) {
+			fFileSystem->OpenOwnerSequenceUnlock(sequence);
 			return result;
+		}
 
 		break;
 	} while (true);
@@ -643,9 +658,10 @@ NFS4Inode::OpenAttr(OpenState* state, const char* name, int mode,
 	state->fOpened = true;
 
 	if (confirm)
-		return ConfirmOpen(fInfo.fHandle, state);
+		result = ConfirmOpen(fInfo.fHandle, state, &sequence);
 
-	return B_OK;
+	fFileSystem->OpenOwnerSequenceUnlock(sequence);
+	return result;
 }
 
 
@@ -1035,27 +1051,29 @@ NFS4Inode::AcquireLock(OpenFileCookie* cookie, LockInfo* lockInfo, bool wait)
 
 		status_t result = request.Send();
 		if (result != B_OK) {
-			fFileSystem->OpenOwnerSequenceUnlock(false);
+			fFileSystem->OpenOwnerSequenceUnlock(sequence);
 			return result;
 		}
 
 		ReplyInterpreter &reply = request.Reply();
 
+		sequence += IncrementSequence(reply.NFS4Error());
+
 		reply.PutFH();
 		result = reply.Lock(lockInfo);
 
 		ownerLocker.Unlock();
 		if (wait && reply.NFS4Error() == NFS4ERR_DENIED) {
-			fFileSystem->OpenOwnerSequenceUnlock();
+			fFileSystem->OpenOwnerSequenceUnlock(sequence);
 			snooze_etc(sSecToBigTime(5), B_SYSTEM_TIMEBASE,
 				B_RELATIVE_TIMEOUT);
 			sequence = fFileSystem->OpenOwnerSequenceLock();
 			continue;
 		}
-		if (HandleErrors(reply.NFS4Error(), serv, cookie))
+		if (HandleErrors(reply.NFS4Error(), serv, cookie, NULL, &sequence))
 			continue;
 
-		fFileSystem->OpenOwnerSequenceUnlock();
+		fFileSystem->OpenOwnerSequenceUnlock(sequence);
 		if (result != B_OK)
 			return result;
 

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

@@ -30,37 +30,63 @@ NFS4Object::HandleErrors(uint32 nfs4Error, RPC::Server* serv,
 		// server needs more time, we need to wait
 		case NFS4ERR_LOCKED:
 		case NFS4ERR_DELAY:
+			if (sequence != NULL)
+				fFileSystem->OpenOwnerSequenceUnlock(*sequence);
+
 			if (cookie == NULL) {
 				snooze_etc(sSecToBigTime(5), B_SYSTEM_TIMEBASE,
 					B_RELATIVE_TIMEOUT);
+
+				if (sequence != NULL)
+					*sequence = fFileSystem->OpenOwnerSequenceLock();
+
 				return true;
 			} else if ((cookie->fMode & O_NONBLOCK) == 0) {
 				status_t result = acquire_sem_etc(cookie->fSnoozeCancel, 1,
 					B_RELATIVE_TIMEOUT, sSecToBigTime(5));
+
+				if (sequence != NULL)
+					*sequence = fFileSystem->OpenOwnerSequenceLock();
+
 				if (result != B_TIMED_OUT) {
 					release_sem(cookie->fSnoozeCancel);
-					return false;
+					return true;
 				}
-				return true;
+				return false;
 			}
+
+			if (sequence != NULL)
+				*sequence = fFileSystem->OpenOwnerSequenceLock();
 			return false;
 
 		// server is in grace period, we need to wait
 		case NFS4ERR_GRACE:
 			leaseTime = fFileSystem->NFSServer()->LeaseTime();
+			if (sequence != NULL)
+				fFileSystem->OpenOwnerSequenceUnlock(*sequence);
+
 			if (cookie == NULL) {
 				snooze_etc(sSecToBigTime(leaseTime) / 3, B_SYSTEM_TIMEBASE,
 					B_RELATIVE_TIMEOUT);
+				if (sequence != NULL)
+					*sequence = fFileSystem->OpenOwnerSequenceLock();
 				return true;
 			} else if ((cookie->fMode & O_NONBLOCK) == 0) {
 				status_t result = acquire_sem_etc(cookie->fSnoozeCancel, 1,
 					B_RELATIVE_TIMEOUT, sSecToBigTime(leaseTime) / 3);
+
+				if (sequence != NULL)
+					*sequence = fFileSystem->OpenOwnerSequenceLock();
+
 				if (result != B_TIMED_OUT) {
 					release_sem(cookie->fSnoozeCancel);
-					return false;
+					return true;
 				}
-				return true;
+				return false;
 			}
+
+			if (sequence != NULL)
+				*sequence = fFileSystem->OpenOwnerSequenceLock();
 			return false;
 
 		// server has rebooted, reclaim share and try again
@@ -68,13 +94,12 @@ NFS4Object::HandleErrors(uint32 nfs4Error, RPC::Server* serv,
 		case NFS4ERR_STALE_STATEID:
 			if (state != NULL) {
 				if (sequence != NULL)
-					fFileSystem->OpenOwnerSequenceUnlock(false);
+					fFileSystem->OpenOwnerSequenceUnlock(*sequence);
 
 				fFileSystem->NFSServer()->ServerRebooted(state->fClientID);
-dprintf("returned rebooted\n");
 				if (sequence != NULL)
 					*sequence = fFileSystem->OpenOwnerSequenceLock();
-dprintf("locked again\n");
+
 				return true;
 			}
 			return false;
@@ -106,9 +131,9 @@ dprintf("locked again\n");
 
 
 status_t
-NFS4Object::ConfirmOpen(const FileHandle& fh, OpenState* state)
+NFS4Object::ConfirmOpen(const FileHandle& fh, OpenState* state,
+	uint32* sequence)
 {
-	uint32 sequence = fFileSystem->OpenOwnerSequenceLock();
 	do {
 		RPC::Server* serv = fFileSystem->Server();
 		Request request(serv);
@@ -116,21 +141,19 @@ NFS4Object::ConfirmOpen(const FileHandle& fh, OpenState* state)
 		RequestBuilder& req = request.Builder();
 
 		req.PutFH(fh);
-		req.OpenConfirm(sequence, state->fStateID, state->fStateSeq);
+		req.OpenConfirm(*sequence, state->fStateID, state->fStateSeq);
 
 		status_t result = request.Send();
-		if (result != B_OK) {
-			fFileSystem->OpenOwnerSequenceUnlock(false);
+		if (result != B_OK)
 			return result;
-		}
 
 		ReplyInterpreter& reply = request.Reply();
 
+		*sequence += IncrementSequence(reply.NFS4Error());
+
 		if (HandleErrors(reply.NFS4Error(), serv, NULL, state))
 			continue;
 
-		fFileSystem->OpenOwnerSequenceUnlock();
-
 		reply.PutFH();
 		result = reply.OpenConfirm(&state->fStateSeq);
 		if (result != B_OK)
@@ -140,3 +163,16 @@ NFS4Object::ConfirmOpen(const FileHandle& fh, OpenState* state)
 	} while (true);
 }
 
+
+uint32
+NFS4Object::IncrementSequence(uint32 error)
+{
+	if (error != NFS4ERR_STALE_CLIENTID && error != NFS4ERR_STALE_STATEID
+		&& error != NFS4ERR_BAD_STATEID && error != NFS4ERR_BAD_SEQID
+		&& error != NFS4ERR_BADXDR && error != NFS4ERR_RESOURCE
+		&& error != NFS4ERR_NOFILEHANDLE)
+		return 1;
+
+	return 0;
+}
+

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

@@ -18,14 +18,17 @@ class OpenState;
 
 class NFS4Object {
 public:
-	bool		HandleErrors(uint32 nfs4Error, RPC::Server* serv,
-					OpenStateCookie* cookie = NULL, OpenState* state = NULL,
-					uint32* sequence = NULL);
+			bool		HandleErrors(uint32 nfs4Error, RPC::Server* serv,
+							OpenStateCookie* cookie = NULL,
+							OpenState* state = NULL, uint32* sequence = NULL);
 
-	status_t	ConfirmOpen(const FileHandle& fileHandle, OpenState* state);
+			status_t	ConfirmOpen(const FileHandle& fileHandle,
+							OpenState* state, uint32* sequence);
 
-	FileInfo	fInfo;
-	FileSystem*	fFileSystem;
+	static	uint32		IncrementSequence(uint32 error);
+
+			FileInfo	fInfo;
+			FileSystem*	fFileSystem;
 };
 
 

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

@@ -143,10 +143,10 @@ OpenState::Reclaim(uint64 newClientID)
 	if (fClientID == newClientID)
 		return B_OK;
 	fClientID = newClientID;
-dprintf("reclaim start\n");
+
 	_ReclaimOpen(newClientID);
 	_ReclaimLocks(newClientID);
-dprintf("reclaim end\n");
+
 	return B_OK;
 }
 
@@ -154,13 +154,12 @@ dprintf("reclaim end\n");
 status_t
 OpenState::_ReclaimOpen(uint64 newClientID)
 {
-	dprintf("reclaim %s\n", fInfo.fName);
-
 	bool confirm;
 	OpenDelegationData delegation;
 	delegation.fType = OPEN_DELEGATE_NONE;
 	delegation.fRecall = false;
 
+	status_t result;
 	uint32 sequence = fFileSystem->OpenOwnerSequenceLock();
 	OpenDelegation delegType = fDelegation != NULL ? fDelegation->Type()
 		: OPEN_DELEGATE_NONE;
@@ -174,24 +173,26 @@ OpenState::_ReclaimOpen(uint64 newClientID)
 			OPEN4_NOCREATE, fFileSystem->OpenOwner(), NULL, NULL, 0, false,
 			delegType);
 
-		status_t result = request.Send();
+		result = request.Send();
 		if (result != B_OK) {
-			fFileSystem->OpenOwnerSequenceUnlock(false);
+			fFileSystem->OpenOwnerSequenceUnlock(sequence);
 			return result;
 		}
 
 		ReplyInterpreter& reply = request.Reply();
 
+		sequence += IncrementSequence(reply.NFS4Error());
+
 		if (HandleErrors(reply.NFS4Error(), server, NULL, NULL, &sequence))
 			continue;
 
- 		fFileSystem->OpenOwnerSequenceUnlock();
-
 		reply.PutFH();
 
 		result = reply.Open(fStateID, &fStateSeq, &confirm, &delegation);
-		if (result != B_OK)
+		if (result != B_OK) {
+	 		fFileSystem->OpenOwnerSequenceUnlock(sequence);
 			return result;
+		}
 
 		break;
 	} while (true);
@@ -207,17 +208,16 @@ OpenState::_ReclaimOpen(uint64 newClientID)
 	}
 
 	if (confirm)
-		return ConfirmOpen(fInfo.fHandle, this);
+		result = ConfirmOpen(fInfo.fHandle, this, &sequence);
 
-	return B_OK;
+ 	fFileSystem->OpenOwnerSequenceUnlock(sequence);
+	return result;
 }
 
 
 status_t
 OpenState::_ReclaimLocks(uint64 newClientID)
 {
-	dprintf("reclaim locks %s\n", fInfo.fName);
-
 	MutexLocker _(fLocksLock);
 	LockInfo* linfo = fLocks;
 	while (linfo != NULL) {
@@ -279,15 +279,17 @@ OpenState::Close()
 
 		status_t result = request.Send();
 		if (result != B_OK) {
-			fFileSystem->OpenOwnerSequenceUnlock(false);
+			fFileSystem->OpenOwnerSequenceUnlock(sequence);
 			return result;
 		}
 
 		ReplyInterpreter& reply = request.Reply();
 
+		sequence += IncrementSequence(reply.NFS4Error());
+
 		if (HandleErrors(reply.NFS4Error(), serv, NULL, this, &sequence))
 			continue;
- 		fFileSystem->OpenOwnerSequenceUnlock();
+ 		fFileSystem->OpenOwnerSequenceUnlock(sequence);
 
 		reply.PutFH();