- Add a lock for protecting against msleep(bp, ...) wakeup(bp) races.

 - Create a new function bdone() which sets B_DONE and calls wakup(bp). This
   is suitable for use as b_iodone for buf consumers who are not going
   through the buf cache.
 - Create a new function bwait() which waits for the buf to be done at a set
   priority and with a specific wmesg.
 - Replace several cases where the above functionality was implemented
   without locking with the new functions.

4 files changed, 46 insertions, 36 deletions

diff --git a/sys/fs/specfs/spec_vnops.c b/sys/fs/specfs/spec_vnops.c
index 4f2c145..b53c819 100644
--- a/sys/fs/specfs/spec_vnops.c
+++ b/sys/fs/specfs/spec_vnops.c
@@ -123,8 +123,6 @@ spec_vnoperate(ap)
 	return (VOCALL(spec_vnodeop_p, ap->a_desc->vdesc_offset, ap));
 }
 
-static void spec_getpages_iodone(struct buf *bp);
-
 /*
  * Open a special file.
  */
@@ -689,15 +687,6 @@ spec_advlock(ap)
 	return (ap->a_flags & F_FLOCK ? EOPNOTSUPP : EINVAL);
 }
 
-static void
-spec_getpages_iodone(bp)
-	struct buf *bp;
-{
-
-	bp->b_flags |= B_DONE;
-	wakeup(bp);
-}
-
 static int
 spec_getpages(ap)
 	struct vop_getpages_args *ap;
@@ -755,7 +744,7 @@ spec_getpages(ap)
 
 	/* Build a minimal buffer header. */
 	bp->b_iocmd = BIO_READ;
-	bp->b_iodone = spec_getpages_iodone;
+	bp->b_iodone = bdone;
 
 	/* B_PHYS is not set, but it is nice to fill this in. */
 	KASSERT(bp->b_rcred == NOCRED, ("leaking read ucred"));
@@ -778,11 +767,7 @@ spec_getpages(ap)
 	spec_xstrategy(bp->b_vp, bp);
 
 	s = splbio();
