Subject: Re: Use vn_rdwr in vnd (Re: kern/34882)
To: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
From: Julio M. Merino Vidal <jmmv84@gmail.com>
List: tech-kern
Date: 11/10/2006 15:08:16
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?

> > 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.

> or how it can be fixed
> by incrementing v_numoutput.  can you explain?

If you remove that, as soon as you run the newfs command above the
system will panic with a "neg numoutput".

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.

-- 
Julio M. Merino Vidal <jmmv84@gmail.com>
The Julipedia - http://julipedia.blogspot.com/