Re: biodone() and splbio ?

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

On Tue, Jan 19, 2010 at 10:52:49PM +0100, Manuel Bouyer wrote:
> On Tue, Jan 19, 2010 at 10:43:04PM +0100, Manuel Bouyer wrote:
> > [...]
> > If not, dk.c needs to be fixed to raise to splbio() in its done routine.
> > raidcframe already does it. I suspect ccdiodone() and vndiodone()
> > need it too.
> 
> Maybe not; it seems ccd is MP-safe.
> For vnd, the lack of appropriate spl call here can corrupt sc_active count,
> but it seems it can't corrupt the buffer queue.


Here is my proposed patch for this issue. dk(4) tested, and this most probably
fixed the issue (uptime > 1d, would crash within a few hours before),
cgd(4) and vnd(4) compile-tested only but I don't expect problems here.

Does anyone see a problem with this ?

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

Index: cgd.c
===================================================================
RCS file: /cvsroot/src/sys/dev/cgd.c,v
retrieving revision 1.53.4.2
diff -u -r1.53.4.2 cgd.c
--- cgd.c       4 Apr 2009 17:18:53 -0000       1.53.4.2
+++ cgd.c       21 Jan 2010 16:45:39 -0000
@@ -344,13 +344,13 @@
        return 0;
 }
 
-/* expected to be called at splbio() */
 static void
 cgdiodone(struct buf *nbp)
 {
        struct  buf *obp = nbp->b_private;
        struct  cgd_softc *cs = getcgd_softc(obp->b_dev);
        struct  dk_softc *dksc = &cs->sc_dksc;
+       int s;
 
        KDASSERT(cs);
 
@@ -385,10 +385,12 @@
        obp->b_resid = 0;
        if (obp->b_error != 0)
                obp->b_resid = obp->b_bcount;
+       s = splbio();
        disk_unbusy(&dksc->sc_dkdev, obp->b_bcount - obp->b_resid,
            (obp->b_flags & B_READ));
        biodone(obp);
        dk_iodone(di, dksc);
+       splx(s);
 }
 
 /* XXX: we should probably put these into dksubr.c, mostly */
Index: vnd.c
===================================================================
RCS file: /cvsroot/src/sys/dev/vnd.c,v
retrieving revision 1.187.4.3
diff -u -r1.187.4.3 vnd.c
--- vnd.c       4 Apr 2009 17:20:00 -0000       1.187.4.3
+++ vnd.c       21 Jan 2010 16:45:39 -0000
@@ -862,6 +862,7 @@
        struct vndxfer *vnx = VND_BUFTOXFER(bp);
        struct vnd_softc *vnd = vnx->vx_vnd;
        struct buf *obp = bp->b_private;
+       int s = splbio();
 
        KASSERT(&vnx->vx_buf == bp);
        KASSERT(vnd->sc_active > 0);
@@ -877,6 +878,7 @@
        if (vnd->sc_active == 0) {
                wakeup(&vnd->sc_tab);
        }
+       splx(s);
        obp->b_error = bp->b_error;
        obp->b_resid = bp->b_resid;
        buf_destroy(bp);
Index: dkwedge/dk.c
===================================================================
RCS file: /cvsroot/src/sys/dev/dkwedge/dk.c,v
retrieving revision 1.42
diff -u -r1.42 dk.c
--- dkwedge/dk.c        17 Jun 2008 14:53:10 -0000      1.42
+++ dkwedge/dk.c        21 Jan 2010 16:45:39 -0000
@@ -1100,7 +1100,6 @@
  * dkiodone:
  *
  *     I/O to a wedge has completed; alert the top half.
- *     NOTE: Must be called at splbio()!
  */
 static void
 dkiodone(struct buf *bp)
@@ -1108,6 +1107,8 @@
        struct buf *obp = bp->b_private;
        struct dkwedge_softc *sc = dkwedge_lookup(obp->b_dev);
 
+       int s = splbio();
+
        if (bp->b_error != 0)
                obp->b_error = bp->b_error;
        obp->b_resid = bp->b_resid;
@@ -1125,6 +1126,7 @@
 
        /* Kick the queue in case there is more work we can do. */
        dkstart(sc);
+       splx(s);
 }
 
 /*