Re: dev/sound/pcm/* patch testers wanted

From: Don Lewis <truckman_at_FreeBSD.org>
Date: Mon, 26 Jan 2004 15:04:55 -0800 (PST)
On 26 Jan, To: gbergling_at_0xfce3.net wrote:
> On 26 Jan, Gordon Bergling wrote:
>> On Mon Jan 26, 2004 at 09:25AM -0800, Don Lewis wrote:
>>> On 26 Jan, Gordon Bergling wrote:
>>> 
>>> > The "Danger!" message wasn't appeared, but I got the following on the
>>> > console.
>>> > 
>>> > malloc() of "1024" with the following non-sleepable locks held:
>>> > exclusive sleep mutex pcm0:play:0 (pcm play channel) r = 0 (0xc2be8a80)
>>> > locked _at_
>>> > /storage/os/src/freebsd-current/src/sys/dev/sound/pcm/buffer.c:195
>>> > malloc() of "4096" with the following non-sleepable locks held:
>>> > exclusive sleep mutex pcm0:play:0 (pcm play channel) r = 0 (0xc2be8a80)
>>> > locked _at_
>>> > /storage/os/src/freebsd-current/src/sys/dev/sound/pcm/buffer.c:195

> 
> Quick and dirty solutions:
> 	Switch back to M_NOWAIT even though we don't handle malloc()
>         failure gracefully.
> 
> 	Unlock the channel before calling the setblocksize method, but
> 	an interrupt happening in the middle of sndbuf_resize() would
>         not be a good thing.
> 
> 	Add a pointer back to the channel back to the snd_dbuf structure
> 	so that sndbuf_resize() could do the proper unlocking and
>         locking.

I picked door #3, which had some complications because sndbuf_resize()
was called in some places with the channel locked and some places with
the channel unlocked ...

Index: sys/dev/sound/pcm/buffer.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/sound/pcm/buffer.c,v
retrieving revision 1.21
diff -u -r1.21 buffer.c
--- sys/dev/sound/pcm/buffer.c	27 Nov 2003 19:51:44 -0000	1.21
+++ sys/dev/sound/pcm/buffer.c	26 Jan 2004 21:47:00 -0000
_at__at_ -31,13 +31,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 $");
 
 struct snd_dbuf *
-sndbuf_create(device_t dev, char *drv, char *desc)
+sndbuf_create(device_t dev, char *drv, char *desc, struct pcm_channel *channel)
 {
 	struct snd_dbuf *b;
 
 	b = malloc(sizeof(*b), M_DEVBUF, M_WAITOK | M_ZERO);
 	snprintf(b->name, SNDBUF_NAMELEN, "%s:%s", drv, desc);
 	b->dev = dev;
+	b->channel = channel;
 
 	return b;
 }
_at__at_ -113,27 +114,38 _at__at_
 int
 sndbuf_resize(struct snd_dbuf *b, unsigned int blkcnt, unsigned int blksz)
 {
-	u_int8_t *tmpbuf;
+	u_int8_t *tmpbuf, *f2;
+
+	chn_lock(b->channel);
 	if (b->maxsize == 0)
-		return 0;
+		goto out;
 	if (blkcnt == 0)
 		blkcnt = b->blkcnt;
 	if (blksz == 0)
 		blksz = b->blksz;
-	if (blkcnt < 2 || blksz < 16 || (blkcnt * blksz > b->maxsize))
+	if (blkcnt < 2 || blksz < 16 || (blkcnt * blksz > b->maxsize)) {
+		chn_unlock(b->channel);
 		return EINVAL;
+	}
 	if (blkcnt == b->blkcnt && blksz == b->blksz)
-		return 0;
-	b->blkcnt = blkcnt;
-	b->blksz = blksz;
-	b->bufsize = blkcnt * blksz;
+		goto out;
 
-	tmpbuf = malloc(b->bufsize, M_DEVBUF, M_NOWAIT);
+	chn_unlock(b->channel);
+	tmpbuf = malloc(b->bufsize, M_DEVBUF, M_WAITOK);
 	if (tmpbuf == NULL)
 		return ENOMEM;
-	free(b->tmpbuf, M_DEVBUF);
+	chn_lock(b->channel);
+	b->blkcnt = blkcnt;
+	b->blksz = blksz;
+	b->bufsize = blkcnt * blksz;
+	f2 =  b->tmpbuf;
 	b->tmpbuf = tmpbuf;
 	sndbuf_reset(b);
+	chn_unlock(b->channel);
+	free(f2, M_DEVBUF);
+	return 0;
+out:
+	chn_unlock(b->channel);
 	return 0;
 }
 
_at__at_ -142,21 +154,27 _at__at_
 {
         u_int8_t *buf, *tmpbuf, *f1, *f2;
         unsigned int bufsize;
+	int ret;
 
 	if (blkcnt < 2 || blksz < 16)
 		return EINVAL;
 
 	bufsize = blksz * blkcnt;
 
-	buf = malloc(bufsize, M_DEVBUF, M_NOWAIT);
-	if (buf == NULL)
-		return ENOMEM;
+	chn_unlock(b->channel);
+	buf = malloc(bufsize, M_DEVBUF, M_WAITOK);
+	if (buf == NULL) {
+		ret = ENOMEM;
+		goto out;
+	}
 
-	tmpbuf = malloc(bufsize, M_DEVBUF, M_NOWAIT);
+	tmpbuf = malloc(bufsize, M_DEVBUF, M_WAITOK);
    	if (tmpbuf == NULL) {
 		free(buf, M_DEVBUF);
-		return ENOMEM;
+		ret = ENOMEM;
+		goto out;
 	}
+	chn_lock(b->channel);
 
 	b->blkcnt = blkcnt;
 	b->blksz = blksz;
_at__at_ -167,13 +185,18 _at__at_
 	b->buf = buf;
 	b->tmpbuf = tmpbuf;
 
+	sndbuf_reset(b);
+
+	chn_unlock(b->channel);
       	if (f1)
 		free(f1, M_DEVBUF);
       	if (f2)
 		free(f2, M_DEVBUF);
 
-	sndbuf_reset(b);
-	return 0;
+	ret = 0;
+out:
+	chn_lock(b->channel);
+	return ret;
 }
 
 void
Index: sys/dev/sound/pcm/buffer.h
===================================================================
RCS file: /home/ncvs/src/sys/dev/sound/pcm/buffer.h,v
retrieving revision 1.8
diff -u -r1.8 buffer.h
--- sys/dev/sound/pcm/buffer.h	27 Nov 2003 19:51:44 -0000	1.8
+++ sys/dev/sound/pcm/buffer.h	26 Jan 2004 21:28:03 -0000
_at__at_ -53,10 +53,11 _at__at_
 	bus_dma_tag_t dmatag;
 	u_int32_t buf_addr;
 	struct selinfo sel;
+	struct pcm_channel *channel;
 	char name[SNDBUF_NAMELEN];
 };
 
-struct snd_dbuf *sndbuf_create(device_t dev, char *drv, char *desc);
+struct snd_dbuf *sndbuf_create(device_t dev, char *drv, char *desc, struct pcm_channel *channel);
 void sndbuf_destroy(struct snd_dbuf *b);
 
 void sndbuf_dump(struct snd_dbuf *b, char *s, u_int32_t what);
Index: sys/dev/sound/pcm/channel.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/sound/pcm/channel.c,v
retrieving revision 1.93
diff -u -r1.93 channel.c
--- sys/dev/sound/pcm/channel.c	5 Dec 2003 02:08:13 -0000	1.93
+++ sys/dev/sound/pcm/channel.c	26 Jan 2004 21:53:02 -0000
_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,6 +749,16 _at__at_
 	c->devinfo = NULL;
 	c->feeder = NULL;
 
+	ret = ENOMEM;
+	b = sndbuf_create(c->dev, c->name, "primary", c);
+	if (b == NULL)
+		goto out;
+	bs = sndbuf_create(c->dev, c->name, "secondary", c);
+	if (bs == NULL)
+		goto out;
+
+	CHN_LOCK(c);
+
 	ret = EINVAL;
 	fc = feeder_getclass(NULL);
 	if (fc == NULL)
_at__at_ -753,21 +766,23 _at__at_
 	if (chn_addfeeder(c, fc, NULL))
 		goto out;
 
-	ret = ENOMEM;
-	b = sndbuf_create(c->dev, c->name, "primary");
-	if (b == NULL)
-		goto out;
-	bs = sndbuf_create(c->dev, c->name, "secondary");
-	if (bs == NULL)
-		goto out;
+	/*
+	 * XXX - sndbuf_setup() & sndbuf_resize() expect to be called
+	 *	 with the channel unlocked because they are also called
+	 *	 from driver methods that don't know about locking
+	 */
+	CHN_UNLOCK(c);
 	sndbuf_setup(bs, NULL, 0);
