git: 35400672df83 - main - sound: Use bus_topo_lock() where appropriate

From: Christos Margiolis <christos_at_FreeBSD.org>
Date: Sun, 30 Mar 2025 17:48:29 UTC
The branch main has been updated by christos:

URL: https://cgit.FreeBSD.org/src/commit/?id=35400672df83e337f8792df1972a15003b603930

commit 35400672df83e337f8792df1972a15003b603930
Author:     Christos Margiolis <christos@FreeBSD.org>
AuthorDate: 2025-03-30 17:46:14 +0000
Commit:     Christos Margiolis <christos@FreeBSD.org>
CommitDate: 2025-03-30 17:46:14 +0000

    sound: Use bus_topo_lock() where appropriate
    
    Lock around uses of devclass_*() and replace leftover
    CTLFLAG_NEEDGIANTs with CTLFLAG_MPSAFE.
    
    Sponsored by:   The FreeBSD Foundation
    MFC after:      1 week
    Reviewed by:    imp, jhb
    Differential Revision:  https://reviews.freebsd.org/D46700
---
 sys/dev/sound/pcm/channel.c     |  6 ++++--
 sys/dev/sound/pcm/dsp.c         |  6 ++++++
 sys/dev/sound/pcm/feeder_rate.c |  4 +++-
 sys/dev/sound/pcm/mixer.c       |  5 +++++
 sys/dev/sound/pcm/sound.c       | 19 +++++++++++++++++--
 sys/dev/sound/pcm/vchan.c       | 28 +++++++++++++++++++++++-----
 6 files changed, 58 insertions(+), 10 deletions(-)

diff --git a/sys/dev/sound/pcm/channel.c b/sys/dev/sound/pcm/channel.c
index 31a56a8b82e2..4d13f20a5262 100644
--- a/sys/dev/sound/pcm/channel.c
+++ b/sys/dev/sound/pcm/channel.c
@@ -132,6 +132,7 @@ chn_vpc_proc(int reset, int db)
 	struct pcm_channel *c;
 	int i;
 
+	bus_topo_lock();
 	for (i = 0; pcm_devclass != NULL &&
 	    i < devclass_get_maxunit(pcm_devclass); i++) {
 		d = devclass_get_softc(pcm_devclass, i);
@@ -150,6 +151,7 @@ chn_vpc_proc(int reset, int db)
 		PCM_RELEASE(d);
 		PCM_UNLOCK(d);
 	}
+	bus_topo_unlock();
 }
 
 static int
@@ -170,7 +172,7 @@ sysctl_hw_snd_vpc_0db(SYSCTL_HANDLER_ARGS)
 	return (0);
 }
 SYSCTL_PROC(_hw_snd, OID_AUTO, vpc_0db,
-    CTLTYPE_INT | CTLFLAG_RWTUN | CTLFLAG_NEEDGIANT, 0, sizeof(int),
+    CTLTYPE_INT | CTLFLAG_RWTUN | CTLFLAG_MPSAFE, 0, sizeof(int),
     sysctl_hw_snd_vpc_0db, "I",
     "0db relative level");
 
@@ -190,7 +192,7 @@ sysctl_hw_snd_vpc_reset(SYSCTL_HANDLER_ARGS)
 	return (0);
 }
 SYSCTL_PROC(_hw_snd, OID_AUTO, vpc_reset,
-    CTLTYPE_INT | CTLFLAG_RW | CTLFLAG_NEEDGIANT, 0, sizeof(int),
+    CTLTYPE_INT | CTLFLAG_RW | CTLFLAG_MPSAFE, 0, sizeof(int),
     sysctl_hw_snd_vpc_reset, "I",
     "reset volume on all channels");
 
diff --git a/sys/dev/sound/pcm/dsp.c b/sys/dev/sound/pcm/dsp.c
index 422c64c1b880..c5caeea8a002 100644
--- a/sys/dev/sound/pcm/dsp.c
+++ b/sys/dev/sound/pcm/dsp.c
@@ -2016,6 +2016,7 @@ dsp_oss_audioinfo(struct cdev *i_dev, oss_audioinfo *ai, bool ex)
 	if (ai->dev == -1 && i_dev->si_devsw != &dsp_cdevsw)
 		return (EINVAL);
 
