Use a mutex to synchronize the driver top and bottom halves instead of

using critcal_enter() and critical_exit() to attempt to replace spl*()
calls.  The critical section was calling selrecord(), which locks an
MTX_DEF mutex, which is not legal in a critical section.

Tested by: Stefan Ehmann <shoesoft@gmx.net> and "make universe"
Approved by: re (scottl)

4 files changed, 62 insertions, 9 deletions

diff --git a/sys/dev/bktr/bktr_core.c b/sys/dev/bktr/bktr_core.c
index b9d21b8..3297263 100644
--- a/sys/dev/bktr/bktr_core.c
+++ b/sys/dev/bktr/bktr_core.c
@@ -526,6 +526,9 @@ common_bktr_attach( bktr_ptr_t bktr, int unit, u_long pci_id, u_int rev )
 	}
 #endif	/* FreeBSD or BSDi */
 
+#ifdef USE_VBIMUTEX
+	mtx_init(&bktr->vbimutex, "bktr vbi lock", NULL, MTX_DEF);
+#endif
 
 /* If this is a module, save the current contiguous memory */
 #if defined(BKTR_FREEBSD_MODULE)
@@ -807,6 +810,7 @@ common_bktr_intr( void *arg )
 	 * both Odd and Even VBI data is captured. Therefore we do this
 	 * in the Even field interrupt handler.
 	 */
+	LOCK_VBI(bktr);
 	if (  (bktr->vbiflags & VBI_CAPTURE)
 	    &&(bktr->vbiflags & VBI_OPEN)
             &&(field==EVEN_F)) {
@@ -826,6 +830,7 @@ common_bktr_intr( void *arg )
 
 
 	}
+	UNLOCK_VBI(bktr);
 
 	/*
 	 *  Register the completed field
@@ -1066,8 +1071,13 @@ video_open( bktr_ptr_t bktr )
 int
 vbi_open( bktr_ptr_t bktr )
 {
-	if (bktr->vbiflags & VBI_OPEN)		/* device is busy */
+
+	LOCK_VBI(bktr);
+
+	if (bktr->vbiflags & VBI_OPEN) {	/* device is busy */
+		UNLOCK_VBI(bktr);
 		return( EBUSY );
+	}
 
 	bktr->vbiflags |= VBI_OPEN;
 
@@ -1081,6 +1091,8 @@ vbi_open( bktr_ptr_t bktr )
 	bzero((caddr_t) bktr->vbibuffer, VBI_BUFFER_SIZE);
 	bzero((caddr_t) bktr->vbidata,  VBI_DATA_SIZE);
 
+	UNLOCK_VBI(bktr);
+
 	return( 0 );
 }
 
@@ -1166,8 +1178,12 @@ int
 vbi_close( bktr_ptr_t bktr )
 {
 
+	LOCK_VBI(bktr);
+
 	bktr->vbiflags &= ~VBI_OPEN;
 
+	UNLOCK_VBI(bktr);
+
 	return( 0 );
 }
 
@@ -1232,19 +1248,32 @@ video_read(bktr_ptr_t bktr, int unit, dev_t dev, struct uio *uio)
 int
 vbi_read(bktr_ptr_t bktr, struct uio *uio, int ioflag)
 {
-	int             readsize, readsize2;
+	int             readsize, readsize2, start;
 	int             status;
 
+	/*
+	 * XXX - vbi_read() should be protected against being re-entered
+	 * while it is unlocked for the uiomove.
+	 */
+	LOCK_VBI(bktr);
 
 	while(bktr->vbisize == 0) {
 		if (ioflag & IO_NDELAY) {
-			return EWOULDBLOCK;
+			status = EWOULDBLOCK;
+			goto out;
 		}
 
 		bktr->vbi_read_blocked = TRUE;
+#ifdef USE_VBIMUTEX
+		if ((status = msleep(VBI_SLEEP, &bktr->vbimutex, VBIPRI, "vbi",
+		    0))) {
+			goto out;
+		}
+#else
 		if ((status = tsleep(VBI_SLEEP, VBIPRI, "vbi", 0))) {
-			return status;
+			goto out;
 		}
+#endif
 	}
 
 	/* Now we have some data to give to the user */
@@ -1262,13 +1291,19 @@ vbi_read(bktr_ptr_t bktr, struct uio *uio, int ioflag)
 		/* We need to wrap around */
 
 		readsize2 = VBI_BUFFER_SIZE - bktr->vbistart;
-               	status = uiomove((caddr_t)bktr->vbibuffer + bktr->vbistart, readsize2, uio);
-		status += uiomove((caddr_t)bktr->vbibuffer, (readsize - readsize2), uio);
+		start =  bktr->vbistart;
+		UNLOCK_VBI(bktr);
+               	status = uiomove((caddr_t)bktr->vbibuffer + start, readsize2, uio);
+		if (status == 0)
+			status = uiomove((caddr_t)bktr->vbibuffer, (readsize - readsize2), uio);
 	} else {
+		UNLOCK_VBI(bktr);
 		/* We do not need to wrap around */
 		status = uiomove((caddr_t)bktr->vbibuffer + bktr->vbistart, readsize, uio);
 	}
 
