Try and make the nfsrv_localunlock() function in the experimental

NFSv4 server more readable. Mostly changes to comments, but a
case of >= is changed to >, since == can never happen. Also, I've
added a couple of KASSERT()s and a slight optimization, since
once the "else if" case happens, subsequent locks in the list can't
have any effect. None of these changes fixes any known bug.

MFC after:	2 weeks

1 files changed, 36 insertions, 8 deletions

diff --git a/sys/fs/nfsserver/nfs_nfsdstate.c b/sys/fs/nfsserver/nfs_nfsdstate.c
index 86b866e..908c028 100644
--- a/sys/fs/nfsserver/nfs_nfsdstate.c
+++ b/sys/fs/nfsserver/nfs_nfsdstate.c
@@ -4957,31 +4957,56 @@ nfsrv_locallock(vnode_t vp, struct nfslockfile *lfp, int flags,
 
 /*
  * Local lock unlock. Unlock all byte ranges that are no longer locked
- * by NFSv4.
+ * by NFSv4. To do this, unlock any subranges of first-->end that
+ * do not overlap with the byte ranges of any lock in the lfp->lf_lock
+ * list. This list has all locks for the file held by other
+ * <clientid, lockowner> tuples. The list is ordered by increasing
+ * lo_first value, but may have entries that overlap each other, for
+ * the case of read locks.
  */
 static void
 nfsrv_localunlock(vnode_t vp, struct nfslockfile *lfp, uint64_t init_first,
     uint64_t init_end, NFSPROC_T *p)
 {
 	struct nfslock *lop;
-
-	uint64_t first, end;
+	uint64_t first, end, prevfirst;
 
 	first = init_first;
 	end = init_end;
 	while (first < init_end) {
 		/* Loop through all nfs locks, adjusting first and end */
+		prevfirst = 0;
 		LIST_FOREACH(lop, &lfp->lf_lock, lo_lckfile) {
+			KASSERT(prevfirst <= lop->lo_first,
+			    ("nfsv4 locks out of order"));
+			KASSERT(lop->lo_first < lop->lo_end,
+			    ("nfsv4 bogus lock"));
+			prevfirst = lop->lo_first;
 			if (first >= lop->lo_first &&
 			    first < lop->lo_end)
-				/* Overlaps initial part */
+				/*
+				 * Overlaps with initial part, so trim
+				 * off that initial part by moving first past
+				 * it.
+				 */
 				first = lop->lo_end;
 			else if (end > lop->lo_first &&
-			    lop->lo_first >= first)
-				/* Begins before end and past first */
+			    lop->lo_first > first) {
+				/*
+				 * This lock defines the end of the
+				 * segment to unlock, so set end to the
+				 * start of it and break out of the loop.
+				 */
 				end = lop->lo_first;
+				break;
+			}
 			if (first >= end)
-				/* shrunk to 0 so this iteration is done */
+				/*
+				 * There is no segment left to do, so
+				 * break out of this loop and then exit
+				 * the outer while() since first will be set
+				 * to end, which must equal init_end here.
+				 */
 				break;
 		}
 		if (first < end) {
@@ -4991,7 +5016,10 @@ nfsrv_localunlock(vnode_t vp, struct nfslockfile *lfp, uint64_t init_first,
 			nfsrv_locallock_commit(lfp, NFSLCK_UNLOCK,
 			    first, end);
 		}
-		/* and move on to the rest of the range */
+		/*
+		 * Now move past this segment and look for any further
+		 * segment in the range, if there is one.
+		 */
 		first = end;
 		end = init_end;
 	}