-
-	/* We definitely need to be at splbio here. */
-	while ((bp->b_flags & B_DONE) == 0)
-		tsleep(bp, PVM, "spread", 0);
-
+	bwait(bp, PVM, "spread");
 	splx(s);
 
 	if ((bp->b_ioflags & BIO_ERROR) != 0) {
diff --git a/sys/kern/kern_physio.c b/sys/kern/kern_physio.c
index 01e3750..aebbbdb 100644
--- a/sys/kern/kern_physio.c
+++ b/sys/kern/kern_physio.c
@@ -30,12 +30,6 @@
 #include <vm/vm.h>
 #include <vm/vm_extern.h>
 
-static void
-physwakeup(struct buf *bp)
-{
-	wakeup(bp);
-}
-
 int
 physio(dev_t dev, struct uio *uio, int ioflag)
 {
@@ -68,7 +62,7 @@ physio(dev_t dev, struct uio *uio, int ioflag)
 			else 
 				bp->b_iocmd = BIO_WRITE;
 			bp->b_dev = dev;
-			bp->b_iodone = physwakeup;
+			bp->b_iodone = bdone;
 			bp->b_data = uio->uio_iov[i].iov_base;
 			bp->b_bcount = uio->uio_iov[i].iov_len;
 			bp->b_offset = uio->uio_offset;
@@ -116,8 +110,10 @@ physio(dev_t dev, struct uio *uio, int ioflag)
 
 			DEV_STRATEGY(bp);
 			spl = splbio();
-			while ((bp->b_flags & B_DONE) == 0)
-				tsleep(bp, PRIBIO, "physstr", 0);
+			if (uio->uio_rw == UIO_READ)
+				bwait(bp, PRIBIO, "physrd");
+			else
+				bwait(bp, PRIBIO, "physwr");
 			splx(spl);
 
 			if (uio->uio_segflg == UIO_USERSPACE)
diff --git a/sys/kern/vfs_bio.c b/sys/kern/vfs_bio.c
index 7785e9d..a5c6760 100644
--- a/sys/kern/vfs_bio.c
+++ b/sys/kern/vfs_bio.c
@@ -213,6 +213,12 @@ static int needsbuffer;
 static struct mtx nblock;
 
 /*
+ * Lock that protects against bwait()/bdone()/B_DONE races.
+ */
+
+static struct mtx bdonelock;
+
+/*
  * Definitions for the buffer free lists.
  */
 #define BUFFER_QUEUES	6	/* number of free buffer queues */
@@ -484,6 +490,7 @@ bufinit(void)
 	mtx_init(&rbreqlock, "runningbufspace lock", NULL, MTX_DEF);
 	mtx_init(&nblock, "needsbuffer lock", NULL, MTX_DEF);
 	mtx_init(&bdlock, "buffer daemon lock", NULL, MTX_DEF);
+	mtx_init(&bdonelock, "bdone lock", NULL, MTX_DEF);
 
 	/* next, make a null set of free lists */
 	for (i = 0; i < BUFFER_QUEUES; i++)
@@ -2925,11 +2932,13 @@ allocbuf(struct buf *bp, int size)
 void
 biodone(struct bio *bp)
 {
+	mtx_lock(&bdonelock);
 	bp->bio_flags |= BIO_DONE;
+	if (bp->bio_done == NULL)
+		wakeup(bp);
+	mtx_unlock(&bdonelock);
 	if (bp->bio_done != NULL)
 		bp->bio_done(bp);
-	else
-		wakeup(bp);
 }
 
 /*
@@ -2942,8 +2951,10 @@ int
 biowait(struct bio *bp, const char *wchan)
 {
 
+	mtx_lock(&bdonelock);
 	while ((bp->bio_flags & BIO_DONE) == 0)
-		msleep(bp, NULL, PRIBIO, wchan, hz / 10);
+		msleep(bp, &bdonelock, PRIBIO, wchan, hz / 10);
+	mtx_unlock(&bdonelock);
 	if (bp->bio_error != 0)
 		return (bp->bio_error);
 	if (!(bp->bio_flags & BIO_ERROR))
@@ -3002,12 +3013,10 @@ bufwait(register struct buf * bp)
 	int s;
 
 	s = splbio();
-	while ((bp->b_flags & B_DONE) == 0) {
-		if (bp->b_iocmd == BIO_READ)
-			tsleep(bp, PRIBIO, "biord", 0);
-		else
-			tsleep(bp, PRIBIO, "biowr", 0);
-	}
+	if (bp->b_iocmd == BIO_READ)
+		bwait(bp, PRIBIO, "biord");
+	else
+		bwait(bp, PRIBIO, "biowr");
 	splx(s);
 	if (bp->b_flags & B_EINTR) {
 		bp->b_flags &= ~B_EINTR;
@@ -3214,7 +3223,7 @@ bufdone(struct buf *bp)
 		else
 			bqrelse(bp);
 	} else {
-		wakeup(bp);
+		bdone(bp);
 	}
 	splx(s);
 }
@@ -3697,6 +3706,24 @@ vunmapbuf(struct buf *bp)
 	bp->b_data = bp->b_saveaddr;
 }
 
+void
+bdone(struct buf *bp)
+{
+	mtx_lock(&bdonelock);
+	bp->b_flags |= B_DONE;
+	wakeup(bp);
+	mtx_unlock(&bdonelock);
+}
+
+void
+bwait(struct buf *bp, u_char pri, const char *wchan)
+{
+	mtx_lock(&bdonelock);
+	while ((bp->b_flags & B_DONE) == 0)
+		msleep(bp, &bdonelock, pri, wchan, 0);
+	mtx_unlock(&bdonelock);
+}
+
 #include "opt_ddb.h"
 #ifdef DDB
 #include <ddb/ddb.h>
diff --git a/sys/sys/buf.h b/sys/sys/buf.h
index f8a2416..60d0d42 100644
--- a/sys/sys/buf.h
+++ b/sys/sys/buf.h
@@ -528,6 +528,8 @@ void	pbrelvp(struct buf *);
 int	allocbuf(struct buf *bp, int size);
 void	reassignbuf(struct buf *, struct vnode *);
 struct	buf *trypbuf(int *);
+void	bwait(struct buf *, u_char, const char *);
+void	bdone(struct buf *);
 
 #endif /* _KERNEL */