+	bus_topo_lock();
 	for (unit = 0; pcm_devclass != NULL &&
 	    unit < devclass_get_maxunit(pcm_devclass); unit++) {
 		d = devclass_get_softc(pcm_devclass, unit);
@@ -2023,6 +2024,7 @@ dsp_oss_audioinfo(struct cdev *i_dev, oss_audioinfo *ai, bool ex)
 			if ((ai->dev == -1 && unit == snd_unit) ||
 			    ai->dev == unit) {
 				dsp_oss_audioinfo_unavail(ai, unit);
+				bus_topo_unlock();
 				return (0);
 			} else {
 				d = NULL;
@@ -2041,6 +2043,7 @@ dsp_oss_audioinfo(struct cdev *i_dev, oss_audioinfo *ai, bool ex)
 			d = NULL;
 		}
 	}
+	bus_topo_unlock();
 
 	/* Exhausted the search -- nothing is locked, so return. */
 	if (d == NULL)
@@ -2197,6 +2200,7 @@ dsp_oss_engineinfo(struct cdev *i_dev, oss_audioinfo *ai)
 	 * Search for the requested audio device (channel).  Start by
 	 * iterating over pcm devices.
 	 */ 
+	bus_topo_lock();
 	for (unit = 0; pcm_devclass != NULL &&
 	    unit < devclass_get_maxunit(pcm_devclass); unit++) {
 		d = devclass_get_softc(pcm_devclass, unit);
@@ -2346,9 +2350,11 @@ dsp_oss_engineinfo(struct cdev *i_dev, oss_audioinfo *ai)
 
 		CHN_UNLOCK(ch);
 		PCM_UNLOCK(d);
+		bus_topo_unlock();
 
 		return (0);
 	}
+	bus_topo_unlock();
 
 	/* Exhausted the search -- nothing is locked, so return. */
 	return (EINVAL);
diff --git a/sys/dev/sound/pcm/feeder_rate.c b/sys/dev/sound/pcm/feeder_rate.c
index 9ea454cdee1e..9c29142b9d6b 100644
--- a/sys/dev/sound/pcm/feeder_rate.c
+++ b/sys/dev/sound/pcm/feeder_rate.c
@@ -258,6 +258,7 @@ sysctl_hw_snd_feeder_rate_quality(SYSCTL_HANDLER_ARGS)
 	 * set resampler quality if and only if it is exist as
 	 * part of feeder chains and the channel is idle.
 	 */
+	bus_topo_lock();
 	for (i = 0; pcm_devclass != NULL &&
 	    i < devclass_get_maxunit(pcm_devclass); i++) {
 		d = devclass_get_softc(pcm_devclass, i);
@@ -279,11 +280,12 @@ sysctl_hw_snd_feeder_rate_quality(SYSCTL_HANDLER_ARGS)
 		PCM_RELEASE(d);
 		PCM_UNLOCK(d);
 	}
+	bus_topo_unlock();
 
 	return (0);
 }
 SYSCTL_PROC(_hw_snd, OID_AUTO, feeder_rate_quality,
-    CTLTYPE_INT | CTLFLAG_RWTUN | CTLFLAG_NEEDGIANT, 0, sizeof(int),
+    CTLTYPE_INT | CTLFLAG_RWTUN | CTLFLAG_MPSAFE, 0, sizeof(int),
     sysctl_hw_snd_feeder_rate_quality, "I",
     "sample rate converter quality ("__XSTRING(Z_QUALITY_MIN)"=low .. "
     __XSTRING(Z_QUALITY_MAX)"=high)");
diff --git a/sys/dev/sound/pcm/mixer.c b/sys/dev/sound/pcm/mixer.c
index 7bd0a2e14c46..092af3298f0e 100644
--- a/sys/dev/sound/pcm/mixer.c
+++ b/sys/dev/sound/pcm/mixer.c
@@ -1444,12 +1444,14 @@ mixer_oss_mixerinfo(struct cdev *i_dev, oss_mixerinfo *mi)
 	 * There's a 1:1 relationship between mixers and PCM devices, so
 	 * begin by iterating over PCM devices and search for our mixer.
 	 */
