Revert "kern/vfs_vnops.c: generalize the lock primitive for file foffset"

Temporarily back this out to fix the tree while I try 16.0 snapshot
builds.  It will come back once D52626 lands.

This reverts commit 94a0f9f.

Discussed with:	kib

Author: cperciva

sys/kern/vfs_vnops.c

@@ -798,82 +798,58 @@ vn_rdwr_inchunks(enum uio_rw rw, struct vnode *vp, void *base, size_t len,
 }
 
 #if OFF_MAX <= LONG_MAX
-static void
-file_v_lock(struct file *fp, short lock_bit, short lock_wait_bit)
+off_t
+foffset_lock(struct file *fp, int flags)
 {
-	short *flagsp;
+	volatile short *flagsp;
+	off_t res;
 	short state;
 
-	flagsp = &fp->f_vflags;
-	state = atomic_load_16(flagsp);
-	if ((state & lock_bit) == 0 &&
-	    atomic_cmpset_acq_16(flagsp, state, state | lock_bit))
-		return;
+	KASSERT((flags & FOF_OFFSET) == 0, ("FOF_OFFSET passed"));
+
+	if ((flags & FOF_NOLOCK) != 0)
+		return (atomic_load_long(&fp->f_offset));
 
-	sleepq_lock(flagsp);
+	/*
+	 * According to McKusick the vn lock was protecting f_offset here.
+	 * It is now protected by the FOFFSET_LOCKED flag.
+	 */
+	flagsp = &fp->f_vnread_flags;
+	if (atomic_cmpset_acq_16(flagsp, 0, FOFFSET_LOCKED))
+		return (atomic_load_long(&fp->f_offset));
+
+	sleepq_lock(&fp->f_vnread_flags);
 	state = atomic_load_16(flagsp);
 	for (;;) {
-		if ((state & lock_bit) == 0) {
+		if ((state & FOFFSET_LOCKED) == 0) {
 			if (!atomic_fcmpset_acq_16(flagsp, &state,
-			    state | lock_bit))
+			    FOFFSET_LOCKED))
 				continue;
 			break;
 		}
-		if ((state & lock_wait_bit) == 0) {
+		if ((state & FOFFSET_LOCK_WAITING) == 0) {
 			if (!atomic_fcmpset_acq_16(flagsp, &state,
-			    state | lock_wait_bit))
+			    state | FOFFSET_LOCK_WAITING))
 				continue;
 		}
 		DROP_GIANT();
-		sleepq_add(flagsp, NULL, "vofflock", 0, 0);
-		sleepq_wait(flagsp, PRI_MAX_KERN);
+		sleepq_add(&fp->f_vnread_flags, NULL, "vofflock", 0, 0);
+		sleepq_wait(&fp->f_vnread_flags, PRI_MAX_KERN);
 		PICKUP_GIANT();
-		sleepq_lock(flagsp);
+		sleepq_lock(&fp->f_vnread_flags);
 		state = atomic_load_16(flagsp);
 	}
-	sleepq_release(flagsp);
-}
-
-static void
-file_v_unlock(struct file *fp, short lock_bit, short lock_wait_bit)
-{
-	short *flagsp;
-	short state;
-
-	flagsp = &fp->f_vflags;
-	state = atomic_load_16(flagsp);
-	if ((state & lock_wait_bit) == 0 &&
-	    atomic_cmpset_rel_16(flagsp, state, state & ~lock_bit))
-		return;
-
-	sleepq_lock(flagsp);
-	MPASS((*flagsp & lock_bit) != 0);
-	MPASS((*flagsp & lock_wait_bit) != 0);
-	atomic_clear_16(flagsp, lock_bit | lock_wait_bit);
-	sleepq_broadcast(flagsp, SLEEPQ_SLEEP, 0, 0);
-	sleepq_release(flagsp);
-}
-
-off_t
-foffset_lock(struct file *fp, int flags)
-{
-	KASSERT((flags & FOF_OFFSET) == 0, ("FOF_OFFSET passed"));
-
-	if ((flags & FOF_NOLOCK) == 0) {
-		file_v_lock(fp, FILE_V_FOFFSET_LOCKED,
-		    FILE_V_FOFFSET_LOCK_WAITING);
-	}
-
-	/*
-	 * According to McKusick the vn lock was protecting f_offset here.
-	 * It is now protected by the FOFFSET_LOCKED flag.
-	 */
-	return (atomic_load_long(&fp->f_offset));
+	res = atomic_load_long(&fp->f_offset);
+	sleepq_release(&fp->f_vnread_flags);
+	return (res);
 }
 
 void
 foffset_unlock(struct file *fp, off_t val, int flags)
 {
+	volatile short *flagsp;
+	short state;
+
 	KASSERT((flags & FOF_OFFSET) == 0, ("FOF_OFFSET passed"));
 
 	if ((flags & FOF_NOUPDATE) == 0)
@@ -883,10 +859,21 @@ foffset_unlock(struct file *fp, off_t val, int flags)
 	if ((flags & FOF_NEXTOFF_W) != 0)
 		fp->f_nextoff[UIO_WRITE] = val;
 
-	if ((flags & FOF_NOLOCK) == 0) {
-		file_v_unlock(fp, FILE_V_FOFFSET_LOCKED,
-		    FILE_V_FOFFSET_LOCK_WAITING);
-	}
+	if ((flags & FOF_NOLOCK) != 0)
+		return;
+
+	flagsp = &fp->f_vnread_flags;
+	state = atomic_load_16(flagsp);
+	if ((state & FOFFSET_LOCK_WAITING) == 0 &&
+	    atomic_cmpset_rel_16(flagsp, state, 0))
+		return;
+
+	sleepq_lock(&fp->f_vnread_flags);
+	MPASS((fp->f_vnread_flags & FOFFSET_LOCKED) != 0);
+	MPASS((fp->f_vnread_flags & FOFFSET_LOCK_WAITING) != 0);
+	fp->f_vnread_flags = 0;
+	sleepq_broadcast(&fp->f_vnread_flags, SLEEPQ_SLEEP, 0, 0);
+	sleepq_release(&fp->f_vnread_flags);
 }
 
 static off_t
