* Re: [PATCH] Bluetooth: btmtksdio: fix deadlock in close and reset paths [not found] <FD5D03449312D17C+20260806-btmtksdio-deadlock-fix-v1-1-3a2d4392d117@uniontech.com> @ 2026-08-07 16:10 ` Luiz Augusto von Dentz 2026-08-10 10:22 ` [PATCH v2] " ZhaoJinming 0 siblings, 1 reply; 3+ messages in thread From: Luiz Augusto von Dentz @ 2026-08-07 16:10 UTC (permalink / raw) To: ZhaoJinming Cc: Marcel Holtmann, linux-bluetooth, Matthias Brugger, AngeloGioacchino Del Regno, linux-kernel, linux-arm-kernel, linux-mediatek Hi ZhaoJinming, On Thu, Aug 6, 2026 at 2:36 AM ZhaoJinming <zhaojinming@uniontech.com> wrote: > > btmtksdio_close() and btmtksdio_reset() call cancel_work_sync() on > bdev->txrx_work while holding the sdio host lock, which is also acquired > by btmtksdio_txrx_work(). If txrx_work is queued when close/reset runs, > a worker thread may start it after the host lock is taken and block in > sdio_claim_host(), while cancel_work_sync() waits for the work to > finish. The host lock is only released after cancel_work_sync() > returns, so both sides wait forever, deadlocking close/reset. > > Fix this by releasing the sdio host lock before calling > cancel_work_sync(), then re-acquiring it afterwards. The interrupt is > already disabled (sdio_release_irq() in close, C_INT_EN_CLR in reset) > before the work is cancelled, so no new work can be scheduled and > cancel_work_sync() fully quiesces txrx_work before the device is torn > down. This mirrors the pattern already used by btmtksdio_flush(), > which cancels the work without holding the host lock. > > Signed-off-by: ZhaoJinming <zhaojinming@uniontech.com> > --- > btmtksdio_close() and btmtksdio_reset() call cancel_work_sync() on > bdev->txrx_work while holding the sdio host lock that > btmtksdio_txrx_work() also acquires. If txrx_work is queued at that > point, a worker thread can start it and block in sdio_claim_host(), > while cancel_work_sync() waits for the work to finish and the host lock > is only released afterwards - a deadlock. > > This series releases the host lock around cancel_work_sync() so the work > can always complete, mirroring the pattern already used by > btmtksdio_flush(). > --- > > --- > drivers/bluetooth/btmtksdio.c | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/drivers/bluetooth/btmtksdio.c b/drivers/bluetooth/btmtksdio.c > index c6f80c419e901e71b21e550449250a5a6755d100..879d580226d25ad398105a158de1545dace33718 100644 > --- a/drivers/bluetooth/btmtksdio.c > +++ b/drivers/bluetooth/btmtksdio.c > @@ -746,8 +746,16 @@ static int btmtksdio_close(struct hci_dev *hdev) > > sdio_release_irq(bdev->func); > > + /* No new work can be scheduled after sdio_release_irq(), so cancel the > + * work outside the sdio host lock. btmtksdio_txrx_work() also claims > + * the host, so canceling it while holding the lock would deadlock. > + */ > + sdio_release_host(bdev->func); > + > cancel_work_sync(&bdev->txrx_work); > > + sdio_claim_host(bdev->func); > + > btmtksdio_fw_pmctrl(bdev); > > clear_bit(BTMTKSDIO_FUNC_ENABLED, &bdev->tx_state); > @@ -1293,8 +1301,18 @@ static void btmtksdio_reset(struct hci_dev *hdev) > > sdio_writel(bdev->func, C_INT_EN_CLR, MTK_REG_CHLPCR, NULL); > skb_queue_purge(&bdev->txq); > + > + /* With the interrupt disabled, the SDIO IRQ handler can no longer > + * schedule txrx_work. Cancel the work outside the sdio host lock; > + * btmtksdio_txrx_work() also claims the host, so canceling it while > + * holding the lock would deadlock. > + */ > + sdio_release_host(bdev->func); > + > cancel_work_sync(&bdev->txrx_work); > > + sdio_claim_host(bdev->func); > + > gpiod_set_value_cansleep(bdev->reset, 1); > msleep(100); > gpiod_set_value_cansleep(bdev->reset, 0); > > --- > base-commit: 0d839570765118029aa8bf4a95444c6a11aacf85 > change-id: 20260806-btmtksdio-deadlock-fix-f9421f1a7879 > > Best regards, > -- > ZhaoJinming <zhaojinming@uniontech.com> Sashiko found a problem: https://sashiko.dev/#/patchset/FD5D03449312D17C%2B20260806-btmtksdio-deadlock-fix-v1-1-3a2d4392d117%40uniontech.com -- Luiz Augusto von Dentz ^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH v2] Bluetooth: btmtksdio: fix deadlock in close and reset paths 2026-08-07 16:10 ` [PATCH] Bluetooth: btmtksdio: fix deadlock in close and reset paths Luiz Augusto von Dentz @ 2026-08-10 10:22 ` ZhaoJinming 2026-08-11 20:10 ` patchwork-bot+bluetooth 0 siblings, 1 reply; 3+ messages in thread From: ZhaoJinming @ 2026-08-10 10:22 UTC (permalink / raw) To: Luiz Augusto von Dentz, linux-bluetooth Cc: Marcel Holtmann, Matthias Brugger, AngeloGioacchino Del Regno, linux-kernel, linux-arm-kernel, linux-mediatek, ZhaoJinming btmtksdio_close() and btmtksdio_reset() call cancel_work_sync() on bdev->txrx_work while holding the sdio host lock, which is also acquired by btmtksdio_txrx_work(). If txrx_work is queued when close/reset runs, a worker thread may start it after the host lock is taken and block in sdio_claim_host(), while cancel_work_sync() waits for the work to finish. The host lock is only released after cancel_work_sync() returns, so both sides wait forever, deadlocking close/reset. Fix this by releasing the sdio host lock before calling cancel_work_sync(), then re-acquiring it afterwards. In btmtksdio_close() the interrupt is already disabled by sdio_release_irq(), which also unregisters the IRQ handler, so no new work can be scheduled and cancel_work_sync() fully quiesces txrx_work. btmtksdio_reset() must additionally unregister the IRQ handler before dropping the host lock: btmtksdio_txrx_work() unconditionally re-enables the device interrupt (C_INT_EN_SET) when the handler is still registered, so an in-flight worker would re-enable interrupts and be rescheduled while the device is being reset, defeating the cancellation. The IRQ is re-claimed by btmtksdio_open() when the HCI device is re-opened after the reset. This mirrors the pattern already used by btmtksdio_flush(), which cancels the work without holding the host lock. Signed-off-by: ZhaoJinming <zhaojinming@uniontech.com> --- Changes in v2: - Unregister the IRQ handler in btmtksdio_reset() before dropping the host lock, so a concurrent txrx_work cannot re-enable the device interrupt (C_INT_EN_SET) and be rescheduled during reset. --- drivers/bluetooth/btmtksdio.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/drivers/bluetooth/btmtksdio.c b/drivers/bluetooth/btmtksdio.c index c6f80c419e901e71b21e550449250a5a6755d100..23650df4fb06114ab7be1f0b30eb61a15322b288 100644 --- a/drivers/bluetooth/btmtksdio.c +++ b/drivers/bluetooth/btmtksdio.c @@ -746,8 +746,16 @@ static int btmtksdio_close(struct hci_dev *hdev) sdio_release_irq(bdev->func); + /* No new work can be scheduled after sdio_release_irq(), so cancel the + * work outside the sdio host lock. btmtksdio_txrx_work() also claims + * the host, so canceling it while holding the lock would deadlock. + */ + sdio_release_host(bdev->func); + cancel_work_sync(&bdev->txrx_work); + sdio_claim_host(bdev->func); + btmtksdio_fw_pmctrl(bdev); clear_bit(BTMTKSDIO_FUNC_ENABLED, &bdev->tx_state); @@ -1293,8 +1301,22 @@ static void btmtksdio_reset(struct hci_dev *hdev) sdio_writel(bdev->func, C_INT_EN_CLR, MTK_REG_CHLPCR, NULL); skb_queue_purge(&bdev->txq); + + /* Unregister the IRQ before releasing the host lock so that a + * concurrently running btmtksdio_txrx_work() cannot re-enable the + * device interrupt (C_INT_EN_SET) and be rescheduled while the device + * is being reset. btmtksdio_txrx_work() also claims the host, so the + * work must be cancelled outside the sdio host lock to avoid a + * deadlock. The IRQ is re-claimed by btmtksdio_open() when the HCI + * device is re-opened after the reset. + */ + sdio_release_irq(bdev->func); + sdio_release_host(bdev->func); + cancel_work_sync(&bdev->txrx_work); + sdio_claim_host(bdev->func); + gpiod_set_value_cansleep(bdev->reset, 1); msleep(100); gpiod_set_value_cansleep(bdev->reset, 0); --- base-commit: 0d839570765118029aa8bf4a95444c6a11aacf85 change-id: 20260806-btmtksdio-deadlock-fix-f9421f1a7879 Best regards, -- ZhaoJinming <zhaojinming@uniontech.com> ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] Bluetooth: btmtksdio: fix deadlock in close and reset paths 2026-08-10 10:22 ` [PATCH v2] " ZhaoJinming @ 2026-08-11 20:10 ` patchwork-bot+bluetooth 0 siblings, 0 replies; 3+ messages in thread From: patchwork-bot+bluetooth @ 2026-08-11 20:10 UTC (permalink / raw) To: ZhaoJinming Cc: luiz.dentz, linux-bluetooth, marcel, matthias.bgg, angelogioacchino.delregno, linux-kernel, linux-arm-kernel, linux-mediatek Hello: This patch was applied to bluetooth/bluetooth-next.git (master) by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>: On Mon, 10 Aug 2026 18:22:38 +0800 you wrote: > btmtksdio_close() and btmtksdio_reset() call cancel_work_sync() on > bdev->txrx_work while holding the sdio host lock, which is also acquired > by btmtksdio_txrx_work(). If txrx_work is queued when close/reset runs, > a worker thread may start it after the host lock is taken and block in > sdio_claim_host(), while cancel_work_sync() waits for the work to > finish. The host lock is only released after cancel_work_sync() > returns, so both sides wait forever, deadlocking close/reset. > > [...] Here is the summary with links: - [v2] Bluetooth: btmtksdio: fix deadlock in close and reset paths https://git.kernel.org/bluetooth/bluetooth-next/c/fca8fe614904 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-08-11 20:10 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <FD5D03449312D17C+20260806-btmtksdio-deadlock-fix-v1-1-3a2d4392d117@uniontech.com>
2026-08-07 16:10 ` [PATCH] Bluetooth: btmtksdio: fix deadlock in close and reset paths Luiz Augusto von Dentz
2026-08-10 10:22 ` [PATCH v2] " ZhaoJinming
2026-08-11 20:10 ` patchwork-bot+bluetooth
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox