panic on 5.2 BETA: blockable sleep lock
Don Lewis
truckman at FreeBSD.org
Thu Nov 27 16:02:19 PST 2003
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 */
More information about the freebsd-current
mailing list