@@ -895,35 +882,7 @@ foffset_read(struct file *fp)
 
 	return (atomic_load_long(&fp->f_offset));
 }
-
-#else	/* OFF_MAX <= LONG_MAX */
-
-static void
-file_v_lock_mtxp(struct file *fp, struct mtx *mtxp, short lock_bit,
-    short lock_wait_bit)
-{
-	mtx_assert(mtxp, MA_OWNED);
-
-	while ((fp->f_vflags & lock_bit) != 0) {
-		fp->f_vflags |= lock_wait_bit;
-		msleep(&fp->f_vflags, mtxp, PRI_MAX_KERN,
-		    "vofflock", 0);
-	}
-	fp->f_vflags |= lock_bit;
-}
-
-static void
-file_v_unlock_mtxp(struct file *fp, struct mtx *mtxp, short lock_bit,
-    short lock_wait_bit)
-{
-	mtx_assert(mtxp, MA_OWNED);
-
-	KASSERT((fp->f_vflags & lock_bit) != 0, ("Lost lock_bit"));
-	if ((fp->f_vflags & lock_wait_bit) != 0)
-		wakeup(&fp->f_vflags);
-	fp->f_vflags &= ~(lock_bit | lock_wait_bit);
-}
-
+#else
 off_t
 foffset_lock(struct file *fp, int flags)
 {
@@ -935,8 +894,12 @@ foffset_lock(struct file *fp, int flags)
 	mtxp = mtx_pool_find(mtxpool_sleep, fp);
 	mtx_lock(mtxp);
 	if ((flags & FOF_NOLOCK) == 0) {
-		file_v_lock_mtxp(fp, mtxp, FILE_V_FOFFSET_LOCKED,
-		    FILE_V_FOFFSET_LOCK_WAITING);
+		while (fp->f_vnread_flags & FOFFSET_LOCKED) {
+			fp->f_vnread_flags |= FOFFSET_LOCK_WAITING;
+			msleep(&fp->f_vnread_flags, mtxp, PRI_MAX_KERN,
+			    "vofflock", 0);
+		}
+		fp->f_vnread_flags |= FOFFSET_LOCKED;
 	}
 	res = fp->f_offset;
 	mtx_unlock(mtxp);
@@ -959,8 +922,11 @@ foffset_unlock(struct file *fp, off_t val, int flags)
 	if ((flags & FOF_NEXTOFF_W) != 0)
 		fp->f_nextoff[UIO_WRITE] = val;
 	if ((flags & FOF_NOLOCK) == 0) {
-		file_v_unlock_mtxp(fp, mtxp, FILE_V_FOFFSET_LOCKED,
-		    FILE_V_FOFFSET_LOCK_WAITING);
+		KASSERT((fp->f_vnread_flags & FOFFSET_LOCKED) != 0,
+		    ("Lost FOFFSET_LOCKED"));
+		if (fp->f_vnread_flags & FOFFSET_LOCK_WAITING)
+			wakeup(&fp->f_vnread_flags);
+		fp->f_vnread_flags = 0;
 	}
 	mtx_unlock(mtxp);
 }

sys/sys/file.h

@@ -197,7 +197,7 @@ struct file {
 	struct vnode 	*f_vnode;	/* NULL or applicable vnode */
 	struct ucred	*f_cred;	/* associated credentials. */
 	short		f_type;		/* descriptor type */
-	short		f_vflags;	/* (f) Sleep lock flags for members */
+	short		f_vnread_flags; /* (f) Sleep lock for f_offset */
 	/*
 	 *  DTYPE_VNODE specific fields.
 	 */
@@ -220,8 +220,8 @@ struct file {
 #define	f_cdevpriv	f_vnun.fvn_cdevpriv
 #define	f_advice	f_vnun.fvn_advice
 
-#define	FILE_V_FOFFSET_LOCKED		0x0001
-#define	FILE_V_FOFFSET_LOCK_WAITING	0x0002
+#define	FOFFSET_LOCKED       0x1
+#define	FOFFSET_LOCK_WAITING 0x2
 #endif /* __BSD_VISIBLE */
 
 #endif /* _KERNEL || _WANT_FILE */