From owner-freebsd-current@FreeBSD.ORG Mon Jan 26 15:05:58 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 B735116A4CE; Mon, 26 Jan 2004 15:05:58 -0800 (PST) Received: from gw.catspoiler.org (217-ip-163.nccn.net [209.79.217.163]) by mx1.FreeBSD.org (Postfix) with ESMTP id 64A5143D45; Mon, 26 Jan 2004 15:05:53 -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 i0QN4t7E078963; Mon, 26 Jan 2004 15:05:00 -0800 (PST) (envelope-from truckman@FreeBSD.org) Message-Id: <200401262305.i0QN4t7E078963@gw.catspoiler.org> Date: Mon, 26 Jan 2004 15:04:55 -0800 (PST) From: Don Lewis To: gbergling@0xfce3.net In-Reply-To: <200401262107.i0QL7j7E078697@gw.catspoiler.org> MIME-Version: 1.0 Content-Type: TEXT/plain; charset=us-ascii cc: cg@FreeBSD.org cc: freebsd-current@FreeBSD.org cc: mat@cnd.mcgill.ca Subject: Re: dev/sound/pcm/* patch testers wanted 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: Mon, 26 Jan 2004 23:05:58 -0000 On 26 Jan, To: gbergling@0xfce3.net wrote: > On 26 Jan, Gordon Bergling wrote: >> On Mon Jan 26, 2004 at 09:25AM -0800, Don Lewis wrote: >>> On 26 Jan, Gordon Bergling wrote: >>> >>> > The "Danger!" message wasn't appeared, but I got the following on the >>> > console. >>> > >>> > malloc() of "1024" with the following non-sleepable locks held: >>> > exclusive sleep mutex pcm0:play:0 (pcm play channel) r = 0 (0xc2be8a80) >>> > locked @ >>> > /storage/os/src/freebsd-current/src/sys/dev/sound/pcm/buffer.c:195 >>> > malloc() of "4096" with the following non-sleepable locks held: >>> > exclusive sleep mutex pcm0:play:0 (pcm play channel) r = 0 (0xc2be8a80) >>> > locked @ >>> > /storage/os/src/freebsd-current/src/sys/dev/sound/pcm/buffer.c:195 > > Quick and dirty solutions: > Switch back to M_NOWAIT even though we don't handle malloc() > failure gracefully. > > Unlock the channel before calling the setblocksize method, but > an interrupt happening in the middle of sndbuf_resize() would > not be a good thing. > > Add a pointer back to the channel back to the snd_dbuf structure > so that sndbuf_resize() could do the proper unlocking and > locking. I picked door #3, which had some complications because sndbuf_resize() was called in some places with the channel locked and some places with the channel unlocked ... Index: sys/dev/sound/pcm/buffer.c =================================================================== RCS file: /home/ncvs/src/sys/dev/sound/pcm/buffer.c,v retrieving revision 1.21 diff -u -r1.21 buffer.c --- sys/dev/sound/pcm/buffer.c 27 Nov 2003 19:51:44 -0000 1.21 +++ sys/dev/sound/pcm/buffer.c 26 Jan 2004 21:47:00 -0000 @@ -31,13 +31,14 @@ SND_DECLARE_FILE("$FreeBSD: src/sys/dev/sound/pcm/buffer.c,v 1.21 2003/11/27 19:51:44 matk Exp $"); struct snd_dbuf * -sndbuf_create(device_t dev, char *drv, char *desc) +sndbuf_create(device_t dev, char *drv, char *desc, struct pcm_channel *channel) { struct snd_dbuf *b; b = malloc(sizeof(*b), M_DEVBUF, M_WAITOK | M_ZERO); snprintf(b->name, SNDBUF_NAMELEN, "%s:%s", drv, desc); b->dev = dev; + b->channel = channel; return b; } @@ -113,27 +114,38 @@ int sndbuf_resize(struct snd_dbuf *b, unsigned int blkcnt, unsigned int blksz) { - u_int8_t *tmpbuf; + u_int8_t *tmpbuf, *f2; + + chn_lock(b->channel); if (b->maxsize == 0) - return 0; + goto out; if (blkcnt == 0) blkcnt = b->blkcnt; if (blksz == 0) blksz = b->blksz; - if (blkcnt < 2 || blksz < 16 || (blkcnt * blksz > b->maxsize)) + if (blkcnt < 2 || blksz < 16 || (blkcnt * blksz > b->maxsize)) { + chn_unlock(b->channel); return EINVAL; + } if (blkcnt == b->blkcnt && blksz == b->blksz) - return 0; - b->blkcnt = blkcnt; - b->blksz = blksz; - b->bufsize = blkcnt * blksz; + goto out; - tmpbuf = malloc(b->bufsize, M_DEVBUF, M_NOWAIT); + chn_unlock(b->channel); + tmpbuf = malloc(b->bufsize, M_DEVBUF, M_WAITOK); if (tmpbuf == NULL) return ENOMEM; - free(b->tmpbuf, M_DEVBUF); + chn_lock(b->channel); + b->blkcnt = blkcnt; + b->blksz = blksz; + b->bufsize = blkcnt * blksz; + f2 = b->tmpbuf; b->tmpbuf = tmpbuf; sndbuf_reset(b); + chn_unlock(b->channel); + free(f2, M_DEVBUF); + return 0; +out: + chn_unlock(b->channel); return 0; } @@ -142,21 +154,27 @@ { u_int8_t *buf, *tmpbuf, *f1, *f2; unsigned int bufsize; + int ret; if (blkcnt < 2 || blksz < 16) return EINVAL; bufsize = blksz * blkcnt; - buf = malloc(bufsize, M_DEVBUF, M_NOWAIT); - if (buf == NULL) - return ENOMEM; + chn_unlock(b->channel); + buf = malloc(bufsize, M_DEVBUF, M_WAITOK); + if (buf == NULL) { + ret = ENOMEM; + goto out; + } - tmpbuf = malloc(bufsize, M_DEVBUF, M_NOWAIT); + tmpbuf = malloc(bufsize, M_DEVBUF, M_WAITOK); if (tmpbuf == NULL) { free(buf, M_DEVBUF); - return ENOMEM; + ret = ENOMEM; + goto out; } + chn_lock(b->channel); b->blkcnt = blkcnt; b->blksz = blksz; @@ -167,13 +185,18 @@ b->buf = buf; b->tmpbuf = tmpbuf; + sndbuf_reset(b); + + chn_unlock(b->channel); if (f1) free(f1, M_DEVBUF); if (f2) free(f2, M_DEVBUF); - sndbuf_reset(b); - return 0; + ret = 0; +out: + chn_lock(b->channel); + return ret; } void Index: sys/dev/sound/pcm/buffer.h =================================================================== RCS file: /home/ncvs/src/sys/dev/sound/pcm/buffer.h,v retrieving revision 1.8 diff -u -r1.8 buffer.h --- sys/dev/sound/pcm/buffer.h 27 Nov 2003 19:51:44 -0000 1.8 +++ sys/dev/sound/pcm/buffer.h 26 Jan 2004 21:28:03 -0000 @@ -53,10 +53,11 @@ bus_dma_tag_t dmatag; u_int32_t buf_addr; struct selinfo sel; + struct pcm_channel *channel; char name[SNDBUF_NAMELEN]; }; -struct snd_dbuf *sndbuf_create(device_t dev, char *drv, char *desc); +struct snd_dbuf *sndbuf_create(device_t dev, char *drv, char *desc, struct pcm_channel *channel); void sndbuf_destroy(struct snd_dbuf *b); void sndbuf_dump(struct snd_dbuf *b, char *s, u_int32_t what); 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 26 Jan 2004 21:53:02 -0000 @@ -70,9 +70,9 @@ chn_lockinit(struct pcm_channel *c, int dir) { if (dir == PCMDIR_PLAY) - c->lock = snd_mtxcreate(c->name, "pcm play channel"); + c->lock = snd_chnmtxcreate(c->name, "pcm play channel"); else - c->lock = snd_mtxcreate(c->name, "pcm record channel"); + c->lock = snd_chnmtxcreate(c->name, "pcm record channel"); } static void @@ -205,16 +205,19 @@ unsigned int ret, amt; CHN_LOCKASSERT(c); - DEB( +/* DEB( if (c->flags & CHN_F_CLOSING) { sndbuf_dump(b, "b", 0x02); sndbuf_dump(bs, "bs", 0x02); - }) + }) */ if (c->flags & CHN_F_MAPPED) sndbuf_acquire(bs, NULL, sndbuf_getfree(bs)); amt = sndbuf_getfree(b); + KASSERT(amt <= sndbuf_getsize(bs), + ("%s(%s): amt %d > source size %d, flags 0x%x", __func__, c->name, + amt, sndbuf_getsize(bs), c->flags)); if (sndbuf_getready(bs) < amt) c->xruns++; @@ -746,6 +749,16 @@ c->devinfo = NULL; c->feeder = NULL; + ret = ENOMEM; + b = sndbuf_create(c->dev, c->name, "primary", c); + if (b == NULL) + goto out; + bs = sndbuf_create(c->dev, c->name, "secondary", c); + if (bs == NULL) + goto out; + + CHN_LOCK(c); + ret = EINVAL; fc = feeder_getclass(NULL); if (fc == NULL) @@ -753,21 +766,23 @@ if (chn_addfeeder(c, fc, NULL)) goto out; - ret = ENOMEM; - b = sndbuf_create(c->dev, c->name, "primary"); - if (b == NULL) - goto out; - bs = sndbuf_create(c->dev, c->name, "secondary"); - if (bs == NULL) - goto out; + /* + * XXX - sndbuf_setup() & sndbuf_resize() expect to be called + * with the channel unlocked because they are also called + * from driver methods that don't know about locking + */ + CHN_UNLOCK(c); sndbuf_setup(bs, NULL, 0); + CHN_LOCK(c); c->bufhard = b; c->bufsoft = bs; c->flags = 0; c->feederflags = 0; ret = ENODEV; + CHN_UNLOCK(c); /* XXX - Unlock for CHANNEL_INIT() malloc() call */ c->devinfo = CHANNEL_INIT(c->methods, devinfo, b, c, dir); + CHN_LOCK(c); if (c->devinfo == NULL) goto out; @@ -789,6 +804,7 @@ out: + CHN_UNLOCK(c); if (ret) { if (c->devinfo) { if (CHANNEL_FREE(c->methods, c->devinfo)) @@ -971,11 +987,17 @@ { struct snd_dbuf *b = c->bufhard; struct snd_dbuf *bs = c->bufsoft; - int bufsz, irqhz, tmp, ret; + int irqhz, tmp, ret, maxsize, reqblksz, tmpblksz; CHN_LOCKASSERT(c); - if (!CANCHANGE(c) || (c->flags & CHN_F_MAPPED)) + if (!CANCHANGE(c) || (c->flags & CHN_F_MAPPED)) { + KASSERT(sndbuf_getsize(bs) == 0 || + sndbuf_getsize(bs) >= sndbuf_getsize(b), + ("%s(%s): bufsoft size %d < bufhard size %d", __func__, + c->name, sndbuf_getsize(bs), sndbuf_getsize(b))); return EINVAL; + } + c->flags |= CHN_F_SETBLOCKSIZE; ret = 0; DEB(printf("%s(%d, %d)\n", __func__, blkcnt, blksz)); @@ -1007,36 +1029,70 @@ c->flags |= CHN_F_HAS_SIZE; } - bufsz = blkcnt * blksz; - - ret = sndbuf_remalloc(bs, blkcnt, blksz); - if (ret) - goto out; + reqblksz = blksz; /* 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); - sndbuf_setblksz(b, (sndbuf_getbps(b) * sndbuf_getspd(b)) / irqhz); + tmpblksz = (sndbuf_getbps(b) * sndbuf_getspd(b)) / irqhz; /* round down to 2^x */ blksz = 32; - while (blksz <= sndbuf_getblksz(b)) + while (blksz <= tmpblksz) blksz <<= 1; blksz >>= 1; /* round down to fit hw buffer size */ - RANGE(blksz, 16, sndbuf_getmaxsize(b) / 2); + if (sndbuf_getmaxsize(b) > 0) + RANGE(blksz, 16, sndbuf_getmaxsize(b) / 2); + else + /* virtual channels don't appear to allocate bufhard */ + RANGE(blksz, 16, CHN_2NDBUFMAXSIZE / 2); DEB(printf("%s: hard blksz requested %d (maxsize %d), ", __func__, blksz, sndbuf_getmaxsize(b))); + /* Increase the size of bufsoft if before increasing bufhard. */ + maxsize = sndbuf_getsize(b); + if (sndbuf_getsize(bs) > maxsize) + maxsize = sndbuf_getsize(bs); + if (reqblksz * blkcnt > maxsize) + maxsize = reqblksz * blkcnt; + if (sndbuf_getsize(bs) != maxsize || sndbuf_getblksz(bs) != reqblksz) { + ret = sndbuf_remalloc(bs, maxsize/reqblksz, reqblksz); + if (ret) + goto out1; + } + + CHN_UNLOCK(c); sndbuf_setblksz(b, CHANNEL_SETBLOCKSIZE(c->methods, c->devinfo, blksz)); + CHN_LOCK(c); + + /* Decrease the size of bufsoft after decreasing bufhard. */ + maxsize = sndbuf_getsize(b); + if (reqblksz * blkcnt > maxsize) + maxsize = reqblksz * blkcnt; + if (maxsize > sndbuf_getsize(bs)) + printf("Danger! %s bufsoft size increasing from %d to %d after CHANNEL_SETBLOCKSIZE()\n", + c->name, sndbuf_getsize(bs), maxsize); + if (sndbuf_getsize(bs) != maxsize || sndbuf_getblksz(bs) != reqblksz) { + ret = sndbuf_remalloc(bs, maxsize/reqblksz, reqblksz); + if (ret) + goto out1; + } irqhz = (sndbuf_getbps(b) * sndbuf_getspd(b)) / sndbuf_getblksz(b); DEB(printf("got %d, irqhz == %d\n", sndbuf_getblksz(b), irqhz)); chn_resetbuf(c); +out1: + KASSERT(sndbuf_getsize(bs) == 0 || + sndbuf_getsize(bs) >= sndbuf_getsize(b), + ("%s(%s): bufsoft size %d < bufhard size %d, reqblksz=%d blksz=%d maxsize=%d blkcnt=%d", + __func__, c->name, sndbuf_getsize(bs), sndbuf_getsize(b), reqblksz, + blksz, maxsize, blkcnt)); out: + c->flags &= ~CHN_F_SETBLOCKSIZE; return ret; } @@ -1216,8 +1272,12 @@ struct pcm_channel *child; int run; - if (SLIST_EMPTY(&c->children)) + CHN_LOCK(c); + + if (SLIST_EMPTY(&c->children)) { + CHN_UNLOCK(c); return ENODEV; + } run = (c->flags & CHN_F_TRIGGERED)? 1 : 0; /* @@ -1253,8 +1313,10 @@ blksz = sndbuf_getmaxsize(c->bufhard) / 2; SLIST_FOREACH(pce, &c->children, link) { child = pce->channel; + CHN_LOCK(child); if (sndbuf_getblksz(child->bufhard) < blksz) blksz = sndbuf_getblksz(child->bufhard); + CHN_UNLOCK(child); } chn_setblocksize(c, 2, blksz); } @@ -1268,13 +1330,28 @@ nrun = 0; SLIST_FOREACH(pce, &c->children, link) { child = pce->channel; + CHN_LOCK(child); if (child->flags & CHN_F_TRIGGERED) nrun = 1; + CHN_UNLOCK(child); } if (nrun && !run) chn_start(c, 1); if (!nrun && run) chn_abort(c); } + CHN_UNLOCK(c); return 0; +} + +void +chn_lock(struct pcm_channel *c) +{ + CHN_LOCK(c); +} + +void +chn_unlock(struct pcm_channel *c) +{ + CHN_UNLOCK(c); } 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 26 Jan 2004 21:17:42 -0000 @@ -99,11 +99,13 @@ void chn_rdupdate(struct pcm_channel *c); int chn_notify(struct pcm_channel *c, u_int32_t flags); +void chn_lock(struct pcm_channel *c); +void chn_unlock(struct pcm_channel *c); #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) @@ -134,6 +136,7 @@ #define CHN_F_MAPPED 0x00010000 /* has been mmap()ed */ #define CHN_F_DEAD 0x00020000 #define CHN_F_BADSETTING 0x00040000 +#define CHN_F_SETBLOCKSIZE 0x00080000 #define CHN_F_VIRTUAL 0x10000000 /* not backed by hardware */ Index: sys/dev/sound/pcm/dsp.c =================================================================== RCS file: /home/ncvs/src/sys/dev/sound/pcm/dsp.c,v retrieving revision 1.71 diff -u -r1.71 dsp.c --- sys/dev/sound/pcm/dsp.c 25 Jan 2004 22:46:22 -0000 1.71 +++ sys/dev/sound/pcm/dsp.c 26 Jan 2004 10:53:10 -0000 @@ -113,8 +113,8 @@ flags = dsp_get_flags(dev); d = dsp_get_info(dev); - pcm_lock(d); pcm_inprog(d, 1); + pcm_lock(d); KASSERT((flags & SD_F_PRIO_SET) != SD_F_PRIO_SET, \ ("getchns: read and write both prioritised")); @@ -159,9 +159,7 @@ CHN_UNLOCK(wrch); if (rdch && rdch != pcm_getfakechan(d) && (prio & SD_F_PRIO_RD)) CHN_UNLOCK(rdch); - pcm_lock(d); pcm_inprog(d, -1); - pcm_unlock(d); } static int @@ -476,6 +474,11 @@ wrch = NULL; if (kill & 2) rdch = NULL; + + if (rdch != NULL) + CHN_LOCK(rdch); + if (wrch != NULL) + CHN_LOCK(wrch); switch(cmd) { #ifdef OLDPCM_IOCTL @@ -497,16 +500,12 @@ p->play_size = 0; p->rec_size = 0; if (wrch) { - CHN_LOCK(wrch); chn_setblocksize(wrch, 2, p->play_size); p->play_size = sndbuf_getblksz(wrch->bufsoft); - CHN_UNLOCK(wrch); } if (rdch) { - CHN_LOCK(rdch); chn_setblocksize(rdch, 2, p->rec_size); p->rec_size = sndbuf_getblksz(rdch->bufsoft); - CHN_UNLOCK(rdch); } } break; @@ -526,16 +525,12 @@ snd_chan_param *p = (snd_chan_param *)arg; if (wrch) { - CHN_LOCK(wrch); chn_setformat(wrch, p->play_format); chn_setspeed(wrch, p->play_rate); - CHN_UNLOCK(wrch); } if (rdch) { - CHN_LOCK(rdch); chn_setformat(rdch, p->rec_format); chn_setspeed(rdch, p->rec_rate); - CHN_UNLOCK(rdch); } } /* FALLTHROUGH */ @@ -557,14 +552,10 @@ struct pcmchan_caps *pcaps = NULL, *rcaps = NULL; dev_t pdev; - if (rdch) { - CHN_LOCK(rdch); + if (rdch) rcaps = chn_getcaps(rdch); - } - if (wrch) { - CHN_LOCK(wrch); + if (wrch) pcaps = chn_getcaps(wrch); - } p->rate_min = max(rcaps? rcaps->minspeed : 0, pcaps? pcaps->minspeed : 0); p->rate_max = min(rcaps? rcaps->maxspeed : 1000000, @@ -580,10 +571,6 @@ p->mixers = 1; /* default: one mixer */ p->inputs = pdev->si_drv1? mix_getdevs(pdev->si_drv1) : 0; p->left = p->right = 100; - if (wrch) - CHN_UNLOCK(wrch); - if (rdch) - CHN_UNLOCK(rdch); } break; @@ -646,59 +633,42 @@ case SNDCTL_DSP_SETBLKSIZE: RANGE(*arg_i, 16, 65536); - if (wrch) { - CHN_LOCK(wrch); + if (wrch) chn_setblocksize(wrch, 2, *arg_i); - CHN_UNLOCK(wrch); - } - if (rdch) { - CHN_LOCK(rdch); + if (rdch) chn_setblocksize(rdch, 2, *arg_i); - CHN_UNLOCK(rdch); - } break; case SNDCTL_DSP_RESET: DEB(printf("dsp reset\n")); if (wrch) { - CHN_LOCK(wrch); chn_abort(wrch); chn_resetbuf(wrch); - CHN_UNLOCK(wrch); } if (rdch) { - CHN_LOCK(rdch); chn_abort(rdch); chn_resetbuf(rdch); - CHN_UNLOCK(rdch); } break; case SNDCTL_DSP_SYNC: DEB(printf("dsp sync\n")); /* chn_sync may sleep */ - if (wrch) { - CHN_LOCK(wrch); + if (wrch) chn_sync(wrch, sndbuf_getsize(wrch->bufsoft) - 4); - CHN_UNLOCK(wrch); - } break; case SNDCTL_DSP_SPEED: /* chn_setspeed may sleep */ tmp = 0; if (wrch) { - CHN_LOCK(wrch); ret = chn_setspeed(wrch, *arg_i); tmp = wrch->speed; - CHN_UNLOCK(wrch); } if (rdch && ret == 0) { - CHN_LOCK(rdch); ret = chn_setspeed(rdch, *arg_i); if (tmp == 0) tmp = rdch->speed; - CHN_UNLOCK(rdch); } *arg_i = tmp; break; @@ -711,17 +681,13 @@ tmp = -1; *arg_i = (*arg_i)? AFMT_STEREO : 0; if (wrch) { - CHN_LOCK(wrch); ret = chn_setformat(wrch, (wrch->format & ~AFMT_STEREO) | *arg_i); tmp = (wrch->format & AFMT_STEREO)? 1 : 0; - CHN_UNLOCK(wrch); } if (rdch && ret == 0) { - CHN_LOCK(rdch); ret = chn_setformat(rdch, (rdch->format & ~AFMT_STEREO) | *arg_i); if (tmp == -1) tmp = (rdch->format & AFMT_STEREO)? 1 : 0; - CHN_UNLOCK(rdch); } *arg_i = tmp; break; @@ -732,22 +698,17 @@ tmp = 0; *arg_i = (*arg_i != 1)? AFMT_STEREO : 0; if (wrch) { - CHN_LOCK(wrch); ret = chn_setformat(wrch, (wrch->format & ~AFMT_STEREO) | *arg_i); tmp = (wrch->format & AFMT_STEREO)? 2 : 1; - CHN_UNLOCK(wrch); } if (rdch && ret == 0) { - CHN_LOCK(rdch); ret = chn_setformat(rdch, (rdch->format & ~AFMT_STEREO) | *arg_i); if (tmp == 0) tmp = (rdch->format & AFMT_STEREO)? 2 : 1; - CHN_UNLOCK(rdch); } *arg_i = tmp; - } else { + } else *arg_i = ((wrch? wrch->format : rdch->format) & AFMT_STEREO)? 2 : 1; - } break; case SOUND_PCM_READ_CHANNELS: @@ -763,17 +724,13 @@ if ((*arg_i != AFMT_QUERY)) { tmp = 0; if (wrch) { - CHN_LOCK(wrch); ret = chn_setformat(wrch, (*arg_i) | (wrch->format & AFMT_STEREO)); tmp = wrch->format & ~AFMT_STEREO; - CHN_UNLOCK(wrch); } if (rdch && ret == 0) { - CHN_LOCK(rdch); ret = chn_setformat(rdch, (*arg_i) | (rdch->format & AFMT_STEREO)); if (tmp == 0) tmp = rdch->format & ~AFMT_STEREO; - CHN_UNLOCK(rdch); } *arg_i = tmp; } else @@ -800,18 +757,14 @@ DEB(printf("SNDCTL_DSP_SETFRAGMENT %d frags, %d sz\n", maxfrags, fragsz)); if (rdch) { - CHN_LOCK(rdch); ret = chn_setblocksize(rdch, maxfrags, fragsz); maxfrags = sndbuf_getblkcnt(rdch->bufsoft); fragsz = sndbuf_getblksz(rdch->bufsoft); - CHN_UNLOCK(rdch); } if (wrch && ret == 0) { - CHN_LOCK(wrch); ret = chn_setblocksize(wrch, maxfrags, fragsz); maxfrags = sndbuf_getblkcnt(wrch->bufsoft); fragsz = sndbuf_getblksz(wrch->bufsoft); - CHN_UNLOCK(wrch); } fragln = 0; @@ -830,12 +783,10 @@ if (rdch) { struct snd_dbuf *bs = rdch->bufsoft; - CHN_LOCK(rdch); a->bytes = sndbuf_getready(bs); a->fragments = a->bytes / sndbuf_getblksz(bs); a->fragstotal = sndbuf_getblkcnt(bs); a->fragsize = sndbuf_getblksz(bs); - CHN_UNLOCK(rdch); } } break; @@ -847,13 +798,11 @@ if (wrch) { struct snd_dbuf *bs = wrch->bufsoft; - CHN_LOCK(wrch); chn_wrupdate(wrch); a->bytes = sndbuf_getfree(bs); a->fragments = a->bytes / sndbuf_getblksz(bs); a->fragstotal = sndbuf_getblkcnt(bs); a->fragsize = sndbuf_getblksz(bs); - CHN_UNLOCK(wrch); } } break; @@ -864,13 +813,11 @@ if (rdch) { struct snd_dbuf *bs = rdch->bufsoft; - CHN_LOCK(rdch); chn_rdupdate(rdch); a->bytes = sndbuf_gettotal(bs); a->blocks = sndbuf_getblocks(bs) - rdch->blocks; a->ptr = sndbuf_getreadyptr(bs); rdch->blocks = sndbuf_getblocks(bs); - CHN_UNLOCK(rdch); } else ret = EINVAL; } @@ -882,13 +829,11 @@ if (wrch) { struct snd_dbuf *bs = wrch->bufsoft; - CHN_LOCK(wrch); chn_wrupdate(wrch); a->bytes = sndbuf_gettotal(bs); a->blocks = sndbuf_getblocks(bs) - wrch->blocks; a->ptr = sndbuf_getreadyptr(bs); wrch->blocks = sndbuf_getblocks(bs); - CHN_UNLOCK(wrch); } else ret = EINVAL; } @@ -906,22 +851,18 @@ case SNDCTL_DSP_SETTRIGGER: if (rdch) { - CHN_LOCK(rdch); rdch->flags &= ~(CHN_F_TRIGGERED | CHN_F_NOTRIGGER); if (*arg_i & PCM_ENABLE_INPUT) chn_start(rdch, 1); else rdch->flags |= CHN_F_NOTRIGGER; - CHN_UNLOCK(rdch); } if (wrch) { - CHN_LOCK(wrch); wrch->flags &= ~(CHN_F_TRIGGERED | CHN_F_NOTRIGGER); if (*arg_i & PCM_ENABLE_OUTPUT) chn_start(wrch, 1); else wrch->flags |= CHN_F_NOTRIGGER; - CHN_UNLOCK(wrch); } break; @@ -938,20 +879,16 @@ struct snd_dbuf *b = wrch->bufhard; struct snd_dbuf *bs = wrch->bufsoft; - CHN_LOCK(wrch); chn_wrupdate(wrch); *arg_i = sndbuf_getready(b) + sndbuf_getready(bs); - CHN_UNLOCK(wrch); } else ret = EINVAL; break; case SNDCTL_DSP_POST: if (wrch) { - CHN_LOCK(wrch); wrch->flags &= ~CHN_F_NOTRIGGER; chn_start(wrch, 1); - CHN_UNLOCK(wrch); } break; @@ -969,6 +906,10 @@ ret = EINVAL; break; } + if (rdch != NULL) + CHN_UNLOCK(rdch); + if (wrch != NULL) + CHN_UNLOCK(wrch); relchns(i_dev, rdch, wrch, 0); splx(s); return ret; Index: sys/dev/sound/pcm/sound.c =================================================================== RCS file: /home/ncvs/src/sys/dev/sound/pcm/sound.c,v retrieving revision 1.88 diff -u -r1.88 sound.c --- sys/dev/sound/pcm/sound.c 20 Jan 2004 03:58:57 -0000 1.88 +++ sys/dev/sound/pcm/sound.c 20 Jan 2004 10:34:46 -0000 @@ -75,7 +75,23 @@ m = malloc(sizeof(*m), M_DEVBUF, M_WAITOK | M_ZERO); if (m == NULL) return NULL; - mtx_init(m, desc, type, MTX_DEF | MTX_RECURSE); + mtx_init(m, desc, type, MTX_DEF); + return m; +#else + return (void *)0xcafebabe; +#endif +} + +void * +snd_chnmtxcreate(const char *desc, const char *type) +{ +#ifdef USING_MUTEX + struct mtx *m; + + m = malloc(sizeof(*m), M_DEVBUF, M_WAITOK | M_ZERO); + if (m == NULL) + return NULL; + mtx_init(m, desc, type, MTX_DEF | MTX_DUPOK); return m; #else return (void *)0xcafebabe; @@ -188,13 +204,16 @@ /* try to create a vchan */ SLIST_FOREACH(sce, &d->channels, link) { c = sce->channel; + CHN_LOCK(c); if (!SLIST_EMPTY(&c->children)) { err = vchan_create(c); + CHN_UNLOCK(c); if (!err) return pcm_chnalloc(d, direction, pid, -1); else device_printf(d->dev, "vchan_create(%s) == %d\n", c->name, err); - } + } else + CHN_UNLOCK(c); } } } @@ -250,15 +269,19 @@ if (num > 0 && d->vchancount == 0) { SLIST_FOREACH(sce, &d->channels, link) { c = sce->channel; + CHN_LOCK(c); if ((c->direction == PCMDIR_PLAY) && !(c->flags & CHN_F_BUSY)) { c->flags |= CHN_F_BUSY; err = vchan_create(c); if (err) { - device_printf(d->dev, "vchan_create(%s) == %d\n", c->name, err); c->flags &= ~CHN_F_BUSY; - } + CHN_UNLOCK(c); + device_printf(d->dev, "vchan_create(%s) == %d\n", c->name, err); + } else + CHN_UNLOCK(c); return; } + CHN_UNLOCK(c); } } if (num == 0 && d->vchancount > 0) { @@ -313,7 +336,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++) { @@ -529,20 +552,23 @@ err = pcm_chn_add(d, ch); if (err) { device_printf(d->dev, "pcm_chn_add(%s) failed, err=%d\n", ch->name, err); - snd_mtxunlock(d->lock); pcm_chn_destroy(ch); return err; } + CHN_LOCK(ch); if (snd_maxautovchans > 0 && (d->flags & SD_F_AUTOVCHAN) && ch->direction == PCMDIR_PLAY && d->vchancount == 0) { ch->flags |= CHN_F_BUSY; err = vchan_create(ch); if (err) { - device_printf(d->dev, "vchan_create(%s) == %d\n", ch->name, err); ch->flags &= ~CHN_F_BUSY; + CHN_UNLOCK(ch); + device_printf(d->dev, "vchan_create(%s) == %d\n", ch->name, err); + return err; } } + CHN_UNLOCK(ch); return err; } @@ -866,11 +892,13 @@ cnt = 0; SLIST_FOREACH(sce, &d->channels, link) { c = sce->channel; + CHN_LOCK(c); if ((c->direction == PCMDIR_PLAY) && (c->flags & CHN_F_VIRTUAL)) { cnt++; if (c->flags & CHN_F_BUSY) busy++; } + CHN_UNLOCK(c); } newcnt = cnt; @@ -888,23 +916,28 @@ /* add new vchans - find a parent channel first */ SLIST_FOREACH(sce, &d->channels, link) { c = sce->channel; + CHN_LOCK(c); /* not a candidate if not a play channel */ if (c->direction != PCMDIR_PLAY) - continue; + goto next; /* not a candidate if a virtual channel */ if (c->flags & CHN_F_VIRTUAL) - continue; + goto next; /* not a candidate if it's in use */ - if ((c->flags & CHN_F_BUSY) && (SLIST_EMPTY(&c->children))) - continue; - /* - * if we get here we're a nonvirtual play channel, and either - * 1) not busy - * 2) busy with children, not directly open - * - * thus we can add children - */ - goto addok; + if (!(c->flags & CHN_F_BUSY) || + !(SLIST_EMPTY(&c->children))) + /* + * if we get here we're a nonvirtual + * play channel, and either + * 1) not busy + * 2) busy with children, not directly + * open + * + * thus we can add children + */ + goto addok; +next: + CHN_UNLOCK(c); } pcm_inprog(d, -1); return EBUSY; @@ -917,6 +950,7 @@ } if (SLIST_EMPTY(&c->children)) c->flags &= ~CHN_F_BUSY; + CHN_UNLOCK(c); } else if (newcnt < cnt) { if (busy > newcnt) { printf("cnt %d, newcnt %d, busy %d\n", cnt, newcnt, busy); @@ -928,13 +962,17 @@ while (err == 0 && newcnt < cnt) { SLIST_FOREACH(sce, &d->channels, link) { c = sce->channel; + CHN_LOCK(c); if ((c->flags & (CHN_F_BUSY | CHN_F_VIRTUAL)) == CHN_F_VIRTUAL) goto remok; + + CHN_UNLOCK(c); } snd_mtxunlock(d->lock); pcm_inprog(d, -1); return EINVAL; remok: + CHN_UNLOCK(c); err = vchan_destroy(c); if (err == 0) cnt--; Index: sys/dev/sound/pcm/sound.h =================================================================== RCS file: /home/ncvs/src/sys/dev/sound/pcm/sound.h,v retrieving revision 1.54 diff -u -r1.54 sound.h --- sys/dev/sound/pcm/sound.h 20 Jan 2004 03:58:57 -0000 1.54 +++ sys/dev/sound/pcm/sound.h 20 Jan 2004 10:34:46 -0000 @@ -238,6 +238,7 @@ driver_intr_t hand, void *param, void **cookiep); void *snd_mtxcreate(const char *desc, const char *type); +void *snd_chnmtxcreate(const char *desc, const char *type); void snd_mtxfree(void *m); void snd_mtxassert(void *m); #define snd_mtxlock(m) mtx_lock(m) Index: sys/dev/sound/pcm/vchan.c =================================================================== RCS file: /home/ncvs/src/sys/dev/sound/pcm/vchan.c,v retrieving revision 1.15 diff -u -r1.15 vchan.c --- sys/dev/sound/pcm/vchan.c 20 Jan 2004 03:58:57 -0000 1.15 +++ sys/dev/sound/pcm/vchan.c 26 Jan 2004 22:07:09 -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("feed_vchan_s16(%s): tmp buffer size %d < count %d, flags = 0x%x", + c->name, sndbuf_getsize(src), count, c->flags); count &= ~1; bzero(b, count); @@ -92,12 +94,14 @@ bzero(tmp, count); SLIST_FOREACH(cce, &c->children, link) { ch = cce->channel; + CHN_LOCK(ch); if (ch->flags & CHN_F_TRIGGERED) { if (ch->flags & CHN_F_MAPPED) sndbuf_acquire(ch->bufsoft, NULL, sndbuf_getfree(ch->bufsoft)); cnt = FEEDER_FEED(ch->feeder, ch, (u_int8_t *)tmp, count, ch->bufsoft); vchan_mix_s16(dst, tmp, cnt / 2); } + CHN_UNLOCK(ch); } return count; @@ -145,13 +149,16 @@ { struct vchinfo *ch = data; struct pcm_channel *parent = ch->parent; + struct pcm_channel *channel = ch->channel; ch->fmt = format; ch->bps = 1; ch->bps <<= (ch->fmt & AFMT_STEREO)? 1 : 0; ch->bps <<= (ch->fmt & AFMT_16BIT)? 1 : 0; ch->bps <<= (ch->fmt & AFMT_32BIT)? 2 : 0; + CHN_UNLOCK(channel); chn_notify(parent, CHN_N_FORMAT); + CHN_LOCK(channel); return 0; } @@ -160,9 +167,12 @@ { struct vchinfo *ch = data; struct pcm_channel *parent = ch->parent; + struct pcm_channel *channel = ch->channel; ch->spd = speed; + CHN_UNLOCK(channel); chn_notify(parent, CHN_N_RATE); + CHN_LOCK(channel); return speed; } @@ -171,14 +181,19 @@ { struct vchinfo *ch = data; struct pcm_channel *parent = ch->parent; + /* struct pcm_channel *channel = ch->channel; */ int prate, crate; ch->blksz = blocksize; + /* CHN_UNLOCK(channel); */ chn_notify(parent, CHN_N_BLOCKSIZE); + CHN_LOCK(parent); + /* CHN_LOCK(channel); */ crate = ch->spd * ch->bps; prate = sndbuf_getspd(parent->bufhard) * sndbuf_getbps(parent->bufhard); blocksize = sndbuf_getblksz(parent->bufhard); + CHN_UNLOCK(parent); blocksize *= prate; blocksize /= crate; @@ -190,12 +205,15 @@ { struct vchinfo *ch = data; struct pcm_channel *parent = ch->parent; + struct pcm_channel *channel = ch->channel; if (go == PCMTRIG_EMLDMAWR || go == PCMTRIG_EMLDMARD) return 0; ch->run = (go == PCMTRIG_START)? 1 : 0; + CHN_UNLOCK(channel); chn_notify(parent, CHN_N_TRIGGER); + CHN_LOCK(channel); return 0; } @@ -235,8 +253,11 @@ struct pcm_channel *child; int err, first; + CHN_UNLOCK(parent); + pce = malloc(sizeof(*pce), M_DEVBUF, M_WAITOK | M_ZERO); if (!pce) { + CHN_LOCK(parent); return ENOMEM; } @@ -244,14 +265,13 @@ child = pcm_chn_create(d, parent, &vchan_class, PCMDIR_VIRTUAL, parent); if (!child) { free(pce, M_DEVBUF); + CHN_LOCK(parent); return ENODEV; } CHN_LOCK(parent); - if (!(parent->flags & CHN_F_BUSY)) { - CHN_UNLOCK(parent); + if (!(parent->flags & CHN_F_BUSY)) return EBUSY; - } first = SLIST_EMPTY(&parent->children); /* add us to our parent channel's children */ @@ -269,6 +289,7 @@ free(pce, M_DEVBUF); } + CHN_LOCK(parent); /* XXX gross ugly hack, murder death kill */ if (first && !err) { err = chn_reset(parent, AFMT_STEREO | AFMT_S16_LE);