Revert anton@ changes about read/write unlocking

https://marc.info/?l=openbsd-cvs&m=156277704122293&w=2

ok anton@

12 files changed, 52 insertions, 145 deletions

diff --git a/sys/dev/pci/drm/drm_linux.c b/sys/dev/pci/drm/drm_linux.c
index 90dc896ad3e..7724d827bea 100644
--- a/sys/dev/pci/drm/drm_linux.c
+++ b/sys/dev/pci/drm/drm_linux.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: drm_linux.c,v 1.43 2019/07/10 16:43:19 anton Exp $	*/
+/*	$OpenBSD: drm_linux.c,v 1.44 2019/07/12 13:56:27 solene Exp $	*/
 /*
  * Copyright (c) 2013 Jonathan Gray <jsg@openbsd.org>
  * Copyright (c) 2015, 2016 Mark Kettenis <kettenis@openbsd.org>
@@ -1346,9 +1346,7 @@ dmabuf_seek(struct file *fp, off_t *offset, int whence, struct proc *p)
 	default:
 		return (EINVAL);
 	}
-	foffset_enter(fp);
-	foffset_leave(fp, newoff, 0);
-	*offset = newoff;
+	fp->f_offset = *offset = newoff;
 	return (0);
 }
 
diff --git a/sys/isofs/cd9660/cd9660_vnops.c b/sys/isofs/cd9660/cd9660_vnops.c
index 7ff1f74a015..5a588bb1a4e 100644
--- a/sys/isofs/cd9660/cd9660_vnops.c
+++ b/sys/isofs/cd9660/cd9660_vnops.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: cd9660_vnops.c,v 1.77 2019/07/10 16:43:19 anton Exp $	*/
+/*	$OpenBSD: cd9660_vnops.c,v 1.78 2019/07/12 13:56:27 solene Exp $	*/
 /*	$NetBSD: cd9660_vnops.c,v 1.42 1997/10/16 23:56:57 christos Exp $	*/
 
 /*-
@@ -1016,7 +1016,7 @@ filt_cd9660read(struct knote *kn, long hint)
 		return (1);
 	}
 
-	kn->kn_data = node->i_size - foffset_get(kn->kn_fp);
+	kn->kn_data = node->i_size - kn->kn_fp->f_offset;
 	if (kn->kn_data == 0 && kn->kn_sfflags & NOTE_EOF) {
 		kn->kn_fflags |= NOTE_EOF;
 		return (1);
diff --git a/sys/kern/kern_descrip.c b/sys/kern/kern_descrip.c
index 2cf66bb04aa..c38ee669517 100644
--- a/sys/kern/kern_descrip.c
+++ b/sys/kern/kern_descrip.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kern_descrip.c,v 1.189 2019/07/10 16:43:19 anton Exp $	*/
+/*	$OpenBSD: kern_descrip.c,v 1.190 2019/07/12 13:56:27 solene Exp $	*/
 /*	$NetBSD: kern_descrip.c,v 1.42 1996/03/30 22:24:38 christos Exp $	*/
 
 /*
@@ -532,14 +532,12 @@ restart:
 			ktrflock(p, &fl);
 #endif
 		if (fl.l_whence == SEEK_CUR) {
-			off_t offset = foffset_get(fp);
-
 			if (fl.l_start == 0 && fl.l_len < 0) {
 				/* lockf(3) compliance hack */
 				fl.l_len = -fl.l_len;
-				fl.l_start = offset - fl.l_len;
+				fl.l_start = fp->f_offset - fl.l_len;
 			} else
