On 27 Nov, Stefan Ehmann wrote:
> On Wed, 2003-11-26 at 08:33, Don Lewis wrote:
>> The problem is that selrecord() wants to lock a MTX_DEF mutex, which can
>> cause a context switch if the mutex is already locked by another thread.
>> This is contrary to what bktr_poll() wants to accomplish by calling
>> critical_enter().
> 
> Strange enough that does not seem to happen with a kernel built without
> INVARIANTS and WITNESS. Does this make any sense or is this just by
> chance?

You might try the patch below with WITNESS enabled.  I don't have the
hardware, so I can't test it.  It compiles for me, but for all I know it
could delete all your files if you run it.

Index: sys/dev/bktr/bktr_core.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/bktr/bktr_core.c,v
retrieving revision 1.131
diff -u -r1.131 bktr_core.c
--- sys/dev/bktr/bktr_core.c    9 Nov 2003 09:17:21 -0000       1.131
+++ sys/dev/bktr/bktr_core.c    27 Nov 2003 23:58:19 -0000
@@ -526,6 +526,9 @@
        }
 #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 @@
         * 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 @@
 
 
        }
+       UNLOCK_VBI(bktr);
 
        /*
         *  Register the completed field
@@ -1066,8 +1071,13 @@
 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 @@
        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 @@
 vbi_close( bktr_ptr_t bktr )
 {
 
+       LOCK_VBI(bktr);
+
        bktr->vbiflags &= ~VBI_OPEN;
 
+       UNLOCK_VBI(bktr);
+
        return( 0 );
 }
 
@@ -1232,19 +1248,32 @@
 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,19 +1291,28 @@
                /* 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;
 
        /* Update vbistart */
        bktr->vbistart += readsize;
        bktr->vbistart = bktr->vbistart % VBI_BUFFER_SIZE; /* wrap around if needed */
+
+out:
+       UNLOCK_VBI(bktr);
 
        return( status );
 
Index: sys/dev/bktr/bktr_os.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/bktr/bktr_os.c,v
retrieving revision 1.39
diff -u -r1.39 bktr_os.c
--- sys/dev/bktr/bktr_os.c      2 Sep 2003 17:30:34 -0000       1.39
+++ sys/dev/bktr/bktr_os.c      27 Nov 2003 23:49:39 -0000
@@ -499,6 +499,9 @@
                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 @@
                return (ENXIO);
        }
 
+       LOCK_VBI(bktr);
        DISABLE_INTR(s);
 
        if (events & (POLLIN | POLLRDNORM)) {
@@ -845,6 +849,7 @@
        }
 
        ENABLE_INTR(s);
+       UNLOCK_VBI(bktr);
 
        return (revents);
 }
Index: sys/dev/bktr/bktr_os.h
===================================================================
RCS file: /home/ncvs/src/sys/dev/bktr/bktr_os.h,v
retrieving revision 1.6
diff -u -r1.6 bktr_os.h
--- sys/dev/bktr/bktr_os.h      18 Dec 2001 00:27:15 -0000      1.6
+++ sys/dev/bktr/bktr_os.h      27 Nov 2003 23:38:51 -0000
@@ -61,9 +61,10 @@
 /************************************/
 #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 @@
 #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
Index: sys/dev/bktr/bktr_reg.h
===================================================================
RCS file: /home/ncvs/src/sys/dev/bktr/bktr_reg.h,v
retrieving revision 1.45
diff -u -r1.45 bktr_reg.h
--- sys/dev/bktr/bktr_reg.h     12 Aug 2003 09:45:34 -0000      1.45
+++ sys/dev/bktr/bktr_reg.h     27 Nov 2003 23:31:53 -0000
@@ -542,6 +542,9 @@
     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 */

_______________________________________________
[EMAIL PROTECTED] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "[EMAIL PROTECTED]"

Reply via email to