Date: Sat, 17 Jan 2004 01:15:13 -0800 (PST) From: Don Lewis <truckman@FreeBSD.org> To: shoesoft@gmx.net Cc: mat@cnd.mcgill.ca Subject: Re: sound/pcm/* bugs (was: Re: page fault panic tracked down (selwakeuppri()) - really sound/pcm/*) Message-ID: <200401170915.i0H9FD7E047822@gw.catspoiler.org> In-Reply-To: <1074282714.8095.39.camel@shoeserv.freebsd>
next in thread | previous in thread | raw e-mail | index | archive | help
On 16 Jan, Stefan Ehmann wrote:
>> The following patch seems to be at least somewhat stable on my machine.
>> I did get one last panic in feed_vchan_s16() that I haven't been able to
>> reproduce since I added some more sanity checks. Since it was caught in
>> the interrupt routine, it's not easy to track down to its root cause. I
>> think there is still a bug somewhere, but it's not easy to trigger.
>
> Good news: no panic so far.
>
> Bad news: sound stopped working after some time (Chances are that the
> system would have panicked on the unpatched kernel at the same time)
>
> /dev/dsp: Operation not supported by device
>
> I'm not sure why this happened.
I think this problem can be caused by a transient malloc() M_NOWAIT
failure.
Changes in this version of the patch:
When increasing blksz in chn_setblocksize(), increase the size
of bufsoft before increasing bufhard, and decrease bufsoft after
decreasing bufhard in an attempt to avoid the buffer overflow in
feed_vchan_s16().
Buffers are now allocated using M_WAITOK, requiring locks to
be dropped for the malloc() calls. This required adding a mutex
parameter to sndbuf_remalloc().
Locking order between parent and child channels is changed. The
parent is now locked first and then the child. The list of
children is protected by the parent's lock. There are still
potential race conditions in the vchan creation/destruction code
because all the locks have to be dropped in order to call
malloc(), etc.
Locking cleaned up to eliminate the need for MTX_RECURSE.
A new mutex allocator for the channel mutexes was added that
initializes the mutexes with the MTX_DUPOK flags so that multiple
channel mutexes of the same type can be held at once.
Locking simplification in dsp_ioctl().
Added KASSERTs in chn_wrfeed().
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 17 Jan 2004 06:11:11 -0000
@@ -30,6 +30,14 @@
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)
{
@@ -128,7 +136,7 @@
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);
@@ -138,25 +146,32 @@
}
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;
@@ -167,13 +182,18 @@
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
@@ -65,7 +65,7 @@
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 17 Jan 2004 05:53:48 -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,18 @@
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(b), ("chn_wrfeed amt > size"));
+ KASSERT(amt <= sndbuf_getsize(bs), ("chn_wrfeed amt > source size"));
if (sndbuf_getready(bs) < amt)
c->xruns++;
@@ -746,13 +748,6 @@
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)
@@ -760,6 +755,16 @@
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;
@@ -767,7 +772,9 @@
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 +796,7 @@
out:
+ CHN_UNLOCK(c);
if (ret) {
if (c->devinfo) {
if (CHANNEL_FREE(c->methods, c->devinfo))
@@ -971,7 +979,7 @@
{
struct snd_dbuf *b = c->bufhard;
struct snd_dbuf *bs = c->bufsoft;
- int bufsz, irqhz, tmp, ret;
+ int irqhz, tmp, ret, maxblksz, reqblksz;
CHN_LOCKASSERT(c);
if (!CANCHANGE(c) || (c->flags & CHN_F_MAPPED))
@@ -1007,14 +1015,22 @@
c->flags |= CHN_F_HAS_SIZE;
}
- bufsz = blkcnt * blksz;
-
- ret = sndbuf_remalloc(bs, blkcnt, blksz);
- if (ret)
- goto out;
+ /* Increase the size of bufsoft before increasing bufhard. */
+ reqblksz = blksz;
+ maxblksz = sndbuf_getblksz(b);
+ if (reqblksz > maxblksz)
+ maxblksz = reqblksz;
+ if (sndbuf_getblksz(bs) != maxblksz) {
+ ret = sndbuf_remalloc(bs, blkcnt, maxblksz, c->lock);
+ if (ret) {
+ printf("%s: sndbuf_remalloc #1 (bs, %d %d) returned %d\n",
+ __func__, blkcnt, maxblksz, 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);
@@ -1032,6 +1048,19 @@
sndbuf_setblksz(b, CHANNEL_SETBLOCKSIZE(c->methods, c->devinfo, blksz));
+ /* Decrease the size of bufsoft after decreasing bufhard. */
+ maxblksz = sndbuf_getblksz(b);
+ if (reqblksz > maxblksz)
+ maxblksz = reqblksz;
+ if (sndbuf_getblksz(bs) != maxblksz) {
+ ret = sndbuf_remalloc(bs, blkcnt, maxblksz, c->lock);
+ if (ret) {
+ printf("%s: sndbuf_remalloc #2 (bs, %d %d) returned %d\n",
+ __func__, blkcnt, maxblksz, ret);
+ goto out;
+ }
+ }
+
irqhz = (sndbuf_getbps(b) * sndbuf_getspd(b)) / sndbuf_getblksz(b);
DEB(printf("got %d, irqhz == %d\n", sndbuf_getblksz(b), irqhz));
@@ -1216,8 +1245,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 +1286,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 +1303,16 @@
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 16 Jan 2004 01:35:12 -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/dsp.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/sound/pcm/dsp.c,v
retrieving revision 1.67
diff -u -r1.67 dsp.c
--- sys/dev/sound/pcm/dsp.c 11 Nov 2003 05:38:28 -0000 1.67
+++ sys/dev/sound/pcm/dsp.c 17 Jan 2004 06:25:08 -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
@@ -480,6 +478,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
@@ -501,16 +504,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;
@@ -530,16 +529,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 */
@@ -561,14 +556,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,
@@ -584,10 +575,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;
@@ -650,16 +637,10 @@
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:
@@ -673,28 +654,21 @@
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;
@@ -707,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;
@@ -728,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:
@@ -759,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
@@ -796,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;
@@ -826,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;
@@ -843,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;
@@ -860,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;
}
@@ -878,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;
}
@@ -902,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;
@@ -934,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;
@@ -965,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.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 17 Jan 2004 08:26:00 -0000
@@ -96,7 +96,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;
@@ -209,13 +225,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);
}
}
}
@@ -271,15 +290,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) {
@@ -334,7 +357,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++) {
@@ -528,20 +551,23 @@
err = pcm_chn_add(d, ch, 1);
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;
}
@@ -852,11 +878,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;
@@ -874,23 +902,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;
@@ -903,6 +936,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);
@@ -914,13 +948,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.52
diff -u -r1.52 sound.h
--- sys/dev/sound/pcm/sound.h 7 Sep 2003 16:28:03 -0000 1.52
+++ sys/dev/sound/pcm/sound.h 17 Jan 2004 05:53:02 -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.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 17 Jan 2004 07:30:05 -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(): tmp buffer size %d < count %d",
+ sndbuf_getsize(src), count);
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 */
@@ -268,13 +288,17 @@
/* 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);
}
+
+ CHN_LOCK(parent);
return err;
}
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200401170915.i0H9FD7E047822>