-				fl.l_start += offset;
+				fl.l_start += fp->f_offset;
 		}
 		switch (fl.l_type) {
 
@@ -604,14 +602,12 @@ restart:
 		if (error)
 			break;
 		if (fl.l_whence == SEEK_CUR) {
-			off_t offset = foffset_get(fp);
-
 			if (fl.l_start == 0 && fl.l_len < 0) {
 				/* lockf(3) compliance hack */
 				fl.l_len = -fl.l_len;
-				fl.l_start = offset - fl.l_len;
+				fl.l_start = fp->f_offset - fl.l_len;
 			} else
-				fl.l_start += offset;
+				fl.l_start += fp->f_offset;
 		}
 		if (fl.l_type != F_RDLCK &&
 		    fl.l_type != F_WRLCK &&
@@ -1281,73 +1277,6 @@ fdrop(struct file *fp, struct proc *p)
 }
 
 /*
- * Get the file offset without keeping the same offset locked upon return.
- */
-off_t
-foffset_get(struct file *fp)
-{
-	off_t offset;
-
-	mtx_enter(&fp->f_mtx);
-	offset = fp->f_offset;
-	mtx_leave(&fp->f_mtx);
-	return (offset);
-}
-
-/*
- * Acquire an exclusive lock of the file offset. The calling thread must call
- * foffset_leave() once done.
- */
-off_t
-foffset_enter(struct file *fp)
-{
-	off_t offset;
-
-	mtx_enter(&fp->f_mtx);
-
-	while (fp->f_olock & FOL_LOCKED) {
-		KASSERT((fp->f_olock & FOL_NWAIT) < FOL_NWAIT);
-		fp->f_olock++;
-		msleep(&fp->f_olock, &fp->f_mtx, PLOCK, "foffset", 0);
-		KASSERT((fp->f_olock & FOL_NWAIT) > 0);
-		fp->f_olock--;
-	}
-	fp->f_olock |= FOL_LOCKED;
-
-	offset = fp->f_offset;
-
-	mtx_leave(&fp->f_mtx);
-
-	return (offset);
-}
-
-/*
- * Write a new file offset and release the lock. The calling thread must already
- * have acquired the lock using foffset_enter().
- * If FO_NOUPDATE is present in flags, only the lock is released and the offset
- * remains unmodified.
- */
-void
-foffset_leave(struct file *fp, off_t offset, int flags)
-{
-	unsigned int nwait;
-
-	mtx_enter(&fp->f_mtx);
-
-	KASSERT(fp->f_olock & FOL_LOCKED);
-
-	if ((flags & FO_NOUPDATE) == 0)
-		fp->f_offset = offset;
-	nwait = fp->f_olock & FOL_NWAIT;
-	fp->f_olock &= ~FOL_LOCKED;
-
-	mtx_leave(&fp->f_mtx);
-
-	if (nwait > 0)
-		wakeup_one(&fp->f_olock);
-}
-
-/*
  * Apply an advisory lock on a file descriptor.
  *
  * Just attempt to get a record lock of the requested type on
diff --git a/sys/kern/kern_sysctl.c b/sys/kern/kern_sysctl.c
index 53b5d9e878f..1cdaf84f032 100644
--- a/sys/kern/kern_sysctl.c
+++ b/sys/kern/kern_sysctl.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kern_sysctl.c,v 1.362 2019/07/12 00:04:59 cheloha Exp $	*/
+/*	$OpenBSD: kern_sysctl.c,v 1.363 2019/07/12 13:56:27 solene Exp $	*/
 /*	$NetBSD: kern_sysctl.c,v 1.17 1996/05/20 17:49:05 mrg Exp $	*/
 
 /*-
@@ -1102,8 +1102,8 @@ fill_file(struct kinfo_file *kf, struct file *fp, struct filedesc *fdp,
 		kf->f_usecount = 0;
 
 		if (suser(p) == 0 || p->p_ucred->cr_uid == fp->f_cred->cr_uid) {
-			mtx_enter(&fp->f_mtx);
 			kf->f_offset = fp->f_offset;
+			mtx_enter(&fp->f_mtx);
 			kf->f_rxfer = fp->f_rxfer;
 			kf->f_rwfer = fp->f_wxfer;
 			kf->f_seek = fp->f_seek;
diff --git a/sys/kern/vfs_syscalls.c b/sys/kern/vfs_syscalls.c
index c334dc98a69..b2aa8939001 100644
--- a/sys/kern/vfs_syscalls.c
+++ b/sys/kern/vfs_syscalls.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: vfs_syscalls.c,v 1.320 2019/07/10 16:43:19 anton Exp $	*/
+/*	$OpenBSD: vfs_syscalls.c,v 1.321 2019/07/12 13:56:27 solene Exp $	*/
 /*	$NetBSD: vfs_syscalls.c,v 1.71 1996/04/23 10:29:02 mycroft Exp $	*/
 
 /*
@@ -2999,7 +2999,6 @@ sys_getdents(struct proc *p, void *v, register_t *retval)
 	struct uio auio;
 	struct iovec aiov;
 	size_t buflen;
-	off_t offset;
 	int error, eofflag;
 
 	buflen = SCARG(uap, buflen);
@@ -3012,16 +3011,12 @@ sys_getdents(struct proc *p, void *v, register_t *retval)
 		error = EBADF;
 		goto bad;
 	}
-
-	offset = foffset_enter(fp);
-	if (offset < 0) {
-		foffset_leave(fp, 0, FO_NOUPDATE);
+	if (fp->f_offset < 0) {
 		error = EINVAL;
 		goto bad;
 	}
 	vp = fp->f_data;
 	if (vp->v_type != VDIR) {
-		foffset_leave(fp, 0, FO_NOUPDATE);
 		error = EINVAL;
 		goto bad;
 	}
@@ -3034,10 +3029,10 @@ sys_getdents(struct proc *p, void *v, register_t *retval)
 	auio.uio_procp = p;
 	auio.uio_resid = buflen;
 	vn_lock(vp, LK_EXCLUSIVE | LK_RETRY);
-	auio.uio_offset = offset;
+	auio.uio_offset = fp->f_offset;
 	error = VOP_READDIR(vp, &auio, fp->f_cred, &eofflag);
+	fp->f_offset = auio.uio_offset;
 	VOP_UNLOCK(vp);
-	foffset_leave(fp, auio.uio_offset, 0);
 	if (error)
 		goto bad;
 	*retval = buflen - auio.uio_resid;
diff --git a/sys/kern/vfs_vnops.c b/sys/kern/vfs_vnops.c
index 43b1dbe1c85..5e3673eedf6 100644
--- a/sys/kern/vfs_vnops.c
+++ b/sys/kern/vfs_vnops.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: vfs_vnops.c,v 1.100 2019/07/10 16:43:19 anton Exp $	*/
+/*	$OpenBSD: vfs_vnops.c,v 1.101 2019/07/12 13:56:27 solene Exp $	*/
 /*	$NetBSD: vfs_vnops.c,v 1.20 1996/02/04 02:18:41 christos Exp $	*/
 
 /*
@@ -342,35 +342,38 @@ vn_read(struct file *fp, struct uio *uio, int fflags)
 	size_t count = uio->uio_resid;
 	off_t offset;
 	int error;
-	int foflags = 0;
 
 	KERNEL_LOCK();
 
+	/*
+	 * Check below can race.  We can block on the vnode lock
+	 * and resume with a different `fp->f_offset' value.
+	 */
 	if ((fflags & FO_POSITION) == 0)
-		uio->uio_offset = offset = foffset_enter(fp);
+		offset = fp->f_offset;
 	else
 		offset = uio->uio_offset;
 
 	/* no wrap around of offsets except on character devices */
 	if (vp->v_type != VCHR && count > LLONG_MAX - offset) {
-		foflags = FO_NOUPDATE;
 		error = EINVAL;
 		goto done;
 	}
 
 	if (vp->v_type == VDIR) {
-		foflags = FO_NOUPDATE;
 		error = EISDIR;
 		goto done;
 	}
 
 	vn_lock(vp, LK_EXCLUSIVE | LK_RETRY);
+	if ((fflags & FO_POSITION) == 0)
+		uio->uio_offset = fp->f_offset;
 	error = VOP_READ(vp, uio, (fp->f_flag & FNONBLOCK) ? IO_NDELAY : 0,
 	    cred);
+	if ((fflags & FO_POSITION) == 0)
+		fp->f_offset += count - uio->uio_resid;
 	VOP_UNLOCK(vp);
 done:
-	if ((fflags & FO_POSITION) == 0)
-		foffset_leave(fp, offset + (count - uio->uio_resid), foflags);
 	KERNEL_UNLOCK();
 	return (error);
 }
