Re: vwakeup: neg numoutput

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

On Tue, Jul 01, 2008 at 10:16:16AM +0000, Andrew Doran wrote:
> > I don't know how to translate this kind of construct then:
> >                 mutex_enter(&vp->v_interlock);
> >             while (vp->v_numoutput > 0)
> >                     cv_wait(&vp->v_cv, &vp->v_interlock);
> >             mutex_exit(&vp->v_interlock);
> 
> You would have to hold the lock to move v_numoutput from a non-zero value to
> to zero.

OK, it's just a matter of keeping the mutex_* around v_numoutput--

> 
> > they're used in several places. There are also places where it seems it's
> > expected to have vp->v_numoutput stable for several lines of code:
> 
> > vfs_subr2.c:
> >         mutex_enter(&vp->v_interlock);
> >             while (vp->v_numoutput != 0)
> >                     cv_wait(&vp->v_cv, &vp->v_interlock);
> >     dirty = !LIST_EMPTY(&vp->v_dirtyblkhd);
> >     mutex_exit(&vp->v_interlock);
> 
> You can't assume that with v_numoutput, it's only used to ensure that
> already in flight I/O operations have completed.

vfs_subr2.c has another place which makes me believe we can't use atomic ops
alone to incement v_numoutput:
315: KASSERT(vp->v_numoutput == 0 && LIST_EMPTY(&vp->v_dirtyblkhd));

if we can increment v_numoutput without hodling the lock, this KASSERT may
fire because v_numoutput may be non-zero here after the VOP_FSYNC().
There's also genfs_do_putpages() for which I'm not sure it's safe to
atomically increment v_numoutput without locking. 

>  
> > there's also genfs_do_putpages().
> > I'm not sure it's doable to change this to atomic ops.
> > 
> > Last, I suspect ccd.c also has locking issues with v_numoutput and locking.
> > 
> > Would using a software interrupt to run the xbdback handler work to take
> > v_interlock mutex ?
> 
> Yes.

OK, so I think I'll got this route for the xbd backend then.

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