From nobody Fri Nov 29 12:28:23 2024 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4Y0CBv63tjz5f646; Fri, 29 Nov 2024 12:28:23 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R10" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Y0CBv3Hz2z47db; Fri, 29 Nov 2024 12:28:23 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1732883303; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=bhu0oqPG7wTgDIs9nvWrc4UQ29S6xT4pWfDP4uR3d64=; b=XwtNw3ylJndlOEM974KPJXp/YA1Olw3tgLUt/JamsXaz+sVd2EhEwLESGy1dalsjDGQGG4 H6r9KddZtKyi0FvFTfSlVuTl6SyNRQ5uqnYEzwPZnI1lP0HDzxFFZalGV6q5//25XtZIlD 6cgqWUJWEIZq2zmakRjZucfDhiWdFyhdOVXbPofV/i/K/+rDd0DcDQetSpzHeQuPEHFNev HKK1La22ZoXcGnEnvOVzcjrrBcDJ41xNCz7ZARiPDfNm/TrUTWGcgxErhAcZWQPPjzL6Vm Tqk9vCRihXKsrEl8wihie86DREcHZM1sgG6LlWRoeNLx6Gz4GPej42GQEu5yug== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1732883303; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=bhu0oqPG7wTgDIs9nvWrc4UQ29S6xT4pWfDP4uR3d64=; b=nMrwBWFnKut0VwMqBAsU4OZhO3A0/ybHrstqV1j0tFEw/dZFKmj6mzWF1JLbSJIS+cNSc1 inxM9LXwUm8mh4dXW5u0H6/oVMG36JxwyRKfXJglnJgEtg0JVd83vgQY86vR6DwGrk9z1F JBPGZ3r44nDxZeGXwpjnTZX7BtUW+ZgTUBB0UFaqIecGS8d0U7QIh+JJ8PPUiiy4XERaaM C+YfC7v9P626MbyBUt4XsSfYnPOSrlCA/hL8KsgydleBXJTGv0QJi73kd5OPb41EYY0qf1 PvKRUEjh99+IWtWkCxxst4BQ61e7+r0E1hJ2uUADTwUPJ+JJQJR3zVOIKMfemw== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1732883303; a=rsa-sha256; cv=none; b=O/mMuo95FoXAfE/gNS7XI229lRdp8sw/JsfQGBFGWvGMgZg09J/hxH2eC/PX8Sh07Y2ckj vGi3y67buPVn2QDO0hmufxa1CXM/pipVuYkH9PyjeNUyVEZLzpN2QAWzWCO36s7tVdGvZw aB/RIF51A0a452NjW1sJng1DjT29A1qSdcTrBcc0HeYw3nJ1s9cHYyibVAYe6rXy6QW31N 8rP5vV+8EGdQN5oyOD3UbLVniSxiaO8e0F1IFUV1Tx2za9qOcD/DIUsILuQ5jMdAgbnhUC wbhIx8Vxc/jdy1gsuciqovyENNkANkwhNMCCrfZCbaWq0riymchig4lFi93ikA== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4Y0CBv2dbWzNff; Fri, 29 Nov 2024 12:28:23 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.18.1/8.18.1) with ESMTP id 4ATCSN5e070335; Fri, 29 Nov 2024 12:28:23 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 4ATCSN8M070332; Fri, 29 Nov 2024 12:28:23 GMT (envelope-from git) Date: Fri, 29 Nov 2024 12:28:23 GMT Message-Id: <202411291228.4ATCSN8M070332@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Christos Margiolis Subject: git: 1682d80337ed - stable/14 - sound: Fix hot-unload panics List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: christos X-Git-Repository: src X-Git-Refname: refs/heads/stable/14 X-Git-Reftype: branch X-Git-Commit: 1682d80337ed9fd6037e43d2f59937cbcc322335 Auto-Submitted: auto-generated The branch stable/14 has been updated by christos: URL: https://cgit.FreeBSD.org/src/commit/?id=1682d80337ed9fd6037e43d2f59937cbcc322335 commit 1682d80337ed9fd6037e43d2f59937cbcc322335 Author: Christos Margiolis AuthorDate: 2024-11-26 14:48:24 +0000 Commit: Christos Margiolis CommitDate: 2024-11-29 12:28:06 +0000 sound: Fix hot-unload panics This patch fixes multiple different panic scenarios occuring during hot-unload: 1. The channel is unlocked in chn_read()/chn_write() for uiomove(9) and in the meantime we enter pcm_killchans() and free it. By the time we have returned from userland and try to lock it back, the channel will have been freed. 2. The parent channel has been freed in pcm_killchans(), but at the same time, some yet-unstopped vchan's chn_read()/chn_write() calls chn_start(), which eventually calls vchan_trigger(), which references the freed parent. 3. PCM_WAIT() panics because it references a freed PCM lock. For scenarios 1 and 2, refactor pcm_killchans() to first make sure all channels have been stopped, and then proceed to free them one by one, as opposed to freeing the first free channel until all channels have been freed. This change makes the code more robust, but might introduce some performance overhead when many channels are allocated, since we continuously loop through the channel list until all of them are stopped, and then we loop one last time to free them. For scenario 3, restructure the code so that we can use destroy_dev(9) instead of destroy_dev_sched(9) in dsp_destroy_dev(). Because destroy_dev(9) blocks until all references to the device have went away, we ensure that the PCM cv and lock will be freed safely. While here, move the delete_unrhdr(9) calls to pcm_killchans() and re-order some lines. Sponsored by: The FreeBSD Foundation MFC after: 2 days Reviewed by: dev_submerge.ch Differential Revision: https://reviews.freebsd.org/D47462 (cherry picked from commit 2839ad58dd8a4cf5294180fc599800c437a8d4d8) --- sys/dev/sound/pcm/dsp.c | 2 +- sys/dev/sound/pcm/sound.c | 97 ++++++++++++++++++++--------------------------- 2 files changed, 43 insertions(+), 56 deletions(-) diff --git a/sys/dev/sound/pcm/dsp.c b/sys/dev/sound/pcm/dsp.c index 6a59bcfc1ade..0f19f064a227 100644 --- a/sys/dev/sound/pcm/dsp.c +++ b/sys/dev/sound/pcm/dsp.c @@ -137,7 +137,7 @@ dsp_destroy_dev(device_t dev) struct snddev_info *d; d = device_get_softc(dev); - destroy_dev_sched(d->dsp_dev); + destroy_dev(d->dsp_dev); } static void diff --git a/sys/dev/sound/pcm/sound.c b/sys/dev/sound/pcm/sound.c index e03bcab6d8fc..ebd2e2d697f8 100644 --- a/sys/dev/sound/pcm/sound.c +++ b/sys/dev/sound/pcm/sound.c @@ -211,40 +211,53 @@ static void pcm_killchans(struct snddev_info *d) { struct pcm_channel *ch; - bool found; + bool again; PCM_BUSYASSERT(d); - do { - found = false; + KASSERT(!PCM_REGISTERED(d), ("%s(): still registered\n", __func__)); + + for (;;) { + again = false; + /* Make sure all channels are stopped. */ CHN_FOREACH(ch, d, channels.pcm) { CHN_LOCK(ch); - /* - * Make sure no channel has went to sleep in the - * meantime. - */ - chn_shutdown(ch); - /* - * We have to give a thread sleeping in chn_sleep() a - * chance to observe that the channel is dead. - */ - if ((ch->flags & CHN_F_SLEEPING) == 0) { - found = true; + if ((ch->flags & CHN_F_SLEEPING) == 0 && + CHN_STOPPED(ch) && ch->inprog == 0) { CHN_UNLOCK(ch); - break; + continue; } + chn_shutdown(ch); + if (ch->direction == PCMDIR_PLAY) + chn_flush(ch); + else + chn_abort(ch); CHN_UNLOCK(ch); + again = true; } - /* - * All channels are still sleeping. Sleep for a bit and try - * again to see if any of them is awake now. + * Some channels are still active. Sleep for a bit and try + * again. */ - if (!found) { - pause_sbt("pcmkillchans", SBT_1MS * 5, 0, 0); - continue; - } + if (again) + pause_sbt("pcmkillchans", mstosbt(5), 0, 0); + else + break; + } + + /* All channels are finally dead. */ + while (!CHN_EMPTY(d, channels.pcm)) { + ch = CHN_FIRST(d, channels.pcm); chn_kill(ch); - } while (!CHN_EMPTY(d, channels.pcm)); + } + + if (d->p_unr != NULL) + delete_unrhdr(d->p_unr); + if (d->vp_unr != NULL) + delete_unrhdr(d->vp_unr); + if (d->r_unr != NULL) + delete_unrhdr(d->r_unr); + if (d->vr_unr != NULL) + delete_unrhdr(d->vr_unr); } static int @@ -511,7 +524,6 @@ int pcm_unregister(device_t dev) { struct snddev_info *d; - struct pcm_channel *ch; d = device_get_softc(dev); @@ -524,28 +536,15 @@ pcm_unregister(device_t dev) PCM_WAIT(d); d->flags |= SD_F_DETACHING; + d->flags |= SD_F_DYING; + d->flags &= ~SD_F_REGISTERED; PCM_ACQUIRE(d); PCM_UNLOCK(d); - CHN_FOREACH(ch, d, channels.pcm) { - CHN_LOCK(ch); - /* - * Do not wait for the timeout in chn_read()/chn_write(). Wake - * up the sleeping thread and kill the channel. - */ - chn_shutdown(ch); - chn_abort(ch); - CHN_UNLOCK(ch); - } + pcm_killchans(d); - /* remove /dev/sndstat entry first */ - sndstat_unregister(dev); - - PCM_LOCK(d); - d->flags |= SD_F_DYING; - d->flags &= ~SD_F_REGISTERED; - PCM_UNLOCK(d); + PCM_RELEASE_QUICK(d); if (d->play_sysctl_tree != NULL) { sysctl_ctx_free(&d->play_sysctl_ctx); @@ -556,24 +555,12 @@ pcm_unregister(device_t dev) d->rec_sysctl_tree = NULL; } + sndstat_unregister(dev); + mixer_uninit(dev); dsp_destroy_dev(dev); - (void)mixer_uninit(dev); - - pcm_killchans(d); - PCM_LOCK(d); - PCM_RELEASE(d); cv_destroy(&d->cv); - PCM_UNLOCK(d); snd_mtxfree(d->lock); - if (d->p_unr != NULL) - delete_unrhdr(d->p_unr); - if (d->vp_unr != NULL) - delete_unrhdr(d->vp_unr); - if (d->r_unr != NULL) - delete_unrhdr(d->r_unr); - if (d->vr_unr != NULL) - delete_unrhdr(d->vr_unr); if (snd_unit == device_get_unit(dev)) { snd_unit = pcm_best_unit(-1);