On Jan 24, Don Lewis wrote: > The is a buffer overflow in the interrupt handler in the vchan code that > occasionally stomps on something on the kernel heap and causes panics in > other parts of the kernel. I tracked down the cause of the buffer > overflow with the help of Stefan Ehmann and made a number of changes to > the sound/pcm/* code to try to avoid triggering the overflow. > [...] Don, I think it should be noted that vchans aren't always enabled by default and those testing this patch should explicitly make sure that vchans are enabled either via sysctl hw.snd.pcmX.vchans=5 I've tested this patch with xmms and mplayer and forcing them to cycle through many songs, and it seems at least functional. I would like to see the patch commited very soon. (Say tommorow?) btw, the locking cleanup is very helpful. --Mat > > Because of the architecture of the sound code, it may not be totally > possible to avoid the potential for a buffer overflow, so I added code > to print a diagnostic message if this situation occurs, as well as code > to panic() the machine if an interrupt happens at the "wrong" time in > this situation and a buffer overflow is imminent. > > I cleaned up the channel locking code, but there are still issues with > the usage of pcm_lock() that have not been addressed. > > I also fixed up a NULL pointer dereference that was noticed by Ryan > Somers. > > You will need a fairly recent version of -CURRENT, with > src/sys/dev/sound/pcm/dsp.c rev 1.70. I highly recommend testing this > patch with the INVARIANTS kernel option, as well as WITNESS, if > possible, to flush out any potential problems and track them quickly to > their source. > > Please let me know if you get the "Danger!" diagnostic message. > > > Here is the complete list of changes: > > Change KASSERT() in feed_vchan16() into an explicit test and call to > panic() so that the buffer overflow just beyond this point is always > caught, even when the code is not compiled with INVARIANTS. > > Change chn_setblocksize() buffer reallocation code to attempt to avoid > the feed_vchan16() buffer overflow by attempting to always keep the > bufsoft buffer at least as large as the bufhard buffer. > > Print a diagnositic message > Danger! %s bufsoft size increasing from %d to %d after CHANNEL_SETBLOCKSIZE() > if our best attempts fail. If feed_vchan16() were to be called by > the interrupt handler while locks are dropped in chn_setblocksize() > to increase the size bufsoft to match the size of bufhard, the panic() > code in feed_vchan16() will be triggered. If the diagnostic message > is printed, it is a warning that a panic is possible if the system > were to see events in an "unlucky" order. > > Change the locking code to avoid the need for MTX_RECURSIVE mutexes. > > Add the MTX_DUPOK option to the channel mutexes and change the locking > sequence to always lock the parent channel before its children to avoid > the possibility of deadlock. > > Actually implement locking assertions for the channel mutexes and fix > the problems found by the resulting assertion violations. > > Clean up the locking code in dsp_ioctl(). > > Allocate the channel buffers using the malloc() M_WAITOK option instead > of M_NOWAIT so that buffer allocation won't fail. Drop locks across > the malloc() calls. > > Add/modify KASSERTS() in attempt to detect problems early. > > Don't dereference a NULL pointer when setting hw.snd.maxautovchans > if a hardware driver is not loaded. Noticed by Ryan Sommers > <ryans at gamersimpact.com>. > > > 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 19 Jan 2004 00:33:51 -0000 > _at__at_ -30,6 +30,14 _at__at_ > > SND_DECLARE_FILE("$FreeBSD: src/sys/dev/sound/pcm/buffer.c,v 1.21 2003/11/27 19:51:44 matk Exp $"); > > +#ifdef USING_MUTEX > +#define MTX_LOCK(lock) mtx_lock(lock); > +#define MTX_UNLOCK(lock) mtx_unlock(lock); > +#else > +#define MTX_LOCK(lock) > +#define MTX_UNLOCK(lock) > +#endif > + > struct snd_dbuf * > sndbuf_create(device_t dev, char *drv, char *desc) > { > _at__at_ -128,7 +136,7 _at__at_ > b->blksz = blksz; > b->bufsize = blkcnt * blksz; > > - tmpbuf = malloc(b->bufsize, M_DEVBUF, M_NOWAIT); > + tmpbuf = malloc(b->bufsize, M_DEVBUF, M_WAITOK); > if (tmpbuf == NULL) > return ENOMEM; > free(b->tmpbuf, M_DEVBUF); > _at__at_ -138,25 +146,32 _at__at_ > } > > int > -sndbuf_remalloc(struct snd_dbuf *b, unsigned int blkcnt, unsigned int blksz) > +sndbuf_remalloc(struct snd_dbuf *b, unsigned int blkcnt, unsigned int blksz, > + struct mtx *lock) > { > 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; > + MTX_UNLOCK(lock); > + 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; > } > + MTX_LOCK(lock); > > b->blkcnt = blkcnt; > b->blksz = blksz; > _at__at_ -167,13 +182,18 _at__at_ > b->buf = buf; > b->tmpbuf = tmpbuf; > > + sndbuf_reset(b); > + > + MTX_UNLOCK(lock); > if (f1) > free(f1, M_DEVBUF); > if (f2) > free(f2, M_DEVBUF); > > - sndbuf_reset(b); > - return 0; > + ret = 0; > +out: > + MTX_LOCK(lock); > + 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 17 Jan 2004 05:40:24 -0000 > _at__at_ -65,7 +65,7 _at__at_ > int sndbuf_setup(struct snd_dbuf *b, void *buf, unsigned int size); > void sndbuf_free(struct snd_dbuf *b); > int sndbuf_resize(struct snd_dbuf *b, unsigned int blkcnt, unsigned int blksz); > -int sndbuf_remalloc(struct snd_dbuf *b, unsigned int blkcnt, unsigned int blksz); > +int sndbuf_remalloc(struct snd_dbuf *b, unsigned int blkcnt, unsigned int blksz, struct mtx *lock); > void sndbuf_reset(struct snd_dbuf *b); > void sndbuf_clear(struct snd_dbuf *b, unsigned int length); > void sndbuf_fillsilence(struct snd_dbuf *b); > 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 20 Jan 2004 20:19:37 -0000 > _at__at_ -70,9 +70,9 _at__at_ > 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 > _at__at_ -205,16 +205,19 _at__at_ > 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++; > > _at__at_ -746,13 +749,6 _at__at_ > c->devinfo = NULL; > c->feeder = NULL; > > - ret = EINVAL; > - fc = feeder_getclass(NULL); > - if (fc == NULL) > - goto out; > - if (chn_addfeeder(c, fc, NULL)) > - goto out; > - > ret = ENOMEM; > b = sndbuf_create(c->dev, c->name, "primary"); > if (b == NULL) > _at__at_ -760,6 +756,16 _at__at_ > bs = sndbuf_create(c->dev, c->name, "secondary"); > if (bs == NULL) > goto out; > + > + CHN_LOCK(c); > + > + ret = EINVAL; > + fc = feeder_getclass(NULL); > + if (fc == NULL) > + goto out; > + if (chn_addfeeder(c, fc, NULL)) > + goto out; > + > sndbuf_setup(bs, NULL, 0); > c->bufhard = b; > c->bufsoft = bs; > _at__at_ -767,7 +773,9 _at__at_ > 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; > > _at__at_ -789,6 +797,7 _at__at_ > > > out: > + CHN_UNLOCK(c); > if (ret) { > if (c->devinfo) { > if (CHANNEL_FREE(c->methods, c->devinfo)) > _at__at_ -971,11 +980,17 _at__at_ > { > 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)); > _at__at_ -1007,36 +1022,68 _at__at_ > 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, c->lock); > + if (ret) > + goto out1; > + } > + > sndbuf_setblksz(b, CHANNEL_SETBLOCKSIZE(c->methods, c->devinfo, blksz)); > > + /* 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, c->lock); > + 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; > } > > _at__at_ -1216,8 +1263,12 _at__at_ > 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; > /* > _at__at_ -1253,8 +1304,10 _at__at_ > 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); > } > _at__at_ -1268,13 +1321,16 _at__at_ > 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; > } > 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 19 Jan 2004 04:32:36 -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) > _at__at_ -134,6 +134,7 _at__at_ > #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.70 > diff -u -r1.70 dsp.c > --- sys/dev/sound/pcm/dsp.c 20 Jan 2004 05:30:09 -0000 1.70 > +++ sys/dev/sound/pcm/dsp.c 23 Jan 2004 09:47:54 -0000 > _at__at_ -113,8 +113,8 _at__at_ > > 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")); > > _at__at_ -159,9 +159,7 _at__at_ > 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 > _at__at_ -476,6 +474,11 _at__at_ > wrch = NULL; > if (kill & 2) > rdch = NULL; > + > + if (rdch != NULL) > + CHN_LOCK(rdch); > + if (wrch != NULL) > + CHN_LOCK(wrch); > > switch(cmd) { > #ifdef OLDPCM_IOCTL > _at__at_ -497,16 +500,12 _at__at_ > 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; > _at__at_ -526,16 +525,12 _at__at_ > 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 */ > _at__at_ -557,14 +552,10 _at__at_ > 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, > _at__at_ -580,10 +571,6 _at__at_ > 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; > > _at__at_ -646,59 +633,42 _at__at_ > > 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; > _at__at_ -711,17 +681,13 _at__at_ > 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; > _at__at_ -732,22 +698,17 _at__at_ > 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: > _at__at_ -763,17 +724,13 _at__at_ > 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 > _at__at_ -800,18 +757,14 _at__at_ > > 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; > _at__at_ -830,12 +783,10 _at__at_ > 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; > _at__at_ -847,13 +798,11 _at__at_ > 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; > _at__at_ -864,13 +813,11 _at__at_ > 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; > } > _at__at_ -882,13 +829,11 _at__at_ > 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; > } > _at__at_ -906,22 +851,18 _at__at_ > > 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; > > _at__at_ -938,20 +879,16 _at__at_ > 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; > > _at__at_ -969,6 +906,10 _at__at_ > 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 > _at__at_ -75,7 +75,23 _at__at_ > 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; > _at__at_ -188,13 +204,16 _at__at_ > /* 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); > } > } > } > _at__at_ -250,15 +269,19 _at__at_ > 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) { > _at__at_ -313,7 +336,7 _at__at_ > 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++) { > _at__at_ -529,20 +552,23 _at__at_ > 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; > } > _at__at_ -866,11 +892,13 _at__at_ > 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; > _at__at_ -888,23 +916,28 _at__at_ > /* 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; > _at__at_ -917,6 +950,7 _at__at_ > } > 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); > _at__at_ -928,13 +962,17 _at__at_ > 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 > _at__at_ -238,6 +238,7 _at__at_ > 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 25 Jan 2004 01:54:21 -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("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); > > _at__at_ -92,12 +94,14 _at__at_ > 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; > _at__at_ -145,13 +149,16 _at__at_ > { > 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; > } > > _at__at_ -160,9 +167,12 _at__at_ > { > 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; > } > > _at__at_ -171,14 +181,19 _at__at_ > { > 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; > > _at__at_ -190,12 +205,15 _at__at_ > { > 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; > } > _at__at_ -235,8 +253,11 _at__at_ > 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; > } > > _at__at_ -244,14 +265,13 _at__at_ > 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 */ > _at__at_ -269,6 +289,7 _at__at_ > 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); > > _______________________________________________ > freebsd-current_at_freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-current > To unsubscribe, send any mail to "freebsd-current-unsubscribe_at_freebsd.org" -- If you optimize everything, you will always be unhappy. - Don KnuthReceived on Sun Jan 25 2004 - 08:36:52 UTC
This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:37:39 UTC