dev/sound/pcm/* patch testers wanted

From: Don Lewis <truckman_at_FreeBSD.org>
Date: Sat, 24 Jan 2004 18:04:02 -0800 (PST)
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.

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);
Received on Sat Jan 24 2004 - 17:04:14 UTC

This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:37:39 UTC