@@ -383,15 +386,11 @@ vn_write(struct file *fp, struct uio *uio, int fflags)
 {
 	struct vnode *vp = fp->f_data;
 	struct ucred *cred = fp->f_cred;
-	off_t offset;
 	int error, ioflag = IO_UNIT;
 	size_t count;
 
 	KERNEL_LOCK();
 
-	if ((fflags & FO_POSITION) == 0)
-		uio->uio_offset = offset = foffset_enter(fp);
-
 	/* note: pwrite/pwritev are unaffected by O_APPEND */
 	if (vp->v_type == VREG && (fp->f_flag & O_APPEND) &&
 	    (fflags & FO_POSITION) == 0)
@@ -402,15 +401,17 @@ vn_write(struct file *fp, struct uio *uio, int fflags)
 	    (vp->v_mount && (vp->v_mount->mnt_flag & MNT_SYNCHRONOUS)))
 		ioflag |= IO_SYNC;
 	vn_lock(vp, LK_EXCLUSIVE | LK_RETRY);
+	if ((fflags & FO_POSITION) == 0)
+		uio->uio_offset = fp->f_offset;
 	count = uio->uio_resid;
 	error = VOP_WRITE(vp, uio, ioflag, cred);
-	VOP_UNLOCK(vp);
 	if ((fflags & FO_POSITION) == 0) {
 		if (ioflag & IO_APPEND)
-			foffset_leave(fp, uio->uio_offset, 0);
+			fp->f_offset = uio->uio_offset;
 		else
-			foffset_leave(fp, offset + (count - uio->uio_resid), 0);
+			fp->f_offset += count - uio->uio_resid;
 	}
