git: d692c314d29a - stable/14 - sound: Implement asynchronous device detach

From: Christos Margiolis <christos_at_FreeBSD.org>
Date: Thu, 18 Apr 2024 12:39:57 UTC
The branch stable/14 has been updated by christos:

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

commit d692c314d29a310efe875e9be05b0ccebe6b241d
Author:     Christos Margiolis <christos@FreeBSD.org>
AuthorDate: 2024-04-11 18:06:50 +0000
Commit:     Christos Margiolis <christos@FreeBSD.org>
CommitDate: 2024-04-18 12:39:31 +0000

    sound: Implement asynchronous device detach
    
    Hot-unplugging a sound device, such as a USB sound card, whilst being
    consumed by an application, results in an infinite loop until either the
    application closes the device's file descriptor, or the channel
    automatically times out after hw.snd.timeout seconds. In the case of a
    detach however, the timeout approach is still not ideal, since we want
    all resources to be released immediatelly, without waiting for N seconds
    until we can use the bus again.
    
    The timeout mechanism works by calling chn_sleep() in chn_read() and
    chn_write() (see pcm/channel.c) in order to send the thread to sleep,
    using cv_timedwait_sig(). Since chn_sleep() sets the CHN_F_SLEEPING flag
    while waiting for cv_timedwait_sig() to return, we can test this flag in
    pcm_unregister() (called during detach) and wakeup the sleeping
    thread(s) to immediately kill the channel(s) being consumed.
    
    Sponsored by:   The FreeBSD Foundation
    MFC after:      2 months
    PR:             194727
    Reviewed by:    dev_submerge.ch, bapt, markj
    Differential Revision:  https://reviews.freebsd.org/D43545
    
    (cherry picked from commit 44e128fe9d92c1a544b801cb56e907a66ef34691)
---
 share/man/man4/snd_uaudio.4 | 11 +----------
 sys/dev/sound/pcm/dsp.c     |  2 +-
 sys/dev/sound/pcm/mixer.c   | 11 -----------
 sys/dev/sound/pcm/sound.c   | 24 ++++++++++--------------
 sys/dev/sound/usb/uaudio.c  | 13 +++----------
 5 files changed, 15 insertions(+), 46 deletions(-)

diff --git a/share/man/man4/snd_uaudio.4 b/share/man/man4/snd_uaudio.4
index b6a6c06a2312..6e2509b8f2ac 100644
--- a/share/man/man4/snd_uaudio.4
+++ b/share/man/man4/snd_uaudio.4
@@ -27,7 +27,7 @@
 .\" ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 .\" POSSIBILITY OF SUCH DAMAGE.
 .\"
-.Dd January 29, 2024
+.Dd March 26, 2024
 .Dt SND_UAUDIO 4
 .Os
 .Sh NAME
@@ -156,15 +156,6 @@ and modified for
 by
 .An Hiten Pandya Aq Mt hmp@FreeBSD.org .
 .Sh BUGS
-The PCM framework in
-.Fx
-only supports synchronous device detach.
-That means all mixer and DSP character devices belonging to a given
-USB audio device must be closed when receiving an error on a DSP read,
-a DSP write or a DSP IOCTL request.
-Else the USB audio driver will wait for this to happen, preventing
-enumeration of new devices on the parenting USB controller.
-.Pp
 Some USB audio devices might refuse to work properly unless the sample
 rate is configured the same for both recording and playback, even if
 only simplex is used.
diff --git a/sys/dev/sound/pcm/dsp.c b/sys/dev/sound/pcm/dsp.c
index 003e57446a69..4112e87ba527 100644
--- a/sys/dev/sound/pcm/dsp.c
+++ b/sys/dev/sound/pcm/dsp.c
@@ -274,7 +274,7 @@ dsp_close(void *data)
 
 	d = priv->sc;
 	/* At this point pcm_unregister() will destroy all channels anyway. */
-	if (!PCM_REGISTERED(d))
+	if (PCM_DETACHING(d))
 		goto skip;
 
 	PCM_GIANT_ENTER(d);
diff --git a/sys/dev/sound/pcm/mixer.c b/sys/dev/sound/pcm/mixer.c
index ee1ed11a8ed0..cc8cf5b1ceea 100644
--- a/sys/dev/sound/pcm/mixer.c
+++ b/sys/dev/sound/pcm/mixer.c
@@ -817,17 +817,6 @@ mixer_uninit(device_t dev)
 	KASSERT(m->type == MIXER_TYPE_PRIMARY,
 	    ("%s(): illegal mixer type=%d", __func__, m->type));
 
-	snd_mtxlock(m->lock);
-
-	if (m->busy) {
-		snd_mtxunlock(m->lock);
-		return EBUSY;
-	}
-
-	/* destroy dev can sleep --hps */
-
-	snd_mtxunlock(m->lock);
-
 	pdev->si_drv1 = NULL;
 	destroy_dev(pdev);
 
diff --git a/sys/dev/sound/pcm/sound.c b/sys/dev/sound/pcm/sound.c
index 53cde49c4905..1517126210c4 100644
--- a/sys/dev/sound/pcm/sound.c
+++ b/sys/dev/sound/pcm/sound.c
@@ -1001,26 +1001,22 @@ pcm_unregister(device_t dev)
 
 	CHN_FOREACH(ch, d, channels.pcm) {
 		CHN_LOCK(ch);
-		if (ch->refcount > 0) {
-			device_printf(dev,
-			    "unregister: channel %s busy (pid %d)\n",
-			    ch->name, ch->pid);
-			CHN_UNLOCK(ch);
-			PCM_RELEASE_QUICK(d);
-			return (EBUSY);
+		if (ch->flags & CHN_F_SLEEPING) {
+			/*
+			 * We are detaching, so do not wait for the timeout in
+			 * chn_read()/chn_write(). Wake up the thread and kill
+			 * the channel immediately.
+			 */
+			CHN_BROADCAST(&ch->intr_cv);
+			ch->flags |= CHN_F_DEAD;
 		}
+		chn_abort(ch);
 		CHN_UNLOCK(ch);
 	}
 
 	dsp_destroy_dev(dev);
 
-	if (mixer_uninit(dev) == EBUSY) {
-		device_printf(dev, "unregister: mixer busy\n");
-		PCM_LOCK(d);
-		PCM_RELEASE(d);
-		PCM_UNLOCK(d);
-		return (EBUSY);
-	}
+	(void)mixer_uninit(dev);
 
 	/* remove /dev/sndstat entry first */
 	sndstat_unregister(dev);
diff --git a/sys/dev/sound/usb/uaudio.c b/sys/dev/sound/usb/uaudio.c
index 5d7396c527e0..2351c2522021 100644
--- a/sys/dev/sound/usb/uaudio.c
+++ b/sys/dev/sound/usb/uaudio.c
@@ -1255,20 +1255,13 @@ uaudio_detach_sub(device_t dev)
 	unsigned i = uaudio_get_child_index_by_dev(sc, dev);
 	int error = 0;
 
-repeat:
 	if (sc->sc_child[i].pcm_registered) {
 		error = pcm_unregister(dev);
-	} else {
-		if (sc->sc_child[i].mixer_init)
-			error = mixer_uninit(dev);
+	} else if (sc->sc_child[i].mixer_init) {
+		error = mixer_uninit(dev);
 	}
 
-	if (error) {
-		device_printf(dev, "Waiting for sound application to exit!\n");
-		usb_pause_mtx(NULL, 2 * hz);
-		goto repeat;		/* try again */
-	}
-	return (0);			/* success */
+	return (error);
 }
 
 static int