pcm(4) related panic

[Mathew Kanner]

On Nov 25, Don Lewis wrote:
> On 25 Nov, Don Lewis wrote:
> > On 25 Nov, Artur Poplawski wrote:
> >> Artur Poplawski <artur at szuruburu.neostrada.pl> wrote:
> >> 
> >>> Hello,                                                                          
> >>>                                                                                 
> >>> On a 5.1-RELEASE and 5.2-BETA machines I have been able to cause a panic 
> >>> like this:
> > 
> >>> Sleeping on "swread" with the following non-sleepable locks held:
> >>> exclusive sleep mutex pcm0:play:0 (pcm channel) r = 0 (0xc1c3d740) locked @ \   
> >>>         /usr/src/sys/dev/sound/pcm/dsp.c:146
> > 
> > This enables the panic.
> > 
> >>> panic: sleeping thread (pid 583) owns a non-sleepable lock
> > 
> > Then the panic happens when another thread tries to grab the mutex.
> > 
> > 
> > The problem is that the pcm code attempts to hold a mutex across a call
> > to uiomove(), which can sleep if the userland buffer that it is trying
> > to access is paged out.  Either the buffer has to be pre-wired before
> > calling getchns(), or the mutex has to be dropped around the call to
> > uiomove().  The amount of memory to be wired should be limited to
> > 'sz' as calculated by chn_read() and chn_write(), which complicates the
> > logic.  Dropping the mutex probably has other issues.
> 
> Following up to myself ...
> 
> It might be safe to drop the mutex for the uiomove() call if the code
> set flags to enforce a limit of one reader and one writer at a time to
> keep the code from being re-entered.  The buffer pointer manipulations
> in sndbuf_dispose() and sndbuf_acquire() would probably still have to be
> protected by the mutex.  If this can be made to work, it would probably
> be preferable to wiring the buffer.  It would have a lot less CPU
> overhead, and would work better with large buffers, which could still be
> allowed to page normally.

	Don,
	I never would have suspected that uio might sleep and panic,
thanks for the clue.

	Artur,
	Could you try the attached patch.

	Thanks,
	--Mat

-- 
	Any idiot can face a crisis; it is this day-to-day living
	that wears you out.
			- Chekhov
-------------- next part --------------
--- channel.c	Sun Nov  9 04:17:22 2003
+++ /sys/dev/sound/pcm/channel.c	Wed Nov 26 02:21:14 2003
@@ -250,6 +250,8 @@
 {
 	int ret, timeout, newsize, count, sz;
 	struct snd_dbuf *bs = c->bufsoft;
+	void *off;
+	int t, x,togo,p;
 
 	CHN_LOCKASSERT(c);
 	/*
@@ -291,7 +293,22 @@
 			sz = MIN(sz, buf->uio_resid);
 			KASSERT(sz > 0, ("confusion in chn_write"));
 			/* printf("sz: %d\n", sz); */
+#if 0
 			ret = sndbuf_uiomove(bs, buf, sz);
+#else
+			togo = sz;
+			while (ret == 0 && togo> 0) {
+				p = sndbuf_getfreeptr(bs);
+				t = MIN(togo, sndbuf_getsize(bs) - p);
+				off = sndbuf_getbufofs(bs, p);
+				CHN_UNLOCK(c);
+				ret = uiomove(off, t, buf);
+				CHN_LOCK(c);
+				togo -= t;
+				x = sndbuf_acquire(bs, NULL, t);
+			}
+			ret = 0;
+#endif
 			if (ret == 0 && !(c->flags & CHN_F_TRIGGERED))
 				chn_start(c, 0);
 		}