From owner-freebsd-current@FreeBSD.ORG Tue Jan 13 22:16:43 2004 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 5477116A4CE; Tue, 13 Jan 2004 22:16:43 -0800 (PST) Received: from gw.catspoiler.org (217-ip-163.nccn.net [209.79.217.163]) by mx1.FreeBSD.org (Postfix) with ESMTP id AD87143D5F; Tue, 13 Jan 2004 22:16:39 -0800 (PST) (envelope-from truckman@FreeBSD.org) Received: from FreeBSD.org (mousie.catspoiler.org [192.168.101.2]) by gw.catspoiler.org (8.12.9p2/8.12.9) with ESMTP id i0E6G17E038163; Tue, 13 Jan 2004 22:16:05 -0800 (PST) (envelope-from truckman@FreeBSD.org) Message-Id: <200401140616.i0E6G17E038163@gw.catspoiler.org> Date: Tue, 13 Jan 2004 22:16:01 -0800 (PST) From: Don Lewis To: shoesoft@gmx.net In-Reply-To: <1073579992.722.6.camel@shoeserv.freebsd> MIME-Version: 1.0 Content-Type: TEXT/plain; charset=us-ascii cc: cg@FreeBSD.org cc: current@FreeBSD.org cc: mat@cnd.mcgill.ca cc: orion@FreeBSD.org Subject: Re: sound/pcm/* bugs (was: Re: page fault panic tracked down (selwakeuppri()) - really sound/pcm/*) X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 14 Jan 2004 06:16:43 -0000 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 @@ -1009,12 +1009,8 @@ 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); @@ -1031,6 +1027,14 @@ 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 @@ -103,7 +103,7 @@ #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 @@ -77,7 +77,9 @@ 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);