From owner-freebsd-current@FreeBSD.ORG Wed Sep 8 07:45:59 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 5C78116A4CE for ; Wed, 8 Sep 2004 07:45:59 +0000 (GMT) Received: from gw.catspoiler.org (217-ip-163.nccn.net [209.79.217.163]) by mx1.FreeBSD.org (Postfix) with ESMTP id DCDC143D46 for ; Wed, 8 Sep 2004 07:45:58 +0000 (GMT) (envelope-from truckman@FreeBSD.org) Received: from FreeBSD.org (mousie.catspoiler.org [192.168.101.2]) by gw.catspoiler.org (8.12.11/8.12.11) with ESMTP id i887joDI040563; Wed, 8 Sep 2004 00:45:55 -0700 (PDT) (envelope-from truckman@FreeBSD.org) Message-Id: <200409080745.i887joDI040563@gw.catspoiler.org> Date: Wed, 8 Sep 2004 00:45:50 -0700 (PDT) From: Don Lewis To: itetcu@people.tecnik93.com In-Reply-To: <20040906103349.57b1d7cd@it.buh.tecnik93.com> MIME-Version: 1.0 Content-Type: TEXT/plain; charset=us-ascii cc: freebsd-current@FreeBSD.org Subject: Re: LOR sndstat.c:165 vm_map.c:2997 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, 08 Sep 2004 07:45:59 -0000 On 6 Sep, Ion-Mihai Tetcu wrote: > # uname -a > FreeBSD it.buh.tecnik93.com 5.3-BETA3 FreeBSD 5.3-BETA3 #1: Sat Sep 4 07:14:07 EEST 2004 itetcu@it.buh.tecnik93.com:/usr/obj/usr/src/sys/IT53_b2_df i386 > > with "normal" debugging stuff minus malloc > > # cat /dev/sndstat > FreeBSD Audio Driver (newpcm) > Installed devices: > pcm0: at io 0xe400 irq 22 kld snd_via8233 (5p/1r/0v channels duplex default) > > It happen when I've first cat /dev/sndstat, it doesn't happen after. > > Sep 6 10:25:28 it kernel: 1st 0xc07f9800 sndstat (sndstat) @ /usr/src/sys/modules/sound/sound/../../../dev/sound/pcm/sndstat.c:165 > Sep 6 10:25:28 it kernel: 2nd 0xc2e6174c user map (user map) @ /usr/src/sys/vm/vm_map.c:2997 > Sep 6 10:25:28 it kernel: KDB: stack backtrace: > Sep 6 10:25:28 it kernel: kdb_backtrace(c0687b96,c2e6174c,c069778e,c069778e,c069780e) at kdb_backtrace+0x2e > Sep 6 10:25:28 it kernel: witness_checkorder(c2e6174c,9,c069780e,bb5,ef65c000) at witness_checkorder+0x6b2 > Sep 6 10:25:28 it kernel: _sx_xlock(c2e6174c,c069780e,bb5,c2279a00,ef26a978) at _sx_xlock+0x7e > Sep 6 10:25:28 it kernel: _vm_map_lock_read(c2e61708,c069780e,bb5,26446e1,804c000) at _vm_map_lock_read+0x4a > Sep 6 10:25:28 it kernel: vm_map_lookup(ef26aa20,804c000,2,ef26aa24,ef26aa14) at vm_map_lookup+0x2e > Sep 6 10:25:28 it kernel: vm_fault(c2e61708,804c000,2,8,c2e8fb00) at vm_fault+0x7e > Sep 6 10:25:28 it kernel: trap_pfault(ef26aaec,0,804c000,c06df600,804c000) at trap_pfault+0xf7 > Sep 6 10:25:28 it kernel: trap(18,10,10,804c000,c4b56000) at trap+0x34d > Sep 6 10:25:28 it kernel: calltrap() at calltrap+0x5 > Sep 6 10:25:28 it kernel: --- trap 0xc, eip = 0xc06509aa, esp = 0xef26ab2c, ebp = 0xef26ab60 --- > Sep 6 10:25:28 it kernel: generic_copyout(c4b56000,8b,ef26ac80,a5,c2e0f660) at generic_copyout+0x36 > Sep 6 10:25:28 it kernel: sndstat_read(c06d6c68,ef26ac80,20000,ef26abc4,c07f82a0) at sndstat_read+0xb5 > Sep 6 10:25:28 it kernel: spec_read(ef26ac0c,ef26ac58,c053c228,ef26ac0c,1020002) at spec_read+0x1e3 > Sep 6 10:25:28 it kernel: spec_vnoperate(ef26ac0c,1020002,c2e8fb00,219,ef26ac80) at spec_vnoperate+0x18 > Sep 6 10:25:28 it kernel: vn_read(c2e0f660,ef26ac80,c4e51480,0,c2e8fb00) at vn_read+0x198 > Sep 6 10:25:28 it kernel: dofileread(c2e8fb00,c2e0f660,3,804c000,1000) at dofileread+0xa4 > Sep 6 10:25:28 it kernel: read(c2e8fb00,ef26ad14,c,431,3) at read+0x6b > Sep 6 10:25:28 it kernel: syscall(2f,2f,2f,1,bfbfec05) at syscall+0x272 > Sep 6 10:25:28 it kernel: Xint0x80_syscall() at Xint0x80_syscall+0x1f > Sep 6 10:25:28 it kernel: --- syscall (3, FreeBSD ELF32, read), eip = 0x280d5fbf, esp = 0xbfbfe9dc, ebp = 0xbfbfea68 --- sndstat_read() is holding a mutex across a call to uiomove(), which is generally not allowed because accessing the user's buffer can block. The mutex could be dropped around the call to uiomove(), but it would require adding another mechanism to keep sndstat_read() from being re-entered. I took the lazy route and converted sndstat_lock from a mutex to an sx lock. Index: sys/dev/sound/pcm/sndstat.c =================================================================== RCS file: /home/ncvs/src/sys/dev/sound/pcm/sndstat.c,v retrieving revision 1.17 diff -u -r1.17 sndstat.c --- sys/dev/sound/pcm/sndstat.c 16 Jun 2004 09:46:57 -0000 1.17 +++ sys/dev/sound/pcm/sndstat.c 8 Sep 2004 07:36:29 -0000 @@ -26,6 +26,9 @@ #include #include +#ifdef USING_MUTEX +#include +#endif SND_DECLARE_FILE("$FreeBSD: src/sys/dev/sound/pcm/sndstat.c,v 1.17 2004/06/16 09:46:57 phk Exp $"); @@ -59,7 +62,7 @@ }; #ifdef USING_MUTEX -static struct mtx sndstat_lock; +static struct sx sndstat_lock; #endif static struct sbuf sndstat_sbuf; static struct cdev *sndstat_dev = 0; @@ -89,12 +92,12 @@ error = sysctl_handle_int(oidp, &verbose, sizeof(verbose), req); if (error == 0 && req->newptr != NULL) { s = spltty(); - mtx_lock(&sndstat_lock); + sx_xlock(&sndstat_lock); if (verbose < 0 || verbose > 3) error = EINVAL; else sndstat_verbose = verbose; - mtx_unlock(&sndstat_lock); + sx_xunlock(&sndstat_lock); splx(s); } return error; @@ -109,14 +112,14 @@ int error; s = spltty(); - mtx_lock(&sndstat_lock); + sx_xlock(&sndstat_lock); if (sndstat_isopen) { - mtx_unlock(&sndstat_lock); + sx_xunlock(&sndstat_lock); splx(s); return EBUSY; } sndstat_isopen = 1; - mtx_unlock(&sndstat_lock); + sx_xunlock(&sndstat_lock); splx(s); if (sbuf_new(&sndstat_sbuf, NULL, 4096, 0) == NULL) { error = ENXIO; @@ -127,9 +130,9 @@ out: if (error) { s = spltty(); - mtx_lock(&sndstat_lock); + sx_xlock(&sndstat_lock); sndstat_isopen = 0; - mtx_unlock(&sndstat_lock); + sx_xunlock(&sndstat_lock); splx(s); } return (error); @@ -141,16 +144,16 @@ intrmask_t s; s = spltty(); - mtx_lock(&sndstat_lock); + sx_xlock(&sndstat_lock); if (!sndstat_isopen) { - mtx_unlock(&sndstat_lock); + sx_xunlock(&sndstat_lock); splx(s); return EBADF; } sbuf_delete(&sndstat_sbuf); sndstat_isopen = 0; - mtx_unlock(&sndstat_lock); + sx_xunlock(&sndstat_lock); splx(s); return 0; } @@ -162,9 +165,9 @@ int l, err; s = spltty(); - mtx_lock(&sndstat_lock); + sx_xlock(&sndstat_lock); if (!sndstat_isopen) { - mtx_unlock(&sndstat_lock); + sx_xunlock(&sndstat_lock); splx(s); return EBADF; } @@ -172,7 +175,7 @@ err = (l > 0)? uiomove(sbuf_data(&sndstat_sbuf) + sndstat_bufptr, l, buf) : 0; sndstat_bufptr += l; - mtx_unlock(&sndstat_lock); + sx_xunlock(&sndstat_lock); splx(s); return err; } @@ -227,12 +230,12 @@ ent->handler = handler; s = spltty(); - mtx_lock(&sndstat_lock); + sx_xlock(&sndstat_lock); SLIST_INSERT_HEAD(&sndstat_devlist, ent, link); if (type == SS_TYPE_MODULE) sndstat_files++; sndstat_maxunit = (unit > sndstat_maxunit)? unit : sndstat_maxunit; - mtx_unlock(&sndstat_lock); + sx_xunlock(&sndstat_lock); splx(s); return 0; @@ -251,18 +254,18 @@ struct sndstat_entry *ent; s = spltty(); - mtx_lock(&sndstat_lock); + sx_xlock(&sndstat_lock); SLIST_FOREACH(ent, &sndstat_devlist, link) { if (ent->dev == dev) { SLIST_REMOVE(&sndstat_devlist, ent, sndstat_entry, link); - mtx_unlock(&sndstat_lock); + sx_xunlock(&sndstat_lock); splx(s); free(ent, M_DEVBUF); return 0; } } - mtx_unlock(&sndstat_lock); + sx_xunlock(&sndstat_lock); splx(s); return ENXIO; @@ -275,19 +278,19 @@ struct sndstat_entry *ent; s = spltty(); - mtx_lock(&sndstat_lock); + sx_xlock(&sndstat_lock); SLIST_FOREACH(ent, &sndstat_devlist, link) { if (ent->dev == NULL && ent->str == str) { SLIST_REMOVE(&sndstat_devlist, ent, sndstat_entry, link); sndstat_files--; - mtx_unlock(&sndstat_lock); + sx_xunlock(&sndstat_lock); splx(s); free(ent, M_DEVBUF); return 0; } } - mtx_unlock(&sndstat_lock); + sx_xunlock(&sndstat_lock); splx(s); return ENXIO; @@ -342,7 +345,7 @@ static int sndstat_init(void) { - mtx_init(&sndstat_lock, "sndstat", NULL, MTX_DEF); + sx_init(&sndstat_lock, "sndstat"); sndstat_dev = make_dev(&sndstat_cdevsw, SND_DEV_STATUS, UID_ROOT, GID_WHEEL, 0444, "sndstat"); return (sndstat_dev != 0)? 0 : ENXIO; @@ -354,9 +357,9 @@ intrmask_t s; s = spltty(); - mtx_lock(&sndstat_lock); + sx_xlock(&sndstat_lock); if (sndstat_isopen) { - mtx_unlock(&sndstat_lock); + sx_xunlock(&sndstat_lock); splx(s); return EBUSY; } @@ -366,7 +369,7 @@ sndstat_dev = 0; splx(s); - mtx_destroy(&sndstat_lock); + sx_destroy(&sndstat_lock); return 0; }