Re: vnode lock and v_numoutput

[Manuel Bouyer <bouyer%antioche.eu.org@localhost>]

On Mon, Jan 26, 2015 at 12:10:53PM +0100, Manuel Bouyer wrote:
> [...]
> 
> I agree that hidding this to the nestiobuf API is probably the right thing
> to do. But I'm also looking for a fix that is safe to pull up to netbsd-5
> and netbsd-6 (the problem was first noticed on netbsd-5, and I reproduced
> it, and tested the patch, on netbsd-6). So if the attached
> patch looks semantically correct, I'll commit it and request pullups.
> Then I'll look at a better fix that can be commited to HEAD pulled up
> to netbsd-7.

with the patch this time ...

-- 
Manuel Bouyer <bouyer%antioche.eu.org@localhost>
     NetBSD: 26 ans d'experience feront toujours la difference
--

Index: vnd.c
===================================================================
RCS file: /cvsroot/src/sys/dev/vnd.c,v
retrieving revision 1.219.8.2
diff -u -p -u -r1.219.8.2 vnd.c
--- vnd.c	5 Jul 2012 18:12:46 -0000	1.219.8.2
+++ vnd.c	26 Jan 2015 11:11:40 -0000
@@ -782,15 +782,10 @@ handle_with_strategy(struct vnd_softc *v
 	size_t resid, sz;
 	off_t bn, offset;
 	struct vnode *vp;
+	struct buf *nbp;
 
 	flags = obp->b_flags;
 
-	if (!(flags & B_READ)) {
-		vp = bp->b_vp;
-		mutex_enter(vp->v_interlock);
-		vp->v_numoutput++;
-		mutex_exit(vp->v_interlock);
-	}
 
 	/* convert to a byte offset within the file. */
 	bn = obp->b_rawblkno * vnd->sc_dkdev.dk_label->d_secsize;
@@ -807,9 +802,8 @@ handle_with_strategy(struct vnd_softc *v
 	 */
 	error = 0;
 	bp->b_resid = bp->b_bcount;
-	for (offset = 0, resid = bp->b_resid; resid;
+	for (offset = 0, resid = bp->b_resid; /* true */;
 	    resid -= sz, offset += sz) {
-		struct buf *nbp;
 		daddr_t nbn;
 		int off, nra;
 
@@ -862,10 +856,33 @@ handle_with_strategy(struct vnd_softc *v
 			    nbp->vb_buf.b_flags, nbp->vb_buf.b_data,
 			    nbp->vb_buf.b_bcount);
 #endif
+		if (resid == sz) {
+			break;
+		}
 		VOP_STRATEGY(vp, nbp);
 		bn += sz;
 	}
-	nestiobuf_done(bp, skipped, error);
+	if (!(flags & B_READ)) {
+		struct vnode *w_vp;
+		/*
+		 * this is the last nested buf, account for
+		 * the parent buf write too.
+		 * This has to be done last, so that
+		 * fsync won't wait for this write which
+		 * has to chance to complete before all nested bufs
+		 * have been queued. But it has to be done
+		 * before the last VOP_STRATEGY() 
+		 * or the call to nestiobuf_done().
+		 */
+		w_vp = bp->b_vp;
+		mutex_enter(w_vp->v_interlock);
+		w_vp->v_numoutput++;
+		mutex_exit(w_vp->v_interlock);
+	}
+	if (skipped) 
+		nestiobuf_done(bp, skipped, error);
+	else 
+		VOP_STRATEGY(vp, nbp);
 }
 
 static void