+	CHN_LOCK(c);
 	c->bufhard = b;
 	c->bufsoft = bs;
 	c->flags = 0;
 	c->feederflags = 0;
 
 	ret = ENODEV;
+	CHN_UNLOCK(c); /* XXX - Unlock for CHANNEL_INIT() malloc() call */
 	c->devinfo = CHANNEL_INIT(c->methods, devinfo, b, c, dir);
+	CHN_LOCK(c);
 	if (c->devinfo == NULL)
 		goto out;
 
_at__at_ -789,6 +804,7 _at__at_
 
 
 out:
+	CHN_UNLOCK(c);
 	if (ret) {
 		if (c->devinfo) {
 			if (CHANNEL_FREE(c->methods, c->devinfo))
_at__at_ -971,11 +987,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 +1029,70 _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);
+		if (ret)
+			goto out1;
+	}
+
+	CHN_UNLOCK(c);
 	sndbuf_setblksz(b, CHANNEL_SETBLOCKSIZE(c->methods, c->devinfo, blksz));
+	CHN_LOCK(c);
+
+	/* Decrease the size of bufsoft after decreasing bufhard. */
+	maxsize = sndbuf_getsize(b);
+	if (reqblksz * blkcnt > maxsize)
+		maxsize = reqblksz * blkcnt;
+	if (maxsize > sndbuf_getsize(bs))
+		printf("Danger! %s bufsoft size increasing from %d to %d after CHANNEL_SETBLOCKSIZE()\n",
+		    c->name, sndbuf_getsize(bs), maxsize);
+	if (sndbuf_getsize(bs) != maxsize || sndbuf_getblksz(bs) != reqblksz) {
+		ret = sndbuf_remalloc(bs, maxsize/reqblksz, reqblksz);
+		if (ret)
+			goto out1;
+	}
 
 	irqhz = (sndbuf_getbps(b) * sndbuf_getspd(b)) / sndbuf_getblksz(b);
 	DEB(printf("got %d, irqhz == %d\n", sndbuf_getblksz(b), irqhz));
 
 	chn_resetbuf(c);
