Re: sound/pcm/* bugs (was: Re: page fault panic tracked down (selwakeuppri()) - really sound/pcm/*)

On  8 Jan, Stefan Ehmann wrote:
> On Thu, 2004-01-08 at 10:58, Don Lewis wrote:

>> Try the patch below in place of my previous patch.  As you might guess,
>> I'm grasping at straws.
> 
> Again - no luck. This time even disabling vchans doesn't help. No sound
> at all. The code seems to be really tricky.
> 
> I get:
> kernel: pcm0:virtual:0: play interrupt timeout, channel dead
> kernel: pcm0:play:0: play interrupt timeout, channel dead
> each try to change vchans aftet that results in a
> kernel: x: 2 (last message repeated 9 times)
> 
> At least our assumption that the panic only occurs with vchans enabled
> seems to be true (~ 8 hrs uptime).

I stared at the code some more and cranked out another patch.  I think
the problem is in chn_setblocksize().  In the case of the csa driver,
blksz is hardwired to 2048.  If the client of one of the vchans attempts
to set blksz to something smaller than that, the vchan will notify its
parent, which will call chn_setblocksize() with smaller requested value.
chn_setblocksize() will resize its bufsoft to the smaller size, but
bufhard will stay at 2048.  This will trigger the buffer overflow in
feed_vchan_s16().

The following patch changes chn_setblocksize() to resize bufsoft after
bufhard so that their bufsz values match.  It would also be possible to
modify the code to resize bufsoft to the larger of the the bufhard bufsz
or the requested value, but I don't see any advantage to this.  I don't
think that the code will do the right thing if a vchan is configured
with a smaller bufsz than its parent since the vchan won't be able to
fill the parent buffer each time it is polled, but at least this should
get rid of the buffer overflow.

I'm tempted to go ahead and commit the CHN_LOCKASSERT() and KASSERT() ->
panic() changes so that I don't have to carry them around anymore.

Index: sys/dev/sound/pcm/channel.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/sound/pcm/channel.c,v
retrieving revision 1.93
diff -u -r1.93 channel.c
--- sys/dev/sound/pcm/channel.c	5 Dec 2003 02:08:13 -0000	1.93
+++ sys/dev/sound/pcm/channel.c	14 Jan 2004 05:45:20 -0000
_at__at_ -1009,12 +1009,8 _at__at_
 
 	bufsz = blkcnt * blksz;
 
-	ret = sndbuf_remalloc(bs, blkcnt, blksz);
-	if (ret)
-		goto out;
-
 	/* adjust for different hw format/speed */
-	irqhz = (sndbuf_getbps(bs) * sndbuf_getspd(bs)) / sndbuf_getblksz(bs);
+	irqhz = (sndbuf_getbps(bs) * sndbuf_getspd(bs)) / blksz;
 	DEB(printf("%s: soft bps %d, spd %d, irqhz == %d\n", __func__, sndbuf_getbps(bs), sndbuf_getspd(bs), irqhz));
 	RANGE(irqhz, 16, 512);
 
_at__at_ -1031,6 +1027,14 _at__at_
 	DEB(printf("%s: hard blksz requested %d (maxsize %d), ", __func__, blksz, sndbuf_getmaxsize(b)));
 
 	sndbuf_setblksz(b, CHANNEL_SETBLOCKSIZE(c->methods, c->devinfo, blksz));
+
+	/*
+	 * Force bufsoft blksz to match bufhard in case CHANNEL_SETBLOCKSIZE()
+	 * forces something larger than requested.
+	 */
+	ret = sndbuf_remalloc(bs, blkcnt, sndbuf_getblksz(b));
+	if (ret)
+		goto out;
 
 	irqhz = (sndbuf_getbps(b) * sndbuf_getspd(b)) / sndbuf_getblksz(b);
 	DEB(printf("got %d, irqhz == %d\n", sndbuf_getblksz(b), irqhz));
Index: sys/dev/sound/pcm/channel.h
===================================================================
RCS file: /home/ncvs/src/sys/dev/sound/pcm/channel.h,v
retrieving revision 1.28
diff -u -r1.28 channel.h
--- sys/dev/sound/pcm/channel.h	7 Sep 2003 16:28:03 -0000	1.28
+++ sys/dev/sound/pcm/channel.h	7 Jan 2004 20:01:38 -0000
_at__at_ -103,7 +103,7 _at__at_
 #ifdef	USING_MUTEX
 #define CHN_LOCK(c) mtx_lock((struct mtx *)((c)->lock))
 #define CHN_UNLOCK(c) mtx_unlock((struct mtx *)((c)->lock))
-#define CHN_LOCKASSERT(c)
+#define CHN_LOCKASSERT(c) mtx_assert((struct mtx *)((c)->lock), MA_OWNED)
 #else
 #define CHN_LOCK(c)
 #define CHN_UNLOCK(c)
Index: sys/dev/sound/pcm/vchan.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/sound/pcm/vchan.c,v
retrieving revision 1.13
diff -u -r1.13 vchan.c
--- sys/dev/sound/pcm/vchan.c	7 Sep 2003 16:28:03 -0000	1.13
+++ sys/dev/sound/pcm/vchan.c	7 Jan 2004 19:58:57 -0000
_at__at_ -77,7 +77,9 _at__at_
 	int16_t *tmp, *dst;
 	unsigned int cnt;
 
-	KASSERT(sndbuf_getsize(src) >= count, ("bad bufsize"));
+	if (sndbuf_getsize(src) < count)
+		panic("tmp buffer size %d < count %d", sndbuf_getsize(src),
+		    count);
 	count &= ~1;
 	bzero(b, count);