+	VOP_UNLOCK(vp);
 
 	KERNEL_UNLOCK();
 	return (error);
@@ -508,7 +509,7 @@ vn_ioctl(struct file *fp, u_long com, caddr_t data, struct proc *p)
 			error = VOP_GETATTR(vp, &vattr, p->p_ucred, p);
 			if (error)
 				return (error);
-			*(int *)data = vattr.va_size - foffset_get(fp);
+			*(int *)data = vattr.va_size - fp->f_offset;
 			return (0);
 		}
 		if (com == FIONBIO || com == FIOASYNC)  /* XXX */
@@ -600,7 +601,7 @@ vn_seek(struct file *fp, off_t *offset, int whence, struct proc *p)
 	struct ucred *cred = p->p_ucred;
 	struct vnode *vp = fp->f_data;
 	struct vattr vattr;
-	off_t curoff, newoff;
+	off_t newoff;
 	int error, special;
 
 	if (vp->v_type == VFIFO)
@@ -610,35 +611,28 @@ vn_seek(struct file *fp, off_t *offset, int whence, struct proc *p)
 	else
 		special = 0;
 
-	curoff = foffset_enter(fp);
 	switch (whence) {
 	case SEEK_CUR:
-		newoff = curoff + *offset;
+		newoff = fp->f_offset + *offset;
 		break;
 	case SEEK_END:
 		error = VOP_GETATTR(vp, &vattr, cred, p);
 		if (error)
-			goto bad;
+			return (error);
 		newoff = *offset + (off_t)vattr.va_size;
 		break;
 	case SEEK_SET:
 		newoff = *offset;
 		break;
 	default:
-		error = EINVAL;
-		goto bad;
+		return (EINVAL);
 	}
