Date: Mon, 26 Jan 2015 12:10:53 +0100
From: Manuel Bouyer <[email protected]>
I'm not sure I understand; the "bump v_numoutput" is always needed;
but if skipped is 0 the last nestiobuf_done() call is done by
biodone() callback and not by the called. So the bump has to happen
before the last buf is queued.
Ah, yes, sorry -- we need one bump for every nested buf that will be
queued and biodone'd, and always one bump for the parent which will
always be biodone'd either by a nested buf or by nestiobuf_done on the
parent.
However, I think there's a bug in your new patch too: suppose VOP_BMAP
fails on one of the middle blocks -- then we won't call VOP_STRATEGY
for the last block even if VOP_BMAP succeeds for that block.
I believe the attached patch will fix that, but I haven't tested it.
1. Guarantee to call VOP_STRATEGY on every nestiobuf.
2. Guarantee no blocking between nestiobuf_setup or v_numoutput++ and
the corresponding VOP_STRATEGY or nestiobuf_done.
3. Guarantee one nestiobuf_setup or v_numoutput++ per VOP_STRATEGY or
nestiobuf_done.
diff -c /home/riastradh/netbsd/nbdev/src/sys/dev/vnd.c
/tmp/riastradh/buffer-content-14028wVF
--- /home/riastradh/netbsd/nbdev/src/sys/dev/vnd.c 2015-01-07
01:37:45.000000000 +0000
+++ /tmp/riastradh/buffer-content-14028wVF 2015-01-26 18:49:09.000000000
+0000
@@ -795,6 +795,7 @@
size_t resid, sz;
off_t bn, offset;
struct vnode *vp;
+ struct buf *nbp = NULL;
flags = obp->b_flags;
@@ -822,10 +823,14 @@
bp->b_resid = bp->b_bcount;
for (offset = 0, resid = bp->b_resid; resid;
resid -= sz, offset += sz) {
- struct buf *nbp;
daddr_t nbn;
int off, nra;
+ if (nbp) {
+ VOP_STRATEGY(vp, nbp);
+ nbp = NULL;
+ }
+
nra = 0;
vn_lock(vnd->sc_vp, LK_EXCLUSIVE | LK_RETRY);
error = VOP_BMAP(vnd->sc_vp, bn / bsize, &vp, &nbn, &nra);
@@ -862,6 +867,7 @@
nbn, sz);
#endif
+ KASSERT(nbp == NULL);
nbp = getiobuf(vp, true);
nestiobuf_setup(bp, nbp, offset, sz);
nbp->b_blkno = nbn + btodb(off);
@@ -875,9 +881,21 @@
nbp->vb_buf.b_flags, nbp->vb_buf.b_data,
nbp->vb_buf.b_bcount);
#endif
- VOP_STRATEGY(vp, nbp);
bn += sz;
}
+ /*
+ * Bump v_numoutput for the nested I/O master only after we
+ * have done all the blocking we will do and before submitting
+ * the last buf to disk.
+ */
+ if (!(flags & B_READ)) {
+ vp = bp->b_vp;
+ mutex_enter(vp->v_interlock);
+ vp->v_numoutput++;
+ mutex_exit(vp->v_interlock);
+ }
+ if (nbp)
+ VOP_STRATEGY(vp, nbp);
nestiobuf_done(bp, skipped, error);
}
Diff finished. Mon Jan 26 18:49:09 2015