+	LOCK_VBI(bktr);
+
 	/* Update the number of bytes left to read */
 	bktr->vbisize -= readsize;
 
@@ -1276,6 +1311,9 @@ vbi_read(bktr_ptr_t bktr, struct uio *uio, int ioflag)
 	bktr->vbistart += readsize;
 	bktr->vbistart = bktr->vbistart % VBI_BUFFER_SIZE; /* wrap around if needed */
 
+out:
+	UNLOCK_VBI(bktr);
+
 	return( status );
 
 }
diff --git a/sys/dev/bktr/bktr_os.c b/sys/dev/bktr/bktr_os.c
index 319e75b..ae32523 100644
--- a/sys/dev/bktr/bktr_os.c
+++ b/sys/dev/bktr/bktr_os.c
@@ -499,6 +499,9 @@ bktr_detach( device_t dev )
 		printf("bktr%d: i2c_attach: can't attach\n",
 		     device_get_unit(dev));
 #endif
+#ifdef USE_VBIMUTEX
+        mtx_destroy(&bktr->vbimutex);
+#endif
 
 	/* Note: We do not free memory for RISC programs, grab buffer, vbi buffers */
 	/* The memory is retained by the bktr_mem module so we can unload and */
@@ -830,6 +833,7 @@ bktr_poll( dev_t dev, int events, struct thread *td)
 		return (ENXIO);
 	}
 
+	LOCK_VBI(bktr);
 	DISABLE_INTR(s);
 
 	if (events & (POLLIN | POLLRDNORM)) {
@@ -845,6 +849,7 @@ bktr_poll( dev_t dev, int events, struct thread *td)
 	}
 
 	ENABLE_INTR(s);
+	UNLOCK_VBI(bktr);
 
 	return (revents);
 }
diff --git a/sys/dev/bktr/bktr_os.h b/sys/dev/bktr/bktr_os.h
index 1740723..d282602 100644
--- a/sys/dev/bktr/bktr_os.h
+++ b/sys/dev/bktr/bktr_os.h
@@ -61,9 +61,10 @@ void            free_bktr_mem(bktr_ptr_t, bus_dmamap_t, vm_offset_t);
 /************************************/
 #if defined(__FreeBSD__)
 #if (__FreeBSD_version >=500000)
+#define USE_VBIMUTEX
 #define	DECLARE_INTR_MASK(s)	/* no need to declare 's' */
-#define DISABLE_INTR(s)		critical_enter()
-#define	ENABLE_INTR(s)		critical_exit()
+#define DISABLE_INTR(s)
+#define ENABLE_INTR(s)
 #else
 #define DECLARE_INTR_MASK(s)	intrmask_t s
 #define DISABLE_INTR(s)		s=spltty()
@@ -75,4 +76,10 @@ void            free_bktr_mem(bktr_ptr_t, bus_dmamap_t, vm_offset_t);
 #define ENABLE_INTR(s)		enable_intr()
 #endif
 
-
+#ifdef USE_VBIMUTEX
+#define LOCK_VBI(bktr)		mtx_lock(&bktr->vbimutex)
+#define UNLOCK_VBI(bktr)	mtx_unlock(&bktr->vbimutex)
+#else
+#define LOCK_VBI(bktr)
+#define UNLOCK_VBI(bktr)
+#endif
diff --git a/sys/dev/bktr/bktr_reg.h b/sys/dev/bktr/bktr_reg.h
index d8ab6c6..a7c3ef5 100644
--- a/sys/dev/bktr/bktr_reg.h
+++ b/sys/dev/bktr/bktr_reg.h
@@ -542,6 +542,9 @@ struct bktr_softc {
     dev_t           tunerdev_alias;	/* alias /dev/tuner to /dev/tuner0 */
     dev_t           vbidev_alias;	/* alias /dev/vbi to /dev/vbi0 */
     #endif
+    #if (__FreeBSD_version >= 500000)
+    struct mtx      vbimutex;  /* Mutex protecting vbi buffer */
+    #endif
     #if (__FreeBSD_version >= 310000)
     bus_space_tag_t	memt;	/* Bus space register access functions */
     bus_space_handle_t	memh;	/* Bus space register access functions */