-	if (!special && newoff < 0) {
-		error = EINVAL;
-		goto bad;
+	if (!special) {
+		if (newoff < 0)
+			return(EINVAL);
 	}
-	foffset_leave(fp, newoff, 0);
-	*offset = newoff;
+	fp->f_offset = *offset = newoff;
 	return (0);
-
-bad:
-	foffset_leave(fp, 0, FO_NOUPDATE);
-	return (error);
 }
 
 /*
diff --git a/sys/miscfs/fuse/fuse_vnops.c b/sys/miscfs/fuse/fuse_vnops.c
index c4351c3155b..6de20b1744e 100644
--- a/sys/miscfs/fuse/fuse_vnops.c
+++ b/sys/miscfs/fuse/fuse_vnops.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: fuse_vnops.c,v 1.53 2019/07/10 16:43:19 anton Exp $ */
+/* $OpenBSD: fuse_vnops.c,v 1.54 2019/07/12 13:56:27 solene Exp $ */
 /*
  * Copyright (c) 2012-2013 Sylvestre Gallon <ccna.syl@gmail.com>
  *
@@ -168,7 +168,7 @@ filt_fusefsread(struct knote *kn, long hint)
 		return (1);
 	}
 
-	kn->kn_data = ip->filesize - foffset_get(kn->kn_fp);
+	kn->kn_data = ip->filesize - kn->kn_fp->f_offset;
 	if (kn->kn_data == 0 && kn->kn_sfflags & NOTE_EOF) {
 		kn->kn_fflags |= NOTE_EOF;
 		return (1);
diff --git a/sys/msdosfs/msdosfs_vnops.c b/sys/msdosfs/msdosfs_vnops.c
index d39361ce732..dcf77c86a21 100644
--- a/sys/msdosfs/msdosfs_vnops.c
+++ b/sys/msdosfs/msdosfs_vnops.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: msdosfs_vnops.c,v 1.123 2019/07/10 16:43:19 anton Exp $	*/
+/*	$OpenBSD: msdosfs_vnops.c,v 1.124 2019/07/12 13:56:27 solene Exp $	*/
 /*	$NetBSD: msdosfs_vnops.c,v 1.63 1997/10/17 11:24:19 ws Exp $	*/
 
 /*-
@@ -2017,7 +2017,7 @@ filt_msdosfsread(struct knote *kn, long hint)
 		return (1);
 	}
 
-	kn->kn_data = dep->de_FileSize - foffset_get(kn->kn_fp);
+	kn->kn_data = dep->de_FileSize - kn->kn_fp->f_offset;
 	if (kn->kn_data == 0 && kn->kn_sfflags & NOTE_EOF) {
 		kn->kn_fflags |= NOTE_EOF;
 		return (1);
diff --git a/sys/nfs/nfs_kq.c b/sys/nfs/nfs_kq.c
index 459e40907a7..a73847a9a2e 100644
--- a/sys/nfs/nfs_kq.c
+++ b/sys/nfs/nfs_kq.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: nfs_kq.c,v 1.23 2019/07/10 16:43:20 anton Exp $ */
+/*	$OpenBSD: nfs_kq.c,v 1.24 2019/07/12 13:56:27 solene Exp $ */
 /*	$NetBSD: nfs_kq.c,v 1.7 2003/10/30 01:43:10 simonb Exp $	*/
 
 /*-
@@ -226,7 +226,7 @@ filt_nfsread(struct knote *kn, long hint)
 		return (1);
 	}
 
-	kn->kn_data = np->n_size - foffset_get(kn->kn_fp);
+	kn->kn_data = np->n_size - kn->kn_fp->f_offset;
 #ifdef DEBUG
 	printf("nfsread event. %lld\n", kn->kn_data);
 #endif
diff --git a/sys/sys/file.h b/sys/sys/file.h
index b9c76212ce2..be123c063f3 100644
--- a/sys/sys/file.h
+++ b/sys/sys/file.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: file.h,v 1.56 2019/07/11 06:15:02 anton Exp $	*/
+/*	$OpenBSD: file.h,v 1.57 2019/07/12 13:56:28 solene Exp $	*/
 /*	$NetBSD: file.h,v 1.11 1995/03/26 20:24:13 jtc Exp $	*/
 
 /*
@@ -64,7 +64,6 @@ struct	fileops {
 	int	(*fo_seek)(struct file *, off_t *, int, struct proc *);
 };
 #define FO_POSITION	0x00000001	/* positioned read/write */
