Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 29 Aug 2012 15:45:59 +0000 (UTC)
From:      Konstantin Belousov <kib@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-9@freebsd.org
Subject:   svn commit: r239843 - in stable/9/sys: compat/linux fs/devfs kern sys ufs/ffs
Message-ID:  <201208291545.q7TFjxX0070678@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: kib
Date: Wed Aug 29 15:45:58 2012
New Revision: 239843
URL: http://svn.freebsd.org/changeset/base/239843

Log:
  MFC r238029:
  Extend the KPI to lock and unlock f_offset member of struct file.  It
  now fully encapsulates all accesses to f_offset, and extends f_offset
  locking to other consumers that need it, in particular, to lseek() and
  variants of getdirentries().

Modified:
  stable/9/sys/compat/linux/linux_file.c
  stable/9/sys/fs/devfs/devfs_vnops.c
  stable/9/sys/kern/kern_descrip.c
  stable/9/sys/kern/vfs_syscalls.c
  stable/9/sys/kern/vfs_vnops.c
  stable/9/sys/sys/file.h
  stable/9/sys/ufs/ffs/ffs_alloc.c
Directory Properties:
  stable/9/sys/   (props changed)
  stable/9/sys/fs/   (props changed)

Modified: stable/9/sys/compat/linux/linux_file.c
==============================================================================
--- stable/9/sys/compat/linux/linux_file.c	Wed Aug 29 15:41:17 2012	(r239842)
+++ stable/9/sys/compat/linux/linux_file.c	Wed Aug 29 15:45:58 2012	(r239843)
@@ -354,15 +354,16 @@ getdents_common(struct thread *td, struc
 		return (EBADF);
 	}
 
+	off = foffset_lock(fp, 0);
 	vp = fp->f_vnode;
 	vfslocked = VFS_LOCK_GIANT(vp->v_mount);
 	if (vp->v_type != VDIR) {
 		VFS_UNLOCK_GIANT(vfslocked);
+		foffset_unlock(fp, off, 0);
 		fdrop(fp, td);
 		return (EINVAL);
 	}
 
-	off = fp->f_offset;
 
 	buflen = max(LINUX_DIRBLKSIZ, nbytes);
 	buflen = min(buflen, MAXBSIZE);
@@ -511,7 +512,6 @@ getdents_common(struct thread *td, struc
 		goto eof;
 	}
 
-	fp->f_offset = off;
 	if (justone)
 		nbytes = resid + linuxreclen;
 
@@ -524,6 +524,7 @@ out:
 
 	VOP_UNLOCK(vp, 0);
 	VFS_UNLOCK_GIANT(vfslocked);
+	foffset_unlock(fp, off, 0);
 	fdrop(fp, td);
 	free(buf, M_TEMP);
 	free(lbuf, M_TEMP);

Modified: stable/9/sys/fs/devfs/devfs_vnops.c
==============================================================================
--- stable/9/sys/fs/devfs/devfs_vnops.c	Wed Aug 29 15:41:17 2012	(r239842)
+++ stable/9/sys/fs/devfs/devfs_vnops.c	Wed Aug 29 15:45:58 2012	(r239843)
@@ -1170,18 +1170,14 @@ devfs_read_f(struct file *fp, struct uio
 	if (ioflag & O_DIRECT)
 		ioflag |= IO_DIRECT;
 
-	if ((flags & FOF_OFFSET) == 0)
-		uio->uio_offset = fp->f_offset;
-
+	foffset_lock_uio(fp, uio, flags | FOF_NOLOCK);
 	error = dsw->d_read(dev, uio, ioflag);
 	if (uio->uio_resid != resid || (error == 0 && resid != 0))
 		vfs_timestamp(&dev->si_atime);
 	td->td_fpop = fpop;
 	dev_relthread(dev, ref);
 
-	if ((flags & FOF_OFFSET) == 0)
-		fp->f_offset = uio->uio_offset;
-	fp->f_nextoff = uio->uio_offset;
+	foffset_unlock_uio(fp, uio, flags | FOF_NOLOCK | FOF_NEXTOFF);
 	return (error);
 }
 
