Subject: Re: Use vn_rdwr in vnd (Re: kern/34882)
To: None <jmmv@NetBSD.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org,>
From: Julio M. Merino Vidal <jmmv84@gmail.com>
List: netbsd-bugs
Date: 11/11/2006 08:40:02
The following reply was made to PR kern/34882; it has been noted by GNATS.

From: "Julio M. Merino Vidal" <jmmv84@gmail.com>
To: "YAMAMOTO Takashi" <yamt@mwd.biglobe.ne.jp>
Cc: tech-kern@netbsd.org, gnats-bugs@NetBSD.org
Subject: Re: Use vn_rdwr in vnd (Re: kern/34882)
Date: Sat, 11 Nov 2006 09:38:56 +0100

 On 11/11/06, YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp> wrote:
 > > On 11/10/06, YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp> wrote:
 > > > > On 11/9/06, YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp> wrote:
 > > > 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"?
 
 As I said, a read with a flush.  But see below.
 
 > > > 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
 
 Of course not because that's what the increment of numoutput is for
 (or was supposed to be for).
 
 > > 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?
 
 Hmm, interesting... I've changed this:
 
 	if (!doread || obp->b_flags & B_VFLUSH)
 		vp->v_numoutput++;
 
 to this:
 
 	printf("%s, %s\n", doread ? "read" : "write",
 	    obp->b_flags & B_VFLUSH ? "flush" : "noflush");
 	if (!doread)
 		vp->v_numoutput++;
 
 And it does not panic.  I'm sure it did before!  And I'm sure I got
 "read, sync" messages from the debug printf.  (But maybe it was
 because I still had some other problem somewhere in the handling of
 petitions.)
 
 So... should I remove all the B_VFLUSH handling from the function?
 Should I change the manual v_numoutput increment to use the
 V_INCR_NUMOUTPUT macro?
 
 Thank you.
 
 -- 
 Julio M. Merino Vidal <jmmv84@gmail.com>
 The Julipedia - http://julipedia.blogspot.com/