-#define FO_NOUPDATE	0x00000002	/* don't update file offset */
 
 /*
  * Kernel descriptor table.
@@ -90,9 +89,8 @@ struct file {
 	u_int	f_count;	/* [a] reference count */
 	struct	ucred *f_cred;	/* [I] credentials associated with descriptor */
 	struct	fileops *f_ops; /* [I] file operation pointers */
-	off_t	f_offset;	/* [f] */
+	off_t	f_offset;	/* [k] */
 	void 	*f_data;	/* [I] private data */
-	u_int	f_olock;	/* [f] offset lock */
 	int	f_iflags;	/* [k] internal flags */
 	uint64_t f_rxfer;	/* [f] total number of read transfers */
 	uint64_t f_wxfer;	/* [f] total number of write transfers */
@@ -118,13 +116,6 @@ struct file {
 
 int	fdrop(struct file *, struct proc *);
 
-off_t	foffset_get(struct file *);
-off_t	foffset_enter(struct file *);
-void	foffset_leave(struct file *, off_t, int);
-
-#define FOL_NWAIT	0x7fffffffu	/* number of waiters */
-#define FOL_LOCKED	0x80000000u	/* file offset is locked */
-
 LIST_HEAD(filelist, file);
 extern int maxfiles;			/* kernel limit on number of open files */
 extern int numfiles;			/* actual number of open files */
diff --git a/sys/tmpfs/tmpfs_vnops.c b/sys/tmpfs/tmpfs_vnops.c
index 69a814ae26a..81ad688fd84 100644
--- a/sys/tmpfs/tmpfs_vnops.c
+++ b/sys/tmpfs/tmpfs_vnops.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: tmpfs_vnops.c,v 1.34 2019/07/10 16:43:20 anton Exp $	*/
+/*	$OpenBSD: tmpfs_vnops.c,v 1.35 2019/07/12 13:56:28 solene Exp $	*/
 /*	$NetBSD: tmpfs_vnops.c,v 1.100 2012/11/05 17:27:39 dholland Exp $	*/
 
 /*
@@ -2645,7 +2645,7 @@ filt_tmpfsread(struct knote *kn, long hint)
 		return (1);
 	}
 
-	kn->kn_data = node->tn_size - foffset_get(kn->kn_fp);
+	kn->kn_data = node->tn_size - kn->kn_fp->f_offset;
 	if (kn->kn_data == 0 && kn->kn_sfflags & NOTE_EOF) {
 		kn->kn_fflags |= NOTE_EOF;
 		return (1);
diff --git a/sys/ufs/ufs/ufs_vnops.c b/sys/ufs/ufs/ufs_vnops.c
index 224a37250a2..fe4b92d0278 100644
--- a/sys/ufs/ufs/ufs_vnops.c
+++ b/sys/ufs/ufs/ufs_vnops.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ufs_vnops.c,v 1.144 2019/07/10 16:43:20 anton Exp $	*/
+/*	$OpenBSD: ufs_vnops.c,v 1.145 2019/07/12 13:56:28 solene Exp $	*/
 /*	$NetBSD: ufs_vnops.c,v 1.18 1996/05/11 18:28:04 mycroft Exp $	*/
 
 /*
@@ -1952,10 +1952,10 @@ filt_ufsread(struct knote *kn, long hint)
 
 #ifdef EXT2FS
 	if (IS_EXT2_VNODE(ip->i_vnode))
-		kn->kn_data = ext2fs_size(ip) - foffset_get(kn->kn_fp);
+		kn->kn_data = ext2fs_size(ip) - kn->kn_fp->f_offset;
 	else
 #endif
-		kn->kn_data = DIP(ip, size) - foffset_get(kn->kn_fp);
+		kn->kn_data = DIP(ip, size) - kn->kn_fp->f_offset;
 	if (kn->kn_data == 0 && kn->kn_sfflags & NOTE_EOF) {
 		kn->kn_fflags |= NOTE_EOF;
 		return (1);