Re: [PATCH] Bluetooth: btmtksdio: fix deadlock in close and reset paths

Luiz Augusto von Dentz <[email protected]>
Newsgroups org.kernel.vger.linux-bluetooth,org.infradead.lists.linux-arm-kernel,org.infradead.lists.linux-mediatek,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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.