git: 9263f854e9a6 - main - sound: Simplify channel creation and deletion process

From: Christos Margiolis <christos_at_FreeBSD.org>
Date: Fri, 18 Oct 2024 08:45:02 UTC
The branch main has been updated by christos:

URL: https://cgit.FreeBSD.org/src/commit/?id=9263f854e9a63cc326a3d5f6331b933c4a010abf

commit 9263f854e9a63cc326a3d5f6331b933c4a010abf
Author:     Christos Margiolis <christos@FreeBSD.org>
AuthorDate: 2024-10-18 08:40:41 +0000
Commit:     Christos Margiolis <christos@FreeBSD.org>
CommitDate: 2024-10-18 08:40:41 +0000

    sound: Simplify channel creation and deletion process
    
    Currently we create and destroy channels with the following consistent
    pattern:
    
    - chn_init() -> pcm_chn_add()
    - pcm_chn_remove() -> chn_kill()
    
    Instead of calling two separate functions, merge pcm_chn_add() with
    chn_init(), and pcm_chn_remove() with chn_kill().
    
    Another benefit of this change is that we avoid the confusion caused by
    having pcm_chn_add(), as well as pcm_addchan().
    
    Sponsored by:   The FreeBSD Foundation
    MFC after:      2 days
    Reviewed by:    dev_submerge.ch, markj
    Differential Revision:  https://reviews.freebsd.org/D46835
---
 sys/dev/sound/pcm/channel.c | 40 +++++++++++++++++++++++
 sys/dev/sound/pcm/sound.c   | 78 +--------------------------------------------
 sys/dev/sound/pcm/sound.h   |  3 --
 sys/dev/sound/pcm/vchan.c   | 25 +++------------
 4 files changed, 45 insertions(+), 101 deletions(-)

diff --git a/sys/dev/sound/pcm/channel.c b/sys/dev/sound/pcm/channel.c
index d1c9bc616dcf..5b0bb105c505 100644
--- a/sys/dev/sound/pcm/channel.c
+++ b/sys/dev/sound/pcm/channel.c
@@ -1311,6 +1311,24 @@ chn_init(struct snddev_info *d, struct pcm_channel *parent, kobj_class_t cls,
 	}
 
 	PCM_LOCK(d);
+	CHN_INSERT_SORT_ASCEND(d, c, channels.pcm);
+
+	switch (c->type) {
+	case SND_DEV_DSPHW_PLAY:
+		d->playcount++;
+		break;
+	case SND_DEV_DSPHW_VPLAY:
+		d->pvchancount++;
+		break;
+	case SND_DEV_DSPHW_REC:
+		d->reccount++;
+		break;
+	case SND_DEV_DSPHW_VREC:
+		d->rvchancount++;
+		break;
+	default:
+		__assert_unreachable();
+	}
 
 	return (c);
 
@@ -1337,11 +1355,33 @@ fail:
 void
 chn_kill(struct pcm_channel *c)
 {
+	struct snddev_info *d = c->parentsnddev;
 	struct snd_dbuf *b = c->bufhard;
 	struct snd_dbuf *bs = c->bufsoft;
 
 	PCM_BUSYASSERT(c->parentsnddev);
 
+	PCM_LOCK(d);
+	CHN_REMOVE(d, c, channels.pcm);
+
+	switch (c->type) {
+	case SND_DEV_DSPHW_PLAY:
+		d->playcount--;
+		break;
+	case SND_DEV_DSPHW_VPLAY:
+		d->pvchancount--;
+		break;
+	case SND_DEV_DSPHW_REC:
+		d->reccount--;
+		break;
+	case SND_DEV_DSPHW_VREC:
+		d->rvchancount--;
+		break;
+	default:
+		__assert_unreachable();
+	}
+	PCM_UNLOCK(d);
+
 	if (CHN_STARTED(c)) {
 		CHN_LOCK(c);
 		chn_trigger(c, PCMTRIG_ABORT);
diff --git a/sys/dev/sound/pcm/sound.c b/sys/dev/sound/pcm/sound.c
index da28a267c81a..e25cb359f793 100644
--- a/sys/dev/sound/pcm/sound.c
+++ b/sys/dev/sound/pcm/sound.c
@@ -188,74 +188,6 @@ SYSCTL_PROC(_hw_snd, OID_AUTO, default_unit,
     sizeof(int), sysctl_hw_snd_default_unit, "I",
     "default sound device");
 
-void
-pcm_chn_add(struct snddev_info *d, struct pcm_channel *ch)
-{
-	PCM_BUSYASSERT(d);
-	PCM_LOCKASSERT(d);
-	KASSERT(ch != NULL && (ch->direction == PCMDIR_PLAY ||
-	    ch->direction == PCMDIR_REC), ("Invalid pcm channel"));
-
-	CHN_INSERT_SORT_ASCEND(d, ch, channels.pcm);
-
-	switch (ch->type) {
-	case SND_DEV_DSPHW_PLAY:
-		d->playcount++;
-		break;
-	case SND_DEV_DSPHW_VPLAY:
-		d->pvchancount++;
-		break;
-	case SND_DEV_DSPHW_REC:
-		d->reccount++;
-		break;
-	case SND_DEV_DSPHW_VREC:
-		d->rvchancount++;
-		break;
-	default:
-		__assert_unreachable();
-	}
-}
-
-int
-pcm_chn_remove(struct snddev_info *d, struct pcm_channel *ch)
-{
-	struct pcm_channel *tmp;
-
-	PCM_BUSYASSERT(d);
-	PCM_LOCKASSERT(d);
-
-	tmp = NULL;
-
-	CHN_FOREACH(tmp, d, channels.pcm) {
-		if (tmp == ch)
-			break;
-	}
-
-	if (tmp != ch)
-		return (EINVAL);
-
-	CHN_REMOVE(d, ch, channels.pcm);
-
-	switch (ch->type) {
-	case SND_DEV_DSPHW_PLAY:
-		d->playcount--;
-		break;
-	case SND_DEV_DSPHW_VPLAY:
-		d->pvchancount--;
-		break;
-	case SND_DEV_DSPHW_REC:
-		d->reccount--;
-		break;
-	case SND_DEV_DSPHW_VREC:
-		d->rvchancount--;
-		break;
-	default:
-		__assert_unreachable();
-	}
-
-	return (0);
-}
-
 int
 pcm_addchan(device_t dev, int dir, kobj_class_t cls, void *devinfo)
 {
@@ -272,8 +204,6 @@ pcm_addchan(device_t dev, int dir, kobj_class_t cls, void *devinfo)
 		PCM_UNLOCK(d);
 		return (ENODEV);
 	}
-
-	pcm_chn_add(d, ch);
 	PCM_UNLOCK(d);
 
 	return (0);
@@ -283,7 +213,6 @@ static void
 pcm_killchans(struct snddev_info *d)
 {
 	struct pcm_channel *ch;
-	int error;
 	bool found;
 
 	PCM_BUSYASSERT(d);
@@ -316,12 +245,7 @@ pcm_killchans(struct snddev_info *d)
 			pause_sbt("pcmkillchans", SBT_1MS * 5, 0, 0);
 			continue;
 		}
-
-		PCM_LOCK(d);
-		error = pcm_chn_remove(d, ch);
-		PCM_UNLOCK(d);
-		if (error == 0)
-			chn_kill(ch);
+		chn_kill(ch);
 	} while (!CHN_EMPTY(d, channels.pcm));
 }
 
