Re: sound/pcm/* bugs (was: Re: page fault panic tracked down (selwakeuppri()) - really sound/pcm/*)

From: Don Lewis <truckman_at_FreeBSD.org>
Date: Sat, 17 Jan 2004 01:15:13 -0800 (PST)
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
_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	17 Jan 2004 05:53:48 -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,18 _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(b), ("chn_wrfeed amt > size"));
+	KASSERT(amt <= sndbuf_getsize(bs), ("chn_wrfeed amt > source size"));
 	if (sndbuf_getready(bs) < amt)
 		c->xruns++;
 
_at__at_ -746,13 +748,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 +755,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 +772,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 +796,7 _at__at_
 
 
 out:
+	CHN_UNLOCK(c);
 	if (ret) {
 		if (c->devinfo) {
 			if (CHANNEL_FREE(c->methods, c->devinfo))
_at__at_ -971,7 +979,7 _at__at_
 {
 	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))
_at__at_ -1007,14 +1015,22 _at__at_
 		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);
 
_at__at_ -1032,6 +1048,19 _at__at_
 
 	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));
 
_at__at_ -1216,8 +1245,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 +1286,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 +1303,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	16 Jan 2004 01:35:12 -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)
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
_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_ -480,6 +478,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_ -501,16 +504,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_ -530,16 +529,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_ -561,14 +556,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_ -584,10 +575,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_ -650,16 +637,10 _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:
_at__at_ -673,28 +654,21 _at__at_
     	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_ -707,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_ -728,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_ -759,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_ -796,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_ -826,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_ -843,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_ -860,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_ -878,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_ -902,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_ -934,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_ -965,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.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
_at__at_ -96,7 +96,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_ -209,13 +225,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_ -271,15 +290,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_ -334,7 +357,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_ -528,20 +551,23 _at__at_
 	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;
 }
_at__at_ -852,11 +878,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_ -874,23 +902,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_ -903,6 +936,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_ -914,13 +948,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.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
_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.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
_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(): tmp buffer size %d < count %d",
+		    sndbuf_getsize(src), count);
 	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_ -268,13 +288,17 _at__at_
 
 	/* 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;
 }
Received on Sat Jan 17 2004 - 00:16:00 UTC

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