Re: [PATCH] Bluetooth: btmtksdio: fix deadlock in close and reset paths
Luiz Augusto von Dentz <[email protected]> Fri, 7 Aug 2026 12:10:23 -0400
| Newsgroups | org.infradead.lists.linux-mediatek,org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-bluetooth,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <CABBYNZK1VKSthhfEi_BRJSX90W_c3sAqCSdsFm3Z6wXYDcWaVA@mail.gmail.com> |
Hi ZhaoJinming, On Thu, Aug 6, 2026 at 2:36 AM ZhaoJinming <[email protected]> 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 <[email protected]> > --- > 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 <[email protected]> Sashiko found a problem: https://sashiko.dev/#/patchset/FD5D03449312D17C%2B20260806-btmtksdio-deadlock-fix-v1-1-3a2d4392d117%40uniontech.com -- Luiz Augusto von Dentz