From owner-freebsd-current@FreeBSD.ORG Wed Jan 14 13:29:09 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 B13AE16A4CE for ; Wed, 14 Jan 2004 13:29:09 -0800 (PST) Received: from gw.catspoiler.org (217-ip-163.nccn.net [209.79.217.163]) by mx1.FreeBSD.org (Postfix) with ESMTP id 3E0BA43D77 for ; Wed, 14 Jan 2004 13:28:40 -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 i0ELSV7E040495; Wed, 14 Jan 2004 13:28:35 -0800 (PST) (envelope-from truckman@FreeBSD.org) Message-Id: <200401142128.i0ELSV7E040495@gw.catspoiler.org> Date: Wed, 14 Jan 2004 13:28:31 -0800 (PST) From: Don Lewis To: shoesoft@gmx.net In-Reply-To: <1074089598.716.5.camel@shoeserv.freebsd> MIME-Version: 1.0 Content-Type: TEXT/plain; charset=us-ascii cc: current@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 21:29:09 -0000 On 14 Jan, Stefan Ehmann wrote: > On Wed, 2004-01-14 at 13:45, Don Lewis wrote: >> On 14 Jan, Stefan Ehmann wrote: >> > No luck - again... >> > >> > panic: mutex pcm0:fake not owned at >> > /usr/src/sys/dev/sound/pcm/channel.c:834 >> > >> > at boottime >> >> I suspect something new got built with INVARIANTS and a working >> CHN_LOCKASSERT() for the first time. Try adding a call to CHN_LOCK() >> after the call to chn_lockinit() in chn_init() and a call to >> CHN_UNLOCK() just after the out: label. These got deleted in rev 1.85, >> though the CHN_UNLOCK() call was in the wrong place in 1.84. > > That seems to fix it - boots fine now. > > Looks like we need some further locks. When I try to set vchans to 4 > with sysctl I get a similar panic. > > panic: mutex pcm0:play:0 not owned at > /usr/src/sys/dev/sound/pcm/channel.c:706 > > #15 0xc04e5b57 in panic () at /usr/src/sys/kern/kern_shutdown.c:550 > #16 0xc04dc4fc in _mtx_assert (m=0xc37ba280, what=0, > file=0xc07ee3e9 "/usr/src/sys/dev/sound/pcm/channel.c", line=706) > at /usr/src/sys/kern/kern_mutex.c:673 > #17 0xc07e436f in chn_reset (c=0x2c2, fmt=268435472) > at /usr/src/sys/dev/sound/pcm/channel.c:706 > #18 0xc07ed1fa in vchan_create (parent=0xc37a8880) > at /usr/src/sys/dev/sound/pcm/vchan.c:273 Sigh ... I had hoped that your earlier testing had meant that locking code was clean. Here's another patch, but I won't guarantee that you won't run into more lock assertion problems. I guess that I should set up some sound hardware here so that I can actually test some of this stuff. 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 12:00:47 -0000 @@ -740,6 +740,7 @@ int ret; chn_lockinit(c, dir); + CHN_LOCK(c); b = NULL; bs = NULL; @@ -789,6 +790,7 @@ out: + CHN_UNLOCK(c); if (ret) { if (c->devinfo) { if (CHANNEL_FREE(c->methods, c->devinfo)) @@ -1009,12 +1011,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 +1029,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/sound.c =================================================================== RCS file: /home/ncvs/src/sys/dev/sound/pcm/sound.c,v retrieving revision 1.86 diff -u -r1.86 sound.c --- sys/dev/sound/pcm/sound.c 8 Dec 2003 01:08:03 -0000 1.86 +++ sys/dev/sound/pcm/sound.c 14 Jan 2004 07:14:05 -0000 @@ -334,7 +334,7 @@ v = snd_maxautovchans; error = sysctl_handle_int(oidp, &v, sizeof(v), req); if (error == 0 && req->newptr != NULL) { - if (v < 0 || v >= SND_MAXVCHANS) + if (v < 0 || v >= SND_MAXVCHANS || pcm_devclass == NULL) return EINVAL; if (v != snd_maxautovchans) { for (i = 0; i < devclass_get_maxunit(pcm_devclass); i++) { 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 14 Jan 2004 21:23:44 -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); @@ -268,12 +270,14 @@ /* XXX gross ugly hack, murder death kill */ if (first && !err) { + CHN_LOCK(parent); err = chn_reset(parent, AFMT_STEREO | AFMT_S16_LE); if (err) printf("chn_reset: %d\n", err); err = chn_setspeed(parent, 44100); if (err) printf("chn_setspeed: %d\n", err); + CHN_UNLOCK(parent); } return err;