Skip site navigation (1)Skip section navigation (2) 
Date:      Wed, 29 Aug 2012 15:41:17 +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: r239842 - stable/9/sys/kern
Message-ID:  <201208291541.q7TFfHsR070051@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: kib
Date: Wed Aug 29 15:41:17 2012
New Revision: 239842
URL: http://svn.freebsd.org/changeset/base/239842

Log:
  MFC r237365:
  Fix locking for f_offset, vn_read() and vn_write() cases only.

Modified:
  stable/9/sys/kern/vfs_vnops.c
Directory Properties:
  stable/9/sys/   (props changed)

Modified: stable/9/sys/kern/vfs_vnops.c
==============================================================================
--- stable/9/sys/kern/vfs_vnops.c	Wed Aug 29 15:38:18 2012	(r239841)
+++ stable/9/sys/kern/vfs_vnops.c	Wed Aug 29 15:41:17 2012	(r239842)
@@ -512,6 +512,66 @@ vn_rdwr_inchunks(rw, vp, base, len, offs
 	return (error);
 }
 
+static void
+foffset_lock(struct file *fp, struct uio *uio, int flags)
+{
+	struct mtx *mtxp;
+
+	if ((flags & FOF_OFFSET) != 0)
+		return;
+
+	/*
+	 * According to McKusick the vn lock was protecting f_offset here.
+	 * It is now protected by the FOFFSET_LOCKED flag.
+	 */
+	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);
+	}
+	fp->f_vnread_flags |= FOFFSET_LOCKED;
+	uio->uio_offset = fp->f_offset;
+	mtx_unlock(mtxp);
+}
+
+static int
+get_advice(struct file *fp, struct uio *uio)
+{
+	struct mtx *mtxp;
+	int ret;
+
+	ret = POSIX_FADV_NORMAL;
+	if (fp->f_advice == NULL)
+		return (ret);
+
+	mtxp = mtx_pool_find(mtxpool_sleep, fp);
+	mtx_lock(mtxp);
+	if (uio->uio_offset >= fp->f_advice->fa_start &&
+	    uio->uio_offset + uio->uio_resid <= fp->f_advice->fa_end)
+		ret = fp->f_advice->fa_advice;
+	mtx_unlock(mtxp);
+	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.
  */
@@ -524,44 +584,22 @@ vn_read(fp, uio, active_cred, flags, td)
 	struct thread *td;
 {
 	struct vnode *vp;
-	int error, ioflag;
 	struct mtx *mtxp;
+	int error, ioflag;
 	int advice, vfslocked;
 	off_t offset, start, end;
 
 	KASSERT(uio->uio_td == td, ("uio_td %p is not td %p",
 	    uio->uio_td, td));
-	mtxp = NULL;
+	KASSERT(flags & FOF_OFFSET, ("No FOF_OFFSET"));
 	vp = fp->f_vnode;
 	ioflag = 0;
 	if (fp->f_flag & FNONBLOCK)
 		ioflag |= IO_NDELAY;
 	if (fp->f_flag & O_DIRECT)
 		ioflag |= IO_DIRECT;
-	advice = POSIX_FADV_NORMAL;
+	advice = get_advice(fp, uio);
 	vfslocked = VFS_LOCK_GIANT(vp->v_mount);
-	/*
-	 * According to McKusick the vn lock was protecting f_offset here.
-	 * It is now protected by the FOFFSET_LOCKED flag.
-	 */
-	if ((flags & FOF_OFFSET) == 0 || fp->f_advice != NULL) {
-		mtxp = mtx_pool_find(mtxpool_sleep, fp);
-		mtx_lock(mtxp);
-		if ((flags & FOF_OFFSET) == 0) {
-			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);
-			}
-			fp->f_vnread_flags |= FOFFSET_LOCKED;
-			uio->uio_offset = fp->f_offset;
-		}
-		if (fp->f_advice != NULL &&
-		    uio->uio_offset >= fp->f_advice->fa_start &&
-		    uio->uio_offset + uio->uio_resid <= fp->f_advice->fa_end)
-			advice = fp->f_advice->fa_advice;
-		mtx_unlock(mtxp);
-	}
 	vn_lock(vp, LK_SHARED | LK_RETRY);
 
 	switch (advice) {
@@ -581,14 +619,6 @@ vn_read(fp, uio, active_cred, flags, td)
 	if (error == 0)
 #endif
 		error = VOP_READ(vp, uio, ioflag, fp->f_cred);
-	if ((flags & FOF_OFFSET) == 0) {
-		fp->f_offset = uio->uio_offset;
-		mtx_lock(mtxp);
-		if (fp->f_vnread_flags & FOFFSET_LOCK_WAITING)
-			wakeup(&fp->f_vnread_flags);
-		fp->f_vnread_flags = 0;
-		mtx_unlock(mtxp);
-	}
 	fp->f_nextoff = uio->uio_offset;
 	VOP_UNLOCK(vp, 0);
 	if (error == 0 && advice == POSIX_FADV_NOREUSE &&
@@ -610,6 +640,7 @@ vn_read(fp, uio, active_cred, flags, td)
 		 */
 		start = offset;
 		end = uio->uio_offset - 1;
+		mtxp = mtx_pool_find(mtxpool_sleep, fp);
 		mtx_lock(mtxp);
 		if (fp->f_advice != NULL &&
 		    fp->f_advice->fa_advice == POSIX_FADV_NOREUSE) {
@@ -641,13 +672,14 @@ vn_write(fp, uio, active_cred, flags, td
 {
 	struct vnode *vp;
 	struct mount *mp;
-	int error, ioflag, lock_flags;
 	struct mtx *mtxp;
+	int error, ioflag, lock_flags;
 	int advice, vfslocked;
 	off_t offset, start, end;
 
 	KASSERT(uio->uio_td == td, ("uio_td %p is not td %p",
 	    uio->uio_td, td));
+	KASSERT(flags & FOF_OFFSET, ("No FOF_OFFSET"));
 	vp = fp->f_vnode;
 	vfslocked = VFS_LOCK_GIANT(vp->v_mount);
 	if (vp->v_type == VREG)
@@ -666,6 +698,8 @@ vn_write(fp, uio, active_cred, flags, td
 	if (vp->v_type != VCHR &&
 	    (error = vn_start_write(vp, &mp, V_WAIT | PCATCH)) != 0)
 		goto unlock;
+
+	advice = get_advice(fp, uio);
  
 	if ((MNT_SHARED_WRITES(mp) ||
 	    ((mp == NULL) && MNT_SHARED_WRITES(vp->v_mount))) &&
@@ -676,19 +710,6 @@ vn_write(fp, uio, active_cred, flags, td
 	}
 
 	vn_lock(vp, lock_flags | LK_RETRY);
-	if ((flags & FOF_OFFSET) == 0)
-		uio->uio_offset = fp->f_offset;
-	advice = POSIX_FADV_NORMAL;
-	mtxp = NULL;
-	if (fp->f_advice != NULL) {
-		mtxp = mtx_pool_find(mtxpool_sleep, fp);
-		mtx_lock(mtxp);
-		if (fp->f_advice != NULL &&
-		    uio->uio_offset >= fp->f_advice->fa_start &&
-		    uio->uio_offset + uio->uio_resid <= fp->f_advice->fa_end)
-			advice = fp->f_advice->fa_advice;
-		mtx_unlock(mtxp);
-	}
 	switch (advice) {
 	case POSIX_FADV_NORMAL:
 	case POSIX_FADV_SEQUENTIAL:
@@ -706,8 +727,6 @@ vn_write(fp, uio, active_cred, flags, td
 	if (error == 0)
 #endif
 		error = VOP_WRITE(vp, uio, ioflag, fp->f_cred);
-	if ((flags & FOF_OFFSET) == 0)
-		fp->f_offset = uio->uio_offset;
 	fp->f_nextoff = uio->uio_offset;
 	VOP_UNLOCK(vp, 0);
 	if (vp->v_type != VCHR)
@@ -746,6 +765,7 @@ vn_write(fp, uio, active_cred, flags, td
 		 */
 		start = offset;
 		end = uio->uio_offset - 1;
+		mtxp = mtx_pool_find(mtxpool_sleep, fp);
 		mtx_lock(mtxp);
 		if (fp->f_advice != NULL &&
 		    fp->f_advice->fa_advice == POSIX_FADV_NOREUSE) {
@@ -830,11 +850,15 @@ vn_io_fault(struct file *fp, struct uio 
 	else
 		doio = vn_write;
 	vp = fp->f_vnode;
+	foffset_lock(fp, uio, flags);
+
 	if (uio->uio_segflg != UIO_USERSPACE || vp->v_type != VREG ||
 	    ((mp = vp->v_mount) != NULL &&
 	    (mp->mnt_kern_flag & MNTK_NO_IOPF) == 0) ||
-	    !vn_io_fault_enable)
-		return (doio(fp, uio, active_cred, flags, td));
+	    !vn_io_fault_enable) {
+		error = doio(fp, uio, active_cred, flags | FOF_OFFSET, td);
+		goto out_last;
+	}
 
 	/*
 	 * The UFS follows IO_UNIT directive and replays back both
@@ -867,7 +891,7 @@ vn_io_fault(struct file *fp, struct uio 
 	}
 
 	save = vm_fault_disable_pagefaults();
-	error = doio(fp, uio, active_cred, flags, td);
+	error = doio(fp, uio, active_cred, flags | FOF_OFFSET, td);
 	if (error != EFAULT)
 		goto out;
 
@@ -918,7 +942,8 @@ vn_io_fault(struct file *fp, struct uio 
 		td->td_ma = ma;
 		td->td_ma_cnt = cnt;
 
-		error = doio(fp, &short_uio, active_cred, flags, td);
+		error = doio(fp, &short_uio, active_cred, flags | FOF_OFFSET,
+		    td);
 		vm_page_unhold_pages(ma, cnt);
 		adv = len - short_uio.uio_resid;
 
@@ -941,6 +966,8 @@ out:
 	vm_fault_enable_pagefaults(save);
 	vn_rangelock_unlock(vp, rl_cookie);
 	free(uio_clone, M_IOV);
+out_last:
+	foffset_unlock(fp, uio, flags);
 	return (error);
 }

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