Skip site navigation (1)Skip section navigation (2) 
Date:      Sun, 20 Oct 2024 11:21:30 GMT
From:      Christos Margiolis <christos@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: 4cf4144d8f7f - stable/14 - sound: Simplify pcm_chnalloc() and fix infinite loop bug
Message-ID:  <202410201121.49KBLUJD061447@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
The branch stable/14 has been updated by christos:

URL: https://cgit.FreeBSD.org/src/commit/?id=4cf4144d8f7fdd51562b45fa8f1f4aa16d444b3a

commit 4cf4144d8f7fdd51562b45fa8f1f4aa16d444b3a
Author:     Christos Margiolis <christos@FreeBSD.org>
AuthorDate: 2024-10-18 08:38:50 +0000
Commit:     Christos Margiolis <christos@FreeBSD.org>
CommitDate: 2024-10-20 11:21:04 +0000

    sound: Simplify pcm_chnalloc() and fix infinite loop bug
    
    Simplify logic to execute the following algorithm:
    
    1. Search for a free (i.e not busy) channel and return successfully.
    2. If no channel was found, either return an ENOTSUP if no VCHAN can be
       created (disabled or we reached the limit), or create one more VCHAN
       and scan for it again.
    3. If the second scan failed again, return EBUSY.
    
    This patch also solves a bug where we'd end up in an infinite loop,
    calling vchan_setnew() with the same `newcnt` value indefinitely, after
    the following scenario:
    
    1. Plug device.
    2. Spawn X channels.
    3. Kill all channels except _last_ opened.
    4. Clean up all unused VCHANs.
    5. Spawn X+1 channels.
    6. Infinite loop in pcm_chnalloc().
    
    I am not exactly sure which part of pcm_chnalloc() caused this bug, but
    the patch fixes it at least...
    
    Sponsored by:   The FreeBSD Foundation
    MFC after:      2 days
    Reviewed by:    dev_submerge.ch
    Differential Revision:  https://reviews.freebsd.org/D46548
    
    (cherry picked from commit b973a14d354f332a3428cbab8e9d4b72cb3e2d6e)
---
 sys/dev/sound/pcm/sound.c | 39 +++++++++++++++------------------------
 1 file changed, 15 insertions(+), 24 deletions(-)

diff --git a/sys/dev/sound/pcm/sound.c b/sys/dev/sound/pcm/sound.c
index b18b83468150..1aaf614078a4 100644
--- a/sys/dev/sound/pcm/sound.c
+++ b/sys/dev/sound/pcm/sound.c
@@ -109,13 +109,12 @@ snd_setup_intr(device_t dev, struct resource *res, int flags, driver_intr_t hand
 	return bus_setup_intr(dev, res, flags, NULL, hand, param, cookiep);
 }
 
-/* return error status and a locked channel */
 int
 pcm_chnalloc(struct snddev_info *d, struct pcm_channel **ch, int direction,
     pid_t pid, char *comm)
 {
 	struct pcm_channel *c;
-	int err, vchancount, vchan_num;
+	int err, vchancount;
 	bool retry;
 
 	KASSERT(d != NULL && ch != NULL &&
@@ -125,46 +124,38 @@ pcm_chnalloc(struct snddev_info *d, struct pcm_channel **ch, int direction,
 	PCM_BUSYASSERT(d);
 
 	*ch = NULL;
-	vchan_num = 0;
 	vchancount = (direction == PCMDIR_PLAY) ? d->pvchancount :
 	    d->rvchancount;
-
 	retry = false;
+
 retry_chnalloc:
-	err = ENOTSUP;
-	/* scan for a free channel */
+	/* Scan for a free channel. */
 	CHN_FOREACH(c, d, channels.pcm) {
 		CHN_LOCK(c);
-		if (c->direction == direction && (c->flags & CHN_F_VIRTUAL)) {
-			if (vchancount < snd_maxautovchans &&
-			    vchan_num < c->unit) {
-			    	CHN_UNLOCK(c);
-				goto vchan_alloc;
-			}
-			vchan_num++;
+		if (c->direction != direction) {
+			CHN_UNLOCK(c);
+			continue;
 		}
-		if (c->direction == direction && !(c->flags & CHN_F_BUSY)) {
+		if (!(c->flags & CHN_F_BUSY)) {
 			c->flags |= CHN_F_BUSY;
 			c->pid = pid;
 			strlcpy(c->comm, (comm != NULL) ? comm :
 			    CHN_COMM_UNKNOWN, sizeof(c->comm));
 			*ch = c;
+
 			return (0);
-		} else if (c->direction == direction && (c->flags & CHN_F_BUSY))
-			err = EBUSY;
+		}
 		CHN_UNLOCK(c);
 	}
-
-	/*
-	 * We came from retry_chnalloc and still didn't find a free channel.
-	 */
+	/* Maybe next time... */
 	if (retry)
-		return (err);
+		return (EBUSY);
 
-vchan_alloc:
-	/* no channel available */
+	/* No channel available. We also cannot create more VCHANs. */
 	if (!(vchancount > 0 && vchancount < snd_maxautovchans))
-		return (err);
+		return (ENOTSUP);
+
+	/* Increase the VCHAN count and try to get the new channel. */
 	err = vchan_setnew(d, direction, vchancount + 1);
 	if (err == 0) {
 		retry = true;

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202410201121.49KBLUJD061447>