From owner-dev-commits-src-branches@freebsd.org Thu Feb 18 08:44:41 2021 Return-Path: Delivered-To: dev-commits-src-branches@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id C910D52FEBB; Thu, 18 Feb 2021 08:44:41 +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 "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Dh7Yj3ctJz3QwW; Thu, 18 Feb 2021 08:44:41 +0000 (UTC) (envelope-from git@FreeBSD.org) 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 6F150150EF; Thu, 18 Feb 2021 08:44:41 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 11I8ifte012991; Thu, 18 Feb 2021 08:44:41 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 11I8ifTP012990; Thu, 18 Feb 2021 08:44:41 GMT (envelope-from git) Date: Thu, 18 Feb 2021 08:44:41 GMT Message-Id: <202102180844.11I8ifTP012990@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Hans Petter Selasky Subject: git: 3ea88413d607 - stable/12 - MFC 12148d4300db: Fix for locking order reversal in USB audio driver, when using mmap(). MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: hselasky X-Git-Repository: src X-Git-Refname: refs/heads/stable/12 X-Git-Reftype: branch X-Git-Commit: 3ea88413d607eea79450cb6570e9a8c6067dd9ae Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-branches@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commits to the stable branches of the FreeBSD src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 18 Feb 2021 08:44:41 -0000 The branch stable/12 has been updated by hselasky: URL: https://cgit.FreeBSD.org/src/commit/?id=3ea88413d607eea79450cb6570e9a8c6067dd9ae commit 3ea88413d607eea79450cb6570e9a8c6067dd9ae Author: Hans Petter Selasky AuthorDate: 2021-02-14 19:29:16 +0000 Commit: Hans Petter Selasky CommitDate: 2021-02-18 08:43:46 +0000 MFC 12148d4300db: Fix for locking order reversal in USB audio driver, when using mmap(). Locking the second lock which causes the LOR, can be skipped because the code updating the shared variables is always executing from the same USB thread. lock order reversal: 1st 0xfffff80005cc3840 pcm7:play:dsp7.p0 (pcm play channel, sleep mutex) @ usb_transfer.c:2342 2nd 0xfffff80005cc3860 pcm7:record:dsp7.r0 (pcm record channel, sleep mutex) @ uaudio.c:2317 lock order pcm record channel -> pcm play channel established at: witness_checkorder+0x461 __mtx_lock_flags+0x98 dsp_mmap_single+0x151 vm_mmap_cdev+0x65 devfs_mmap_f+0x143 kern_mmap_req+0x594 sys_mmap+0x46 amd64_syscall+0x12e fast_syscall_common+0xf8 lock order pcm play channel -> pcm record channel attempted at: witness_checkorder+0xd82 __mtx_lock_flags+0x98 uaudio_chan_play_callback+0xeb usbd_callback_wrapper+0x7ec usb_command_wrapper+0x7e usb_callback_proc+0x8e usb_process+0xf3 fork_exit+0x80 fork_trampoline+0xe Found by: Stefan Ehmann Sponsored by: Mellanox Technologies // NVIDIA Networking (cherry picked from commit 12148d4300dbbd93260bf2801cdb9eda8b3b05a4) --- sys/dev/sound/usb/uaudio.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/sys/dev/sound/usb/uaudio.c b/sys/dev/sound/usb/uaudio.c index 3285710be961..895456d84c58 100644 --- a/sys/dev/sound/usb/uaudio.c +++ b/sys/dev/sound/usb/uaudio.c @@ -2323,11 +2323,16 @@ uaudio_chan_play_callback(struct usb_xfer *xfer, usb_error_t error) case USB_ST_SETUP: tr_setup: if (ch_rec != NULL) { + /* + * NOTE: The play and record callbacks are + * executed from the same USB thread and + * locking the record channel mutex here is + * not needed. This avoids a LOR situation. + */ + /* reset receive jitter counters */ - mtx_lock(ch_rec->pcm_mtx); ch_rec->jitter_curr = 0; ch_rec->jitter_rem = 0; - mtx_unlock(ch_rec->pcm_mtx); } /* reset transmit jitter counters */ @@ -2348,10 +2353,17 @@ tr_setup: */ if (ch_rec != NULL && uaudio_chan_is_async(ch, ch->cur_alt) != 0) { - mtx_lock(ch_rec->pcm_mtx); - if (ch_rec->cur_alt < ch_rec->num_alt) { + uint32_t rec_alt = ch_rec->cur_alt; + if (rec_alt < ch_rec->num_alt) { int64_t tx_jitter; int64_t rx_rate; + /* + * NOTE: The play and record callbacks + * are executed from the same USB + * thread and locking the record + * channel mutex here is not needed. + * This avoids a LOR situation. + */ /* translate receive jitter into transmit jitter */ tx_jitter = ch->usb_alt[ch->cur_alt].sample_rate; @@ -2363,11 +2375,10 @@ tr_setup: ch_rec->jitter_rem = 0; /* compute exact number of transmit jitter samples */ - rx_rate = ch_rec->usb_alt[ch_rec->cur_alt].sample_rate; + rx_rate = ch_rec->usb_alt[rec_alt].sample_rate; ch->jitter_curr += tx_jitter / rx_rate; ch->jitter_rem = tx_jitter % rx_rate; } - mtx_unlock(ch_rec->pcm_mtx); } /* start the SYNC transfer one time per second, if any */