This patch reestablishes the spec_fsync() guarentee that synchronous

fsyncs, which typically occur during unmounting, will drain all dirty
buffers even if it takes multiple passes to do so.  The guarentee was
mangled by the last patch which solved a problem due to -current disabling
interrupts while holding giant (which caused an infinite spin loop waiting for
I/O to complete).  -stable does not have either patch, but has a similar
bug in the original spec_fsync() code which is triggered by a bug in the
softupdates umount code, a fix for which will be committed to -current
as soon as Kirk stamps it.  Then both solutions will be MFC'd to -stable.

-stable currently suffers from a combination of the softupdates bug and
a small window of opportunity in the original spec_fsync() code, and -stable
also suffers from the spin-loop bug but since interrupts are enabled the
spin resolves itself in a few milliseconds.

2 files changed, 32 insertions, 14 deletions

diff --git a/sys/fs/specfs/spec_vnops.c b/sys/fs/specfs/spec_vnops.c
index ba46558..6b33df4 100644
--- a/sys/fs/specfs/spec_vnops.c
+++ b/sys/fs/specfs/spec_vnops.c
@@ -347,10 +347,12 @@ spec_fsync(ap)
 	struct buf *bp;
 	struct buf *nbp;
 	int s;
+	int maxretry = 10000;	/* large, arbitrarily chosen */
 
 	if (!vn_isdisk(vp, NULL))
 		return (0);
 
+loop1:
 	/*
 	 * MARK/SCAN initialization to avoid infinite loops
 	 */
@@ -364,7 +366,7 @@ spec_fsync(ap)
 	/*
 	 * Flush all dirty buffers associated with a block device.
 	 */
-loop:
+loop2:
 	s = splbio();
 	for (bp = TAILQ_FIRST(&vp->v_dirtyblkhd); bp; bp = nbp) {
 		nbp = TAILQ_NEXT(bp, b_vnbufs);
@@ -384,20 +386,27 @@ loop:
 			splx(s);
 			bawrite(bp);
 		}
-		goto loop;
+		goto loop2;
 	}
+
+	/*
+	 * If synchronous the caller expects us to completely resolve all
+	 * dirty buffers in the system.  Wait for in-progress I/O to
+	 * complete (which could include background bitmap writes), then
+	 * retry if dirty blocks still exist.
+	 */
 	if (ap->a_waitfor == MNT_WAIT) {
 		while (vp->v_numoutput) {
 			vp->v_flag |= VBWAIT;
 			(void) tsleep((caddr_t)&vp->v_numoutput, PRIBIO + 1, "spfsyn", 0);
 		}
-#ifdef DIAGNOSTIC
 		if (!TAILQ_EMPTY(&vp->v_dirtyblkhd)) {
-			vprint("spec_fsync: dirty", vp);
-			splx(s);
-			goto loop;
+			if (--maxretry != 0) {
+				splx(s);
+				goto loop1;
+			}
+			vprint("spec_fsync: giving up on dirty", vp);
 		}
-#endif
 	}
 	splx(s);
 	return (0);
diff --git a/sys/miscfs/specfs/spec_vnops.c b/sys/miscfs/specfs/spec_vnops.c
index ba46558..6b33df4 100644
--- a/sys/miscfs/specfs/spec_vnops.c
+++ b/sys/miscfs/specfs/spec_vnops.c
@@ -347,10 +347,12 @@ spec_fsync(ap)
 	struct buf *bp;
 	struct buf *nbp;
 	int s;
+	int maxretry = 10000;	/* large, arbitrarily chosen */
 
 	if (!vn_isdisk(vp, NULL))
 		return (0);
 
+loop1:
 	/*
 	 * MARK/SCAN initialization to avoid infinite loops
 	 */
@@ -364,7 +366,7 @@ spec_fsync(ap)
 	/*
 	 * Flush all dirty buffers associated with a block device.
 	 */
-loop:
+loop2:
 	s = splbio();
 	for (bp = TAILQ_FIRST(&vp->v_dirtyblkhd); bp; bp = nbp) {
 		nbp = TAILQ_NEXT(bp, b_vnbufs);
@@ -384,20 +386,27 @@ loop:
 			splx(s);
 			bawrite(bp);
 		}
-		goto loop;
+		goto loop2;
 	}
+
+	/*
+	 * If synchronous the caller expects us to completely resolve all
+	 * dirty buffers in the system.  Wait for in-progress I/O to
+	 * complete (which could include background bitmap writes), then
+	 * retry if dirty blocks still exist.
+	 */
 	if (ap->a_waitfor == MNT_WAIT) {
 		while (vp->v_numoutput) {
 			vp->v_flag |= VBWAIT;
 			(void) tsleep((caddr_t)&vp->v_numoutput, PRIBIO + 1, "spfsyn", 0);
 		}
-#ifdef DIAGNOSTIC
 		if (!TAILQ_EMPTY(&vp->v_dirtyblkhd)) {
-			vprint("spec_fsync: dirty", vp);
-			splx(s);
-			goto loop;
+			if (--maxretry != 0) {
+				splx(s);
+				goto loop1;
+			}
+			vprint("spec_fsync: giving up on dirty", vp);
 		}
-#endif
 	}
 	splx(s);
 	return (0);