--NextPart-20061111152133-2440400
Content-Type: Text/Plain; charset=us-ascii

> On 11/10/06, YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp> wrote:
> > > On 11/9/06, YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp> wrote:
> > > > > +     /* Flush the vnode if requested. */
> > > > > +     if (obp->b_flags & B_VFLUSH)
> > > > > +             vflushbuf(vp, 0);
> > > > > +
> > > > > +     /* We need to increase the number of outputs on the vnode if
> > > > > +      * there was any write to it (either due to a real write or due
> > > > > +      * to a flush). */
> > > > > +     if (!doread || obp->b_flags & B_VFLUSH)
> > > > > +             vp->v_numoutput++;
> > > >
> > > > can you explain these B_VFLUSH handling?
> > >
> > > For the first part I just tried to flush the vnode.  I don't remember
> > > how I ended exactly in vflushbuf, but it *seemed* reasonable...
> > > Hmm... maybe one ought to call VOP_FSYNC instead?  (And maybe this
> > > handles the case below too.)
> >
> > why do you want to flush the underlying vnode here at all?
> > if you want to preserve stable storage semantics or such,
> > it should be done regardless of B_VFLUSH.
> 
> Because in the non-rw case, B_VFLUSH is passed down to the disk driver
> and I guess that this will do a sync, isn't it?

what's "non-rw case"?

> > > Aside that, the driver was getting read requests that had the B_VFLUSH
> > > bit active. Later on a "neg numoutput" panic happened which I was able
> > > to fix by incrementing v_numoutput.  Yes, that looks extremely ugly
> > > (at least to me) so, please, if you have any better idea on how to fix
> > > it tell me and I'll take a look.
> >
> > i don't know how read requests with B_VFLUSH happens
> 
> Just try a newfs over the vnd device and you'll get into
> handle_with_rw with B_READ and B_VFLUSH set.

i tried the following with the attached patch, but got no panics.

		mount_tmpfs a /mnt
		cd /mnt
		dd if=/dev/zero of=x bs=1m count=1
		vnconfig vnd0 x
		newfs -F /dev/rvnd0d

> I checked other drivers and they seem to do similar things.  See
> ccd.c:770.  Oh, and now I see there is a V_INCR_NUMOUTPUT macro to do
> this, although it is not always used.  It is also preceded by a
> "useful" comment.

they are for !B_READ requests, aren't they?

YAMAMOTO Takashi

--NextPart-20061111152133-2440400
Content-Type: Text/Plain; charset=us-ascii
Content-Disposition: attachment; filename="a.diff"

Index: vnd.c
===================================================================
--- vnd.c	(revision 1895)
+++ vnd.c	(working copy)
@@ -685,6 +685,11 @@ handle_with_rdwr(struct vnd_softc *vnd, 
 	offset = obp->b_rawblkno * vnd->sc_dkdev.dk_label->d_secsize;
 	vp = vnd->sc_vp;
 
+	if (((obp->b_flags | bp->b_flags) & (B_READ|B_VFLUSH))
+	    == (B_READ|B_VFLUSH)) {
+		panic("%s: obp %p bp %p", __func__, obp, bp);
+	}
+
 #if defined(DEBUG)
 	if (vnddebug & VDB_IO)
 		printf("vnd (rdwr): vp %p, %s, rawblkno 0x%" PRIx64

--NextPart-20061111152133-2440400--