+out1:
+	KASSERT(sndbuf_getsize(bs) ==  0 ||
+	    sndbuf_getsize(bs) >= sndbuf_getsize(b),
+	    ("%s(%s): bufsoft size %d < bufhard size %d, reqblksz=%d blksz=%d maxsize=%d blkcnt=%d",
+	    __func__, c->name, sndbuf_getsize(bs), sndbuf_getsize(b), reqblksz,
+	    blksz, maxsize, blkcnt));
 out:
+	c->flags &= ~CHN_F_SETBLOCKSIZE;
 	return ret;
 }
 
_at__at_ -1216,8 +1272,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 +1313,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 +1330,28 _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;
+}
+
+void
+chn_lock(struct pcm_channel *c)
+{
+	CHN_LOCK(c);
+}
+
+void
+chn_unlock(struct pcm_channel *c)
+{
+	CHN_UNLOCK(c);
 }
Index: sys/dev/sound/pcm/channel.h
===================================================================
RCS file: /home/ncvs/src/sys/dev/sound/pcm/channel.h,v
retrieving revision 1.28
diff -u -r1.28 channel.h
--- sys/dev/sound/pcm/channel.h	7 Sep 2003 16:28:03 -0000	1.28
+++ sys/dev/sound/pcm/channel.h	26 Jan 2004 21:17:42 -0000
_at__at_ -99,11 +99,13 _at__at_
 void chn_rdupdate(struct pcm_channel *c);
 
 int chn_notify(struct pcm_channel *c, u_int32_t flags);
+void chn_lock(struct pcm_channel *c);
+void chn_unlock(struct pcm_channel *c);
 
 #ifdef	USING_MUTEX
 #define CHN_LOCK(c) mtx_lock((struct mtx *)((c)->lock))
 #define CHN_UNLOCK(c) mtx_unlock((struct mtx *)((c)->lock))
-#define CHN_LOCKASSERT(c)
+#define CHN_LOCKASSERT(c) mtx_assert((struct mtx *)((c)->lock), MA_OWNED)
 #else
 #define CHN_LOCK(c)
 #define CHN_UNLOCK(c)
_at__at_ -134,6 +136,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.71
diff -u -r1.71 dsp.c
--- sys/dev/sound/pcm/dsp.c	25 Jan 2004 22:46:22 -0000	1.71
+++ sys/dev/sound/pcm/dsp.c	26 Jan 2004 10:53:10 -0000
_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	26 Jan 2004 22:07:09 -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 Mon Jan 26 2004 - 14:05:58 UTC

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