+	bus_topo_lock();
 	for (i = 0; pcm_devclass != NULL &&
 	    i < devclass_get_maxunit(pcm_devclass); i++) {
 		d = devclass_get_softc(pcm_devclass, i);
 		if (!PCM_REGISTERED(d)) {
 			if ((mi->dev == -1 && i == snd_unit) || mi->dev == i) {
 				mixer_oss_mixerinfo_unavail(mi, i);
+				bus_topo_unlock();
 				return (0);
 			} else
 				continue;
@@ -1470,6 +1472,7 @@ mixer_oss_mixerinfo(struct cdev *i_dev, oss_mixerinfo *mi)
 		if (d->mixer_dev->si_drv1 == NULL) {
 			mixer_oss_mixerinfo_unavail(mi, i);
 			PCM_UNLOCK(d);
+			bus_topo_unlock();
 			return (0);
 		}
 
@@ -1550,8 +1553,10 @@ mixer_oss_mixerinfo(struct cdev *i_dev, oss_mixerinfo *mi)
 
 		PCM_UNLOCK(d);
 
+		bus_topo_unlock();
 		return (0);
 	}
+	bus_topo_unlock();
 
 	return (EINVAL);
 }
diff --git a/sys/dev/sound/pcm/sound.c b/sys/dev/sound/pcm/sound.c
index 3bd6087b54da..f6c35769e7f3 100644
--- a/sys/dev/sound/pcm/sound.c
+++ b/sys/dev/sound/pcm/sound.c
@@ -116,17 +116,21 @@ sysctl_hw_snd_default_unit(SYSCTL_HANDLER_ARGS)
 	unit = snd_unit;
 	error = sysctl_handle_int(oidp, &unit, 0, req);
 	if (error == 0 && req->newptr != NULL) {
+		bus_topo_lock();
 		d = devclass_get_softc(pcm_devclass, unit);
-		if (!PCM_REGISTERED(d) || CHN_EMPTY(d, channels.pcm))
+		if (!PCM_REGISTERED(d) || CHN_EMPTY(d, channels.pcm)) {
+			bus_topo_unlock();
 			return EINVAL;
+		}
 		snd_unit = unit;
 		snd_unit_auto = 0;
+		bus_topo_unlock();
 	}
 	return (error);
 }
 /* XXX: do we need a way to let the user change the default unit? */
 SYSCTL_PROC(_hw_snd, OID_AUTO, default_unit,
-    CTLTYPE_INT | CTLFLAG_RWTUN | CTLFLAG_ANYBODY | CTLFLAG_NEEDGIANT, 0,
+    CTLTYPE_INT | CTLFLAG_RWTUN | CTLFLAG_ANYBODY | CTLFLAG_MPSAFE, 0,
     sizeof(int), sysctl_hw_snd_default_unit, "I",
     "default sound device");
 
@@ -213,6 +217,7 @@ pcm_best_unit(int old)
 
 	best = -1;
 	bestprio = -100;
+	bus_topo_lock();
 	for (i = 0; pcm_devclass != NULL &&
 	    i < devclass_get_maxunit(pcm_devclass); i++) {
 		d = devclass_get_softc(pcm_devclass, i);
@@ -228,6 +233,8 @@ pcm_best_unit(int old)
 			bestprio = prio;
 		}
 	}
+	bus_topo_unlock();
+
 	return (best);
 }
 
@@ -557,6 +564,7 @@ sound_oss_sysinfo(oss_sysinfo *si)
 
 	j = 0;
 
+	bus_topo_lock();
 	for (i = 0; pcm_devclass != NULL &&
 	    i < devclass_get_maxunit(pcm_devclass); i++) {
 		d = devclass_get_softc(pcm_devclass, i);
@@ -583,6 +591,7 @@ sound_oss_sysinfo(oss_sysinfo *si)
 
 		PCM_UNLOCK(d);
 	}
+	bus_topo_unlock();
 
 	si->numsynths = 0;	/* OSSv4 docs:  this field is obsolete */
 	/**
@@ -603,9 +612,11 @@ sound_oss_sysinfo(oss_sysinfo *si)
 	 * break if they try to loop through all mixers and some of them are
 	 * not available.
 	 */
+	bus_topo_lock();
 	si->nummixers = devclass_get_maxunit(pcm_devclass);
 	si->numcards = devclass_get_maxunit(pcm_devclass);
 	si->numaudios = devclass_get_maxunit(pcm_devclass);
+	bus_topo_unlock();
 		/* OSSv4 docs:	Intended only for test apps; API doesn't
 		   really have much of a concept of cards.  Shouldn't be
 		   used by applications. */
@@ -631,6 +642,7 @@ sound_oss_card_info(oss_card_info *si)
 	struct snddev_info *d;
 	int i;
 