@@ -1648,8 +1644,7 @@ devfs_write_f(struct file *fp, struct ui
 	ioflag = fp->f_flag & (O_NONBLOCK | O_DIRECT | O_FSYNC);
 	if (ioflag & O_DIRECT)
 		ioflag |= IO_DIRECT;
-	if ((flags & FOF_OFFSET) == 0)
-		uio->uio_offset = fp->f_offset;
+	foffset_lock_uio(fp, uio, flags | FOF_NOLOCK);
 
 	resid = uio->uio_resid;
 
@@ -1661,9 +1656,7 @@ devfs_write_f(struct file *fp, struct ui
 	td->td_fpop = fpop;
 	dev_relthread(dev, ref);
 
-	if ((flags & FOF_OFFSET) == 0)
-		fp->f_offset = uio->uio_offset;
-	fp->f_nextoff = uio->uio_offset;
+	foffset_unlock_uio(fp, uio, flags | FOF_NOLOCK | FOF_NEXTOFF);
 	return (error);
 }
 

Modified: stable/9/sys/kern/kern_descrip.c
==============================================================================
--- stable/9/sys/kern/kern_descrip.c	Wed Aug 29 15:41:17 2012	(r239842)
+++ stable/9/sys/kern/kern_descrip.c	Wed Aug 29 15:45:58 2012	(r239843)
@@ -472,6 +472,7 @@ kern_fcntl(struct thread *td, int fd, in
 	int vfslocked;
 	u_int old, new;
 	uint64_t bsize;
+	off_t foffset;
 
 	vfslocked = 0;
 	error = 0;
@@ -613,14 +614,15 @@ kern_fcntl(struct thread *td, int fd, in
 		}
 		flp = (struct flock *)arg;
 		if (flp->l_whence == SEEK_CUR) {
-			if (fp->f_offset < 0 ||
+			foffset = foffset_get(fp);
+			if (foffset < 0 ||
 			    (flp->l_start > 0 &&
-			     fp->f_offset > OFF_MAX - flp->l_start)) {
+			     foffset > OFF_MAX - flp->l_start)) {
 				FILEDESC_SUNLOCK(fdp);
 				error = EOVERFLOW;
 				break;
 			}
-			flp->l_start += fp->f_offset;
+			flp->l_start += foffset;
 		}
 
 		/*
@@ -714,15 +716,16 @@ kern_fcntl(struct thread *td, int fd, in
 			break;
 		}
 		if (flp->l_whence == SEEK_CUR) {
+			foffset = foffset_get(fp);
 			if ((flp->l_start > 0 &&
-			    fp->f_offset > OFF_MAX - flp->l_start) ||
+			    foffset > OFF_MAX - flp->l_start) ||
 			    (flp->l_start < 0 &&
-			     fp->f_offset < OFF_MIN - flp->l_start)) {
+			     foffset < OFF_MIN - flp->l_start)) {
 				FILEDESC_SUNLOCK(fdp);
 				error = EOVERFLOW;
 				break;
 			}
-			flp->l_start += fp->f_offset;
+			flp->l_start += foffset;
 		}
 		/*
 		 * VOP_ADVLOCK() may block.
@@ -2907,7 +2910,7 @@ sysctl_kern_file(SYSCTL_HANDLER_ARGS)
 			xf.xf_type = fp->f_type;
 			xf.xf_count = fp->f_count;
 			xf.xf_msgcount = 0;
-			xf.xf_offset = fp->f_offset;
+			xf.xf_offset = foffset_get(fp);
 			xf.xf_flag = fp->f_flag;
 			error = SYSCTL_OUT(req, &xf, sizeof(xf));
 			if (error)
@@ -3112,7 +3115,7 @@ sysctl_kern_proc_ofiledesc(SYSCTL_HANDLE
 			kif->kf_flags |= KF_FLAG_DIRECT;
 		if (fp->f_flag & FHASLOCK)
 			kif->kf_flags |= KF_FLAG_HASLOCK;
-		kif->kf_offset = fp->f_offset;
+		kif->kf_offset = foffset_get(fp);
 		if (vp != NULL) {
 			vref(vp);
 			switch (vp->v_type) {
@@ -3456,7 +3459,7 @@ sysctl_kern_proc_filedesc(SYSCTL_HANDLER
 		}
 		refcnt = fp->f_count;
 		fflags = fp->f_flag;
-		offset = fp->f_offset;
+		offset = foffset_get(fp);
 
 		/*
 		 * Create sysctl entry.

Modified: stable/9/sys/kern/vfs_syscalls.c
==============================================================================
--- stable/9/sys/kern/vfs_syscalls.c	Wed Aug 29 15:41:17 2012	(r239842)
+++ stable/9/sys/kern/vfs_syscalls.c	Wed Aug 29 15:45:58 2012	(r239843)
@@ -1992,7 +1992,7 @@ sys_lseek(td, uap)
 	struct file *fp;
 	struct vnode *vp;
 	struct vattr vattr;
-	off_t offset, size;
+	off_t foffset, offset, size;
 	int error, noneg;
 	int vfslocked;
 
@@ -2004,18 +2004,19 @@ sys_lseek(td, uap)
 		return (ESPIPE);
 	}
 	vp = fp->f_vnode;
+	foffset = foffset_lock(fp, 0);
 	vfslocked = VFS_LOCK_GIANT(vp->v_mount);
 	noneg = (vp->v_type != VCHR);
 	offset = uap->offset;
 	switch (uap->whence) {
 	case L_INCR:
 		if (noneg &&
-		    (fp->f_offset < 0 ||
-		    (offset > 0 && fp->f_offset > OFF_MAX - offset))) {
+		    (foffset < 0 ||
+		    (offset > 0 && foffset > OFF_MAX - offset))) {
 			error = EOVERFLOW;
 			break;
 		}
-		offset += fp->f_offset;
+		offset += foffset;
 		break;
 	case L_XTND:
 		vn_lock(vp, LK_SHARED | LK_RETRY);
@@ -2055,12 +2056,12 @@ sys_lseek(td, uap)
 		error = EINVAL;
 	if (error != 0)
 		goto drop;
-	fp->f_offset = offset;
 	VFS_KNOTE_UNLOCKED(vp, 0);
-	*(off_t *)(td->td_retval) = fp->f_offset;
+	*(off_t *)(td->td_retval) = offset;
 drop:
 	fdrop(fp, td);
 	VFS_UNLOCK_GIANT(vfslocked);
+	foffset_unlock(fp, offset, error != 0 ? FOF_NOUPDATE : 0);
 	return (error);
 }
 
@@ -3993,6 +3994,7 @@ kern_ogetdirentries(struct thread *td, s
 	caddr_t dirbuf;
 	int error, eofflag, readcnt, vfslocked;
 	long loff;
+	off_t foffset;
 
 	/* XXX arbitrary sanity limit on `count'. */
 	if (uap->count > 64 * 1024)
@@ -4005,10 +4007,12 @@ kern_ogetdirentries(struct thread *td, s
 		return (EBADF);
 	}
 	vp = fp->f_vnode;
+	foffset = foffset_lock(fp, 0);
 unionread:
 	vfslocked = VFS_LOCK_GIANT(vp->v_mount);
 	if (vp->v_type != VDIR) {
 		VFS_UNLOCK_GIANT(vfslocked);
+		foffset_unlock(fp, foffset, 0);
 		fdrop(fp, td);
 		return (EINVAL);
 	}
@@ -4021,12 +4025,13 @@ unionread:
 	auio.uio_td = td;
 	auio.uio_resid = uap->count;
 	vn_lock(vp, LK_SHARED | LK_RETRY);
-	loff = auio.uio_offset = fp->f_offset;
+	loff = auio.uio_offset = foffset;
 #ifdef MAC
 	error = mac_vnode_check_readdir(td->td_ucred, vp);
 	if (error) {
 		VOP_UNLOCK(vp, 0);
 		VFS_UNLOCK_GIANT(vfslocked);
+		foffset_unlock(fp, foffset, FOF_NOUPDATE);
 		fdrop(fp, td);
 		return (error);
 	}
@@ -4035,7 +4040,7 @@ unionread:
 		if (vp->v_mount->mnt_maxsymlinklen <= 0) {
 			error = VOP_READDIR(vp, &auio, fp->f_cred, &eofflag,
 			    NULL, NULL);
-			fp->f_offset = auio.uio_offset;
+			foffset = auio.uio_offset;
 		} else
 #	endif
 	{
@@ -4047,7 +4052,7 @@ unionread:
 		kiov.iov_base = dirbuf;
 		error = VOP_READDIR(vp, &kuio, fp->f_cred, &eofflag,
 			    NULL, NULL);
-		fp->f_offset = kuio.uio_offset;
+		foffset = kuio.uio_offset;
 		if (error == 0) {
 			readcnt = uap->count - kuio.uio_resid;
 			edp = (struct dirent *)&dirbuf[readcnt];
@@ -4085,6 +4090,7 @@ unionread:
 	if (error) {
 		VOP_UNLOCK(vp, 0);
 		VFS_UNLOCK_GIANT(vfslocked);
+		foffset_unlock(fp, foffset, 0);
 		fdrop(fp, td);
 		return (error);
 	}
@@ -4096,13 +4102,14 @@ unionread:
 		VREF(vp);
 		fp->f_vnode = vp;
 		fp->f_data = vp;
-		fp->f_offset = 0;
+		foffset = 0;
 		vput(tvp);
 		VFS_UNLOCK_GIANT(vfslocked);
 		goto unionread;
 	}
 	VOP_UNLOCK(vp, 0);
 	VFS_UNLOCK_GIANT(vfslocked);
+	foffset_unlock(fp, foffset, 0);
 	fdrop(fp, td);
 	td->td_retval[0] = uap->count - auio.uio_resid;
 	if (error == 0)
@@ -4154,6 +4161,7 @@ kern_getdirentries(struct thread *td, in
 	int vfslocked;
 	long loff;
 	int error, eofflag;
+	off_t foffset;
 
 	AUDIT_ARG_FD(fd);
 	auio.uio_resid = count;
@@ -4167,6 +4175,7 @@ kern_getdirentries(struct thread *td, in
 		return (EBADF);
 	}
 	vp = fp->f_vnode;
+	foffset = foffset_lock(fp, 0);
 unionread:
 	vfslocked = VFS_LOCK_GIANT(vp->v_mount);
 	if (vp->v_type != VDIR) {
@@ -4183,14 +4192,14 @@ unionread:
 	auio.uio_td = td;
 	vn_lock(vp, LK_SHARED | LK_RETRY);
 	AUDIT_ARG_VNODE1(vp);
-	loff = auio.uio_offset = fp->f_offset;
+	loff = auio.uio_offset = foffset;
 #ifdef MAC
 	error = mac_vnode_check_readdir(td->td_ucred, vp);
 	if (error == 0)
 #endif
 		error = VOP_READDIR(vp, &auio, fp->f_cred, &eofflag, NULL,
 		    NULL);
-	fp->f_offset = auio.uio_offset;
+	foffset = auio.uio_offset;
 	if (error) {
 		VOP_UNLOCK(vp, 0);
 		VFS_UNLOCK_GIANT(vfslocked);
@@ -4204,7 +4213,7 @@ unionread:
 		VREF(vp);
 		fp->f_vnode = vp;
 		fp->f_data = vp;
-		fp->f_offset = 0;
+		foffset = 0;
 		vput(tvp);
 		VFS_UNLOCK_GIANT(vfslocked);
 		goto unionread;
@@ -4214,6 +4223,7 @@ unionread:
 	*basep = loff;
 	td->td_retval[0] = count - auio.uio_resid;
 fail:
+	foffset_unlock(fp, foffset, 0);
 	fdrop(fp, td);
 	return (error);
 }

Modified: stable/9/sys/kern/vfs_vnops.c
==============================================================================
--- stable/9/sys/kern/vfs_vnops.c	Wed Aug 29 15:41:17 2012	(r239842)
+++ stable/9/sys/kern/vfs_vnops.c	Wed Aug 29 15:45:58 2012	(r239843)
@@ -512,13 +512,22 @@ vn_rdwr_inchunks(rw, vp, base, len, offs
 	return (error);
 }
 
-static void
-foffset_lock(struct file *fp, struct uio *uio, int flags)
+off_t
+foffset_lock(struct file *fp, int flags)
 {
 	struct mtx *mtxp;
+	off_t res;
 
-	if ((flags & FOF_OFFSET) != 0)
-		return;
+	KASSERT((flags & FOF_OFFSET) == 0, ("FOF_OFFSET passed"));
+
+#if OFF_MAX <= LONG_MAX
+	/*
+	 * Caller only wants the current f_offset value.  Assume that
+	 * the long and shorter integer types reads are atomic.
+	 */
+	if ((flags & FOF_NOLOCK) != 0)
+		return (fp->f_offset);
+#endif
 
 	/*
 	 * According to McKusick the vn lock was protecting f_offset here.
@@ -526,16 +535,68 @@ foffset_lock(struct file *fp, struct uio
 	 */
 	mtxp = mtx_pool_find(mtxpool_sleep, fp);
 	mtx_lock(mtxp);
-	while (fp->f_vnread_flags & FOFFSET_LOCKED) {
-		fp->f_vnread_flags |= FOFFSET_LOCK_WAITING;
-		msleep(&fp->f_vnread_flags, mtxp, PUSER -1,
-		    "vnread offlock", 0);
+	if ((flags & FOF_NOLOCK) == 0) {
+		while (fp->f_vnread_flags & FOFFSET_LOCKED) {
+			fp->f_vnread_flags |= FOFFSET_LOCK_WAITING;
+			msleep(&fp->f_vnread_flags, mtxp, PUSER -1,
+			    "vofflock", 0);
+		}
+		fp->f_vnread_flags |= FOFFSET_LOCKED;
+	}
+	res = fp->f_offset;
+	mtx_unlock(mtxp);
+	return (res);
+}
+
+void
+foffset_unlock(struct file *fp, off_t val, int flags)
+{
+	struct mtx *mtxp;
+
+	KASSERT((flags & FOF_OFFSET) == 0, ("FOF_OFFSET passed"));
+
+#if OFF_MAX <= LONG_MAX
+	if ((flags & FOF_NOLOCK) != 0) {
+		if ((flags & FOF_NOUPDATE) == 0)
+			fp->f_offset = val;
+		if ((flags & FOF_NEXTOFF) != 0)
+			fp->f_nextoff = val;
+		return;
+	}
+#endif
+
+	mtxp = mtx_pool_find(mtxpool_sleep, fp);
+	mtx_lock(mtxp);
+	if ((flags & FOF_NOUPDATE) == 0)
+		fp->f_offset = val;
+	if ((flags & FOF_NEXTOFF) != 0)
+		fp->f_nextoff = val;
+	if ((flags & FOF_NOLOCK) == 0) {
+		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;
 	}
-	fp->f_vnread_flags |= FOFFSET_LOCKED;
-	uio->uio_offset = fp->f_offset;
 	mtx_unlock(mtxp);
 }
 
+void
+foffset_lock_uio(struct file *fp, struct uio *uio, int flags)
+{
+
+	if ((flags & FOF_OFFSET) == 0)
+		uio->uio_offset = foffset_lock(fp, flags);
+}
+
+void
+foffset_unlock_uio(struct file *fp, struct uio *uio, int flags)
+{
+
+	if ((flags & FOF_OFFSET) == 0)
+		foffset_unlock(fp, uio->uio_offset, flags);
+}
+
 static int
 get_advice(struct file *fp, struct uio *uio)
 {
@@ -555,23 +616,6 @@ get_advice(struct file *fp, struct uio *
 	return (ret);
 }
 
-static void
-foffset_unlock(struct file *fp, struct uio *uio, int flags)
-{
-	struct mtx *mtxp;
-
-	if ((flags & FOF_OFFSET) != 0)
-		return;
-
-	fp->f_offset = uio->uio_offset;
-	mtxp = mtx_pool_find(mtxpool_sleep, fp);
-	mtx_lock(mtxp);
-	if (fp->f_vnread_flags & FOFFSET_LOCK_WAITING)
-		wakeup(&fp->f_vnread_flags);
-	fp->f_vnread_flags = 0;
-	mtx_unlock(mtxp);
-}
-
 /*
  * File table vnode read routine.
  */
@@ -850,7 +894,7 @@ vn_io_fault(struct file *fp, struct uio 
 	else
 		doio = vn_write;
 	vp = fp->f_vnode;
-	foffset_lock(fp, uio, flags);
+	foffset_lock_uio(fp, uio, flags);
 
 	if (uio->uio_segflg != UIO_USERSPACE || vp->v_type != VREG ||
 	    ((mp = vp->v_mount) != NULL &&
@@ -967,7 +1011,7 @@ out:
 	vn_rangelock_unlock(vp, rl_cookie);
 	free(uio_clone, M_IOV);
 out_last:
-	foffset_unlock(fp, uio, flags);
+	foffset_unlock_uio(fp, uio, flags);
 	return (error);
 }
 

Modified: stable/9/sys/sys/file.h
==============================================================================
--- stable/9/sys/sys/file.h	Wed Aug 29 15:41:17 2012	(r239842)
+++ stable/9/sys/sys/file.h	Wed Aug 29 15:45:58 2012	(r239843)
@@ -72,10 +72,25 @@ struct socket;
 struct file;
 struct ucred;
 
+#define	FOF_OFFSET	0x01	/* Use the offset in uio argument */
+#define	FOF_NOLOCK	0x02	/* Do not take FOFFSET_LOCK */
+#define	FOF_NEXTOFF	0x04	/* Also update f_nextoff */
+#define	FOF_NOUPDATE	0x10	/* Do not update f_offset */
+off_t foffset_lock(struct file *fp, int flags);
+void foffset_lock_uio(struct file *fp, struct uio *uio, int flags);
+void foffset_unlock(struct file *fp, off_t val, int flags);
+void foffset_unlock_uio(struct file *fp, struct uio *uio, int flags);
+
+static inline off_t
+foffset_get(struct file *fp)
+{
+
+	return (foffset_lock(fp, FOF_NOLOCK));
+}
+
 typedef int fo_rdwr_t(struct file *fp, struct uio *uio,
 		    struct ucred *active_cred, int flags,
 		    struct thread *td);
-#define	FOF_OFFSET	1	/* Use the offset in uio argument */
 typedef	int fo_truncate_t(struct file *fp, off_t length,
 		    struct ucred *active_cred, struct thread *td);
 typedef	int fo_ioctl_t(struct file *fp, u_long com, void *data,

Modified: stable/9/sys/ufs/ffs/ffs_alloc.c
==============================================================================
--- stable/9/sys/ufs/ffs/ffs_alloc.c	Wed Aug 29 15:41:17 2012	(r239842)
+++ stable/9/sys/ufs/ffs/ffs_alloc.c	Wed Aug 29 15:45:58 2012	(r239843)
@@ -2871,10 +2871,9 @@ buffered_write(fp, uio, active_cred, fla
 	if (ip->i_devvp != devvp)
 		return (EINVAL);
 	fs = ip->i_fs;
+	foffset_lock_uio(fp, uio, flags);
 	vfslocked = VFS_LOCK_GIANT(ip->i_vnode->v_mount);
 	vn_lock(devvp, LK_EXCLUSIVE | LK_RETRY);
-	if ((flags & FOF_OFFSET) == 0)
-		uio->uio_offset = fp->f_offset;
 #ifdef DEBUG
 	if (fsckcmds) {
 		printf("%s: buffered write for block %jd\n",
@@ -2899,11 +2898,9 @@ buffered_write(fp, uio, active_cred, fla
 		goto out;
 	}
 	error = bwrite(bp);
-	if ((flags & FOF_OFFSET) == 0)
-		fp->f_offset = uio->uio_offset;
-	fp->f_nextoff = uio->uio_offset;
 out:
 	VOP_UNLOCK(devvp, 0);
 	VFS_UNLOCK_GIANT(vfslocked);
+	foffset_unlock_uio(fp, uio, flags | FOF_NEXTOFF);
 	return (error);
 }



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201208291545.q7TFjxX0070678>