git: 5bd08172b415 - main - snd_dummy: Fix callout(9) races

From: Christos Margiolis <christos_at_FreeBSD.org>
Date: Tue, 26 Nov 2024 14:48:53 UTC
The branch main has been updated by christos:

URL: https://cgit.FreeBSD.org/src/commit/?id=5bd08172b4150503c9cf60ffe3c97716c5bf6fa1

commit 5bd08172b4150503c9cf60ffe3c97716c5bf6fa1
Author:     Christos Margiolis <christos@FreeBSD.org>
AuthorDate: 2024-11-26 14:48:02 +0000
Commit:     Christos Margiolis <christos@FreeBSD.org>
CommitDate: 2024-11-26 14:48:02 +0000

    snd_dummy: Fix callout(9) races
    
    Use callout_init_mtx(9) to associate the callback with the driver's
    lock. Also make sure the callout is stopped properly during detach.
    
    While here, introduce a dummy_active() function to know when it's
    appropriate to stop or not reschedule the callout.
    
    Sponsored by:   The FreeBSD Foundation
    MFC after:      2 days
    Reviewed by:    dev_submerge.ch, markj
    Differential Revision:  https://reviews.freebsd.org/D47459
---
 sys/dev/sound/dummy.c | 34 ++++++++++++++++++++++++++--------
 1 file changed, 26 insertions(+), 8 deletions(-)

diff --git a/sys/dev/sound/dummy.c b/sys/dev/sound/dummy.c
index 9013fd023a7e..b637171d82e4 100644
--- a/sys/dev/sound/dummy.c
+++ b/sys/dev/sound/dummy.c
@@ -67,6 +67,24 @@ struct dummy_softc {
 	struct mtx *lock;
 };
 
+static bool
+dummy_active(struct dummy_softc *sc)
+{
+	struct dummy_chan *ch;
+	int i;
+
+	snd_mtxassert(sc->lock);
+
+	for (i = 0; i < sc->chnum; i++) {
+		ch = &sc->chans[i];
+		if (ch->run)
+			return (true);
+	}
+
+	/* No channel is running at the moment. */
+	return (false);
+}
+
 static void
 dummy_chan_io(void *arg)
 {
@@ -74,7 +92,9 @@ dummy_chan_io(void *arg)
 	struct dummy_chan *ch;
 	int i = 0;
 
-	snd_mtxlock(sc->lock);
+	/* Do not reschedule if no channel is running. */
+	if (!dummy_active(sc))
+		return;
 
 	for (i = 0; i < sc->chnum; i++) {
 		ch = &sc->chans[i];
@@ -89,8 +109,6 @@ dummy_chan_io(void *arg)
 		snd_mtxlock(sc->lock);
 	}
 	callout_schedule(&sc->callout, 1);
-
-	snd_mtxunlock(sc->lock);
 }
 
 static int
@@ -179,15 +197,15 @@ dummy_chan_trigger(kobj_t obj, void *data, int go)
 
 	switch (go) {
 	case PCMTRIG_START:
-		if (!callout_active(&sc->callout))
-			callout_reset(&sc->callout, 1, dummy_chan_io, sc);
 		ch->ptr = 0;
 		ch->run = 1;
+		callout_reset(&sc->callout, 1, dummy_chan_io, sc);
 		break;
 	case PCMTRIG_STOP:
 	case PCMTRIG_ABORT:
 		ch->run = 0;
-		if (callout_active(&sc->callout))
+		/* If all channels are stopped, stop the callout as well. */
+		if (!dummy_active(sc))
 			callout_stop(&sc->callout);
 	default:
 		break;
@@ -292,6 +310,7 @@ dummy_attach(device_t dev)
 	sc = device_get_softc(dev);
 	sc->dev = dev;
 	sc->lock = snd_mtxcreate(device_get_nameunit(dev), "snd_dummy softc");
+	callout_init_mtx(&sc->callout, sc->lock, 0);
 
 	sc->cap_fmts[0] = SND_FORMAT(AFMT_S32_LE, 2, 0);
 	sc->cap_fmts[1] = SND_FORMAT(AFMT_S24_LE, 2, 0);
@@ -316,7 +335,6 @@ dummy_attach(device_t dev)
 	if (pcm_register(dev, status))
 		return (ENXIO);
 	mixer_init(dev, &dummy_mixer_class, sc);
-	callout_init(&sc->callout, 1);
 
 	return (0);
 }
@@ -327,8 +345,8 @@ dummy_detach(device_t dev)
 	struct dummy_softc *sc = device_get_softc(dev);
 	int err;
 
-	callout_drain(&sc->callout);
 	err = pcm_unregister(dev);
+	callout_drain(&sc->callout);
 	snd_mtxfree(sc->lock);
 
 	return (err);