From 3f5b28bc07debb993446b238e1b1178a3c30dfd1 Mon Sep 17 00:00:00 2001
From: Kirk McKusick <mckusick@FreeBSD.org>
Date: Sun, 9 Jan 2000 23:14:57 +0000
Subject: [PATCH] Reorganize softdep_fsync so that it only does the
 inode-is-flushed check before the inode is unlocked while grabbing its parent
 directory. Once it is unlocked, other operations may slip in that could make
 the inode-is-flushed check fail. Allowing other writes to the inode before
 returning from fsync does not break the semantics of fsync since we have
 flushed everything that was dirty at the time of the fsync call.

---
 sys/contrib/softupdates/ffs_softdep.c | 48 ++++++++++++---------------
 sys/ufs/ffs/ffs_softdep.c             | 48 ++++++++++++---------------
 2 files changed, 44 insertions(+), 52 deletions(-)

diff --git a/sys/contrib/softupdates/ffs_softdep.c b/sys/contrib/softupdates/ffs_softdep.c
index 396cd48d50b8..fde758b8f7fa 100644
--- a/sys/contrib/softupdates/ffs_softdep.c
+++ b/sys/contrib/softupdates/ffs_softdep.c
@@ -52,7 +52,7 @@
  * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  *
- *	from: @(#)ffs_softdep.c	9.43 (McKusick) 1/9/00
+ *	from: @(#)ffs_softdep.c	9.44 (McKusick) 1/9/00
  * $FreeBSD$
  */
 
@@ -3587,44 +3587,39 @@ static int
 softdep_fsync(vp)
 	struct vnode *vp;	/* the "in_core" copy of the inode */
 {
-	struct diradd *dap, *olddap;
 	struct inodedep *inodedep;
 	struct pagedep *pagedep;
 	struct worklist *wk;
+	struct diradd *dap;
 	struct mount *mnt;
 	struct vnode *pvp;
 	struct inode *ip;
 	struct buf *bp;
 	struct fs *fs;
 	struct proc *p = CURPROC;		/* XXX */
-	int error, ret, flushparent;
+	int error, flushparent;
 	ino_t parentino;
 	ufs_lbn_t lbn;
 
 	ip = VTOI(vp);
 	fs = ip->i_fs;
-	for (error = 0, flushparent = 0, olddap = NULL; ; ) {
-		ACQUIRE_LOCK(&lk);
-		if (inodedep_lookup(fs, ip->i_number, 0, &inodedep) == 0)
-			break;
-		if (LIST_FIRST(&inodedep->id_inowait) != NULL ||
-		    LIST_FIRST(&inodedep->id_bufwait) != NULL ||
-		    TAILQ_FIRST(&inodedep->id_inoupdt) != NULL ||
-		    TAILQ_FIRST(&inodedep->id_newinoupdt) != NULL)
-			panic("softdep_fsync: pending ops");
+	ACQUIRE_LOCK(&lk);
+	if (inodedep_lookup(fs, ip->i_number, 0, &inodedep) == 0) {
+		FREE_LOCK(&lk);
+		return (0);
+	}
+	if (LIST_FIRST(&inodedep->id_inowait) != NULL ||
+	    LIST_FIRST(&inodedep->id_bufwait) != NULL ||
+	    TAILQ_FIRST(&inodedep->id_inoupdt) != NULL ||
+	    TAILQ_FIRST(&inodedep->id_newinoupdt) != NULL)
+		panic("softdep_fsync: pending ops");
+	for (error = 0, flushparent = 0; ; ) {
 		if ((wk = LIST_FIRST(&inodedep->id_pendinghd)) == NULL)
 			break;
 		if (wk->wk_type != D_DIRADD)
 			panic("softdep_fsync: Unexpected type %s",
 			    TYPENAME(wk->wk_type));
 		dap = WK_DIRADD(wk);
-		/*
-		 * If we have failed to get rid of all the dependencies
-		 * then something is seriously wrong.
-		 */
-		if (dap == olddap)
-			panic("softdep_fsync: flush failed");
-		olddap = dap;
 		/*
 		 * Flush our parent if this directory entry
 		 * has a MKDIR_PARENT dependency.
@@ -3658,11 +3653,10 @@ softdep_fsync(vp)
 		 */
 		FREE_LOCK(&lk);
 		VOP_UNLOCK(vp, 0, p);
-		if ((error = VFS_VGET(mnt, parentino, &pvp)) != 0) {
-			vn_lock(vp, LK_EXCLUSIVE | LK_RETRY, p);
-			return (error);
-		}
+		error = VFS_VGET(mnt, parentino, &pvp);
 		vn_lock(vp, LK_EXCLUSIVE | LK_RETRY, p);
+		if (error != 0)
+			return (error);
 		if (flushparent) {
 			if ((error = UFS_UPDATE(pvp, 1)) != 0) {
 				vput(pvp);
@@ -3674,12 +3668,14 @@ softdep_fsync(vp)
 		 */
 		error = bread(pvp, lbn, blksize(fs, VTOI(pvp), lbn), p->p_ucred,
 		    &bp);
-		ret = VOP_BWRITE(bp->b_vp, bp);
+		if (error == 0)
+			error = VOP_BWRITE(bp->b_vp, bp);
 		vput(pvp);
 		if (error != 0)
 			return (error);
-		if (ret != 0)
-			return (ret);
+		ACQUIRE_LOCK(&lk);
+		if (inodedep_lookup(fs, ip->i_number, 0, &inodedep) == 0)
+			break;
 	}
 	FREE_LOCK(&lk);
 	return (0);
diff --git a/sys/ufs/ffs/ffs_softdep.c b/sys/ufs/ffs/ffs_softdep.c
index 396cd48d50b8..fde758b8f7fa 100644
--- a/sys/ufs/ffs/ffs_softdep.c
+++ b/sys/ufs/ffs/ffs_softdep.c
@@ -52,7 +52,7 @@
  * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  *
- *	from: @(#)ffs_softdep.c	9.43 (McKusick) 1/9/00
+ *	from: @(#)ffs_softdep.c	9.44 (McKusick) 1/9/00
  * $FreeBSD$
  */
 
@@ -3587,44 +3587,39 @@ static int
 softdep_fsync(vp)
 	struct vnode *vp;	/* the "in_core" copy of the inode */
 {
-	struct diradd *dap, *olddap;
 	struct inodedep *inodedep;
 	struct pagedep *pagedep;
 	struct worklist *wk;
+	struct diradd *dap;
 	struct mount *mnt;
 	struct vnode *pvp;
 	struct inode *ip;
 	struct buf *bp;
 	struct fs *fs;
 	struct proc *p = CURPROC;		/* XXX */
-	int error, ret, flushparent;
+	int error, flushparent;
 	ino_t parentino;
 	ufs_lbn_t lbn;
 
 	ip = VTOI(vp);
 	fs = ip->i_fs;
-	for (error = 0, flushparent = 0, olddap = NULL; ; ) {
-		ACQUIRE_LOCK(&lk);
-		if (inodedep_lookup(fs, ip->i_number, 0, &inodedep) == 0)
-			break;
-		if (LIST_FIRST(&inodedep->id_inowait) != NULL ||
-		    LIST_FIRST(&inodedep->id_bufwait) != NULL ||
-		    TAILQ_FIRST(&inodedep->id_inoupdt) != NULL ||
-		    TAILQ_FIRST(&inodedep->id_newinoupdt) != NULL)
-			panic("softdep_fsync: pending ops");
+	ACQUIRE_LOCK(&lk);
+	if (inodedep_lookup(fs, ip->i_number, 0, &inodedep) == 0) {
+		FREE_LOCK(&lk);
+		return (0);
+	}
+	if (LIST_FIRST(&inodedep->id_inowait) != NULL ||
+	    LIST_FIRST(&inodedep->id_bufwait) != NULL ||
+	    TAILQ_FIRST(&inodedep->id_inoupdt) != NULL ||
+	    TAILQ_FIRST(&inodedep->id_newinoupdt) != NULL)
+		panic("softdep_fsync: pending ops");
+	for (error = 0, flushparent = 0; ; ) {
 		if ((wk = LIST_FIRST(&inodedep->id_pendinghd)) == NULL)
 			break;
 		if (wk->wk_type != D_DIRADD)
 			panic("softdep_fsync: Unexpected type %s",
 			    TYPENAME(wk->wk_type));
 		dap = WK_DIRADD(wk);
-		/*
-		 * If we have failed to get rid of all the dependencies
-		 * then something is seriously wrong.
-		 */
-		if (dap == olddap)
-			panic("softdep_fsync: flush failed");
-		olddap = dap;
 		/*
 		 * Flush our parent if this directory entry
 		 * has a MKDIR_PARENT dependency.
@@ -3658,11 +3653,10 @@ softdep_fsync(vp)
 		 */
 		FREE_LOCK(&lk);
 		VOP_UNLOCK(vp, 0, p);
-		if ((error = VFS_VGET(mnt, parentino, &pvp)) != 0) {
-			vn_lock(vp, LK_EXCLUSIVE | LK_RETRY, p);
-			return (error);
-		}
+		error = VFS_VGET(mnt, parentino, &pvp);
 		vn_lock(vp, LK_EXCLUSIVE | LK_RETRY, p);
+		if (error != 0)
+			return (error);
 		if (flushparent) {
 			if ((error = UFS_UPDATE(pvp, 1)) != 0) {
 				vput(pvp);
@@ -3674,12 +3668,14 @@ softdep_fsync(vp)
 		 */
 		error = bread(pvp, lbn, blksize(fs, VTOI(pvp), lbn), p->p_ucred,
 		    &bp);
-		ret = VOP_BWRITE(bp->b_vp, bp);
+		if (error == 0)
+			error = VOP_BWRITE(bp->b_vp, bp);
 		vput(pvp);
 		if (error != 0)
 			return (error);
-		if (ret != 0)
-			return (ret);
+		ACQUIRE_LOCK(&lk);
+		if (inodedep_lookup(fs, ip->i_number, 0, &inodedep) == 0)
+			break;
 	}
 	FREE_LOCK(&lk);
 	return (0);