diff --git a/sys/dev/sound/pcm/sound.h b/sys/dev/sound/pcm/sound.h
index 51c9c91f6f20..f96638081cb9 100644
--- a/sys/dev/sound/pcm/sound.h
+++ b/sys/dev/sound/pcm/sound.h
@@ -248,9 +248,6 @@ SYSCTL_DECL(_hw_snd);
 int pcm_chnalloc(struct snddev_info *d, struct pcm_channel **ch, int direction,
     pid_t pid, char *comm);
 
-void pcm_chn_add(struct snddev_info *d, struct pcm_channel *ch);
-int pcm_chn_remove(struct snddev_info *d, struct pcm_channel *ch);
-
 int pcm_addchan(device_t dev, int dir, kobj_class_t cls, void *devinfo);
 unsigned int pcm_getbuffersize(device_t dev, unsigned int minbufsz, unsigned int deflt, unsigned int maxbufsz);
 int pcm_register(device_t dev, void *devinfo, int numplay, int numrec);
diff --git a/sys/dev/sound/pcm/vchan.c b/sys/dev/sound/pcm/vchan.c
index 8580eb679f35..4da6f83dc0a2 100644
--- a/sys/dev/sound/pcm/vchan.c
+++ b/sys/dev/sound/pcm/vchan.c
@@ -710,9 +710,6 @@ vchan_create(struct pcm_channel *parent)
 		CHN_LOCK(parent);
 		return (ENODEV);
 	}
-
-	/* add us to our grandparent's channel list */
-	pcm_chn_add(d, ch);
 	PCM_UNLOCK(d);
 
 	CHN_LOCK(parent);
@@ -818,12 +815,7 @@ fail:
 	CHN_REMOVE(parent, ch, children);
 	parent->flags &= ~CHN_F_HAS_VCHAN;
 	CHN_UNLOCK(parent);
-	PCM_LOCK(d);
-	if (pcm_chn_remove(d, ch) == 0) {
-		PCM_UNLOCK(d);
-		chn_kill(ch);
-	} else
-		PCM_UNLOCK(d);
+	chn_kill(ch);
 	CHN_LOCK(parent);
 
 	return (ret);
@@ -833,8 +825,6 @@ int
 vchan_destroy(struct pcm_channel *c)
 {
 	struct pcm_channel *parent;
-	struct snddev_info *d;
-	int ret;
 
 	KASSERT(c != NULL && c->parentchannel != NULL &&
 	    c->parentsnddev != NULL, ("%s(): invalid channel=%p",
@@ -842,10 +832,9 @@ vchan_destroy(struct pcm_channel *c)
 
 	CHN_LOCKASSERT(c);
 
-	d = c->parentsnddev;
 	parent = c->parentchannel;
 
-	PCM_BUSYASSERT(d);
+	PCM_BUSYASSERT(c->parentsnddev);
 	CHN_LOCKASSERT(parent);
 
 	CHN_UNLOCK(c);
@@ -866,18 +855,12 @@ vchan_destroy(struct pcm_channel *c)
 
 	CHN_UNLOCK(parent);
 
-	/* remove us from our grandparent's channel list */
-	PCM_LOCK(d);
-	ret = pcm_chn_remove(d, c);
-	PCM_UNLOCK(d);
-
 	/* destroy ourselves */
-	if (ret == 0)
-		chn_kill(c);
+	chn_kill(c);
 
 	CHN_LOCK(parent);
 
-	return (ret);
+	return (0);
 }
 
 int