Re: vwakeup: neg numoutput

[Greg Oster <oster%cs.usask.ca@localhost>]

Manuel Bouyer writes:
> On Sun, Jun 29, 2008 at 02:03:18PM -0600, Greg Oster wrote:
> > > Its done it to me 4 or 5 times now.  Trying to scp or rdist about 7GB 
> > > of /usr/pkg and /usr/local to the domU, it gets about 1 or 2 GB 
> > > transferred then the dom0 panics :-(
> > 
> > In both xbdback_xenbus.c and xbdback.c we have this code:
> > 
> >     if ((xbd_io->xio_buf.b_flags & B_READ) == 0)
> >             xbd_io->xio_buf.b_vp->v_numoutput++;
> > 
> > Should this not read:
> > 
> >     if ((xbd_io->xio_buf.b_flags & B_READ) == 0) {
> >             mutex_enter(&xbd_io->xio_buf.b_vp->v_interlock);
> >             xbd_io->xio_buf.b_vp->v_numoutput++;
> >             mutex_exit(&xbd_io->xio_buf.b_vp->v_interlock);
> >         }
> > 
> > (or somesuch) to properly update v_numoutput ?  Most other changes
> > to v_numoutput are holding the vp->v_interlock, but if this one is, 
> > I'm completely missing how it's doing it...
> 
> I think you're right, the interlock is not taken here.
> But we have a problem: at this point we are in interrupt context.
> Can we take this mutex from interrupt context ?

Hmmm... you might be able to get away with it, but I don't know for 
sure... but this also means the code is calling bdev_strategy() 
from interrupt context as well...  When it was discovered that RAIDframe 
was doing things like that, I ended up creating a raidio thread to 
handle the IO in a non-interrupt context.  (in that code, 
simplelocks are used to protect the queue...)  Maybe something 
similar will be needed here so there's not "too much stuff" happening 
in interrupt context?

Later...

Greg Oster