Skip to content

Commit

Permalink
sound: Implement asynchronous device detach
Browse files Browse the repository at this point in the history
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 44e128f)
  • Loading branch information
christosmarg committed Apr 18, 2024
1 parent e6c51f6 commit d692c31
Show file tree
Hide file tree
Showing 5 changed files with 15 additions and 46 deletions.
11 changes: 1 addition & 10 deletions share/man/man4/snd_uaudio.4
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion sys/dev/sound/pcm/dsp.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
11 changes: 0 additions & 11 deletions sys/dev/sound/pcm/mixer.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
24 changes: 10 additions & 14 deletions sys/dev/sound/pcm/sound.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
13 changes: 3 additions & 10 deletions sys/dev/sound/usb/uaudio.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit d692c31

Please sign in to comment.