All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Bluetooth: btmtksdio: fix deadlock in close and reset paths
@ 2026-08-06  6:35 ZhaoJinming
  2026-08-06  7:48 ` bluez.test.bot
  2026-08-07 16:10 ` [PATCH] " Luiz Augusto von Dentz
  0 siblings, 2 replies; 6+ messages in thread
From: ZhaoJinming @ 2026-08-06  6:35 UTC (permalink / raw)
  To: Marcel Holtmann, Luiz Augusto von Dentz, linux-bluetooth
  Cc: 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.  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>


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* RE: Bluetooth: btmtksdio: fix deadlock in close and reset paths
  2026-08-06  6:35 [PATCH] Bluetooth: btmtksdio: fix deadlock in close and reset paths ZhaoJinming
@ 2026-08-06  7:48 ` bluez.test.bot
  2026-08-07 16:10 ` [PATCH] " Luiz Augusto von Dentz
  1 sibling, 0 replies; 6+ messages in thread
From: bluez.test.bot @ 2026-08-06  7:48 UTC (permalink / raw)
  To: linux-bluetooth, zhaojinming

[-- Attachment #1: Type: text/plain, Size: 1181 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=1141212

---Test result---

Test Summary:
CheckPatch                    PASS      1.93 seconds
VerifyFixes                   PASS      0.12 seconds
VerifySignedoff               PASS      0.11 seconds
GitLint                       PASS      0.29 seconds
SubjectPrefix                 PASS      0.11 seconds
BuildKernel                   PASS      24.87 seconds
CheckAllWarning               PASS      27.42 seconds
CheckSparse                   PASS      26.24 seconds
BuildKernel32                 PASS      24.28 seconds
CheckKernelLLVM               SKIP      0.00 seconds
TestRunnerSetup               PASS      452.08 seconds
IncrementalBuild              PASS      23.62 seconds

Details
##############################
Test: CheckKernelLLVM - SKIP
Desc: Build kernel with LLVM + context analysis
Output:
Clang not found


https://github.com/bluez/bluetooth-next/pull/538

---
Regards,
Linux Bluetooth


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Bluetooth: btmtksdio: fix deadlock in close and reset paths
  2026-08-06  6:35 [PATCH] Bluetooth: btmtksdio: fix deadlock in close and reset paths ZhaoJinming
  2026-08-06  7:48 ` bluez.test.bot
@ 2026-08-07 16:10 ` Luiz Augusto von Dentz
  2026-08-10 10:22   ` [PATCH v2] " ZhaoJinming
  1 sibling, 1 reply; 6+ 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] 6+ messages in thread

* [PATCH v2] Bluetooth: btmtksdio: fix deadlock in close and reset paths
  2026-08-07 16:10 ` [PATCH] " Luiz Augusto von Dentz
@ 2026-08-10 10:22   ` ZhaoJinming
  2026-08-10 11:23     ` [v2] " bluez.test.bot
  2026-08-11 20:10     ` [PATCH v2] " patchwork-bot+bluetooth
  0 siblings, 2 replies; 6+ 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] 6+ messages in thread

* RE: [v2] Bluetooth: btmtksdio: fix deadlock in close and reset paths
  2026-08-10 10:22   ` [PATCH v2] " ZhaoJinming
@ 2026-08-10 11:23     ` bluez.test.bot
  2026-08-11 20:10     ` [PATCH v2] " patchwork-bot+bluetooth
  1 sibling, 0 replies; 6+ messages in thread
From: bluez.test.bot @ 2026-08-10 11:23 UTC (permalink / raw)
  To: linux-bluetooth, zhaojinming

[-- Attachment #1: Type: text/plain, Size: 1181 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=1143277

---Test result---

Test Summary:
CheckPatch                    PASS      0.76 seconds
VerifyFixes                   PASS      0.14 seconds
VerifySignedoff               PASS      0.53 seconds
GitLint                       PASS      0.34 seconds
SubjectPrefix                 PASS      0.13 seconds
BuildKernel                   PASS      26.15 seconds
CheckAllWarning               PASS      29.02 seconds
CheckSparse                   PASS      28.13 seconds
BuildKernel32                 PASS      25.31 seconds
CheckKernelLLVM               SKIP      0.00 seconds
TestRunnerSetup               PASS      463.70 seconds
IncrementalBuild              PASS      24.44 seconds

Details
##############################
Test: CheckKernelLLVM - SKIP
Desc: Build kernel with LLVM + context analysis
Output:
Clang not found


https://github.com/bluez/bluetooth-next/pull/566

---
Regards,
Linux Bluetooth


^ permalink raw reply	[flat|nested] 6+ 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-10 11:23     ` [v2] " bluez.test.bot
@ 2026-08-11 20:10     ` patchwork-bot+bluetooth
  1 sibling, 0 replies; 6+ 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] 6+ messages in thread

end of thread, other threads:[~2026-08-11 20:10 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-06  6:35 [PATCH] Bluetooth: btmtksdio: fix deadlock in close and reset paths ZhaoJinming
2026-08-06  7:48 ` bluez.test.bot
2026-08-07 16:10 ` [PATCH] " Luiz Augusto von Dentz
2026-08-10 10:22   ` [PATCH v2] " ZhaoJinming
2026-08-10 11:23     ` [v2] " bluez.test.bot
2026-08-11 20:10     ` [PATCH v2] " patchwork-bot+bluetooth

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.