+	bus_topo_lock();
 	for (i = 0; pcm_devclass != NULL &&
 	    i < devclass_get_maxunit(pcm_devclass); i++) {
 		d = devclass_get_softc(pcm_devclass, i);
@@ -658,8 +670,11 @@ sound_oss_card_info(oss_card_info *si)
 			PCM_UNLOCK(d);
 		}
 
+		bus_topo_unlock();
 		return (0);
 	}
+	bus_topo_unlock();
+
 	return (ENXIO);
 }
 
diff --git a/sys/dev/sound/pcm/vchan.c b/sys/dev/sound/pcm/vchan.c
index 1f184f21807e..31a4f7db8d70 100644
--- a/sys/dev/sound/pcm/vchan.c
+++ b/sys/dev/sound/pcm/vchan.c
@@ -259,9 +259,13 @@ sysctl_dev_pcm_vchans(SYSCTL_HANDLER_ARGS)
 	struct snddev_info *d;
 	int err, enabled, flag;
 
+	bus_topo_lock();
 	d = devclass_get_softc(pcm_devclass, VCHAN_SYSCTL_UNIT(oidp->oid_arg1));
-	if (!PCM_REGISTERED(d))
+	if (!PCM_REGISTERED(d)) {
+		bus_topo_unlock();
 		return (EINVAL);
+	}
+	bus_topo_unlock();
 
 	PCM_LOCK(d);
 	PCM_WAIT(d);
@@ -317,9 +321,13 @@ sysctl_dev_pcm_vchanmode(SYSCTL_HANDLER_ARGS)
 	int *vchanmode, direction, ret;
 	char dtype[16];
 
+	bus_topo_lock();
 	d = devclass_get_softc(pcm_devclass, VCHAN_SYSCTL_UNIT(oidp->oid_arg1));
-	if (!PCM_REGISTERED(d))
+	if (!PCM_REGISTERED(d)) {
+		bus_topo_unlock();
 		return (EINVAL);
+	}
+	bus_topo_unlock();
 
 	PCM_LOCK(d);
 	PCM_WAIT(d);
@@ -407,9 +415,13 @@ sysctl_dev_pcm_vchanrate(SYSCTL_HANDLER_ARGS)
 	struct pcm_channel *c, *ch;
 	int *vchanrate, direction, ret, newspd, restart;
 
+	bus_topo_lock();
 	d = devclass_get_softc(pcm_devclass, VCHAN_SYSCTL_UNIT(oidp->oid_arg1));
-	if (!PCM_REGISTERED(d))
+	if (!PCM_REGISTERED(d)) {
+		bus_topo_unlock();
 		return (EINVAL);
+	}
+	bus_topo_unlock();
 
 	PCM_LOCK(d);
 	PCM_WAIT(d);
@@ -499,9 +511,13 @@ sysctl_dev_pcm_vchanformat(SYSCTL_HANDLER_ARGS)
 	int *vchanformat, direction, ret, restart;
 	char fmtstr[AFMTSTR_LEN];
 
+	bus_topo_lock();
 	d = devclass_get_softc(pcm_devclass, VCHAN_SYSCTL_UNIT(oidp->oid_arg1));
-	if (!PCM_REGISTERED(d))
+	if (!PCM_REGISTERED(d)) {
+		bus_topo_unlock();
 		return (EINVAL);
+	}
+	bus_topo_unlock();
 
 	PCM_LOCK(d);
 	PCM_WAIT(d);
@@ -749,6 +765,7 @@ sysctl_hw_snd_vchans_enable(SYSCTL_HANDLER_ARGS)
 	if (error != 0 || req->newptr == NULL)
 		return (error);
 
+	bus_topo_lock();
 	snd_vchans_enable = v >= 1;
 
 	for (i = 0; pcm_devclass != NULL &&
@@ -766,11 +783,12 @@ sysctl_hw_snd_vchans_enable(SYSCTL_HANDLER_ARGS)
 			d->flags &= ~(SD_F_PVCHANS | SD_F_RVCHANS);
 		PCM_RELEASE_QUICK(d);
 	}
+	bus_topo_unlock();
 
 	return (0);
 }
 SYSCTL_PROC(_hw_snd, OID_AUTO, vchans_enable,
-    CTLTYPE_INT | CTLFLAG_RWTUN | CTLFLAG_NEEDGIANT, 0, sizeof(int),
+    CTLTYPE_INT | CTLFLAG_RWTUN | CTLFLAG_MPSAFE, 0, sizeof(int),
     sysctl_hw_snd_vchans_enable, "I", "global virtual channel switch");
 
 void