* [PATCH] Bluetooth: btmtksdio: fix infinite loop in btmtksdio_txrx_work()
@ 2026-06-09 12:10 Sergey Senozhatsky
2026-06-09 14:38 ` bluez.test.bot
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Sergey Senozhatsky @ 2026-06-09 12:10 UTC (permalink / raw)
To: Marcel Holtmann, Luiz Augusto von Dentz, Mark-yw Chen, Sean Wang
Cc: Tomasz Figa, linux-bluetooth, linux-kernel, linux-arm-kernel,
linux-mediatek, Sergey Senozhatsky, stable
Every once in a while we see a hung btmtksdio_flush() task:
INFO: task kworker/u17:0:189 blocked for more than 122 seconds.
__cancel_work_timer+0x3f4/0x460
cancel_work_sync+0x1c/0x2c
btmtksdio_flush+0x2c/0x40
hci_dev_open_sync+0x10c4/0x2190
[..]
It all boils down to incorrect time_is_before_jiffies() usage in
btmtksdio_txrx_work(). The btmtksdio_txrx_work() loop is expected
to be terminated if running for longer than 5*HZ. However the
timeout check is twisted: time_is_before_jiffies(old_jiffies + 5*HZ)
evaluates to true when old_jiffies + 5*HZ is in the past i.e. when a
timeout has occurred. Using OR with time_is_before_jiffies(txrx_timeout)
means that:
- before the 5-second timeout: the condition is `int_status || false`,
so it loops as long as there are pending interrupts.
- after the 5-second timeout: the condition becomes `int_status || true`,
which is always true.
When the loop becomes infinite btmtksdio_txrx_work() loop never
terminates and never releases the SDIO host.
Fix loop termination condition to actually enforce a 5*HZ timeout.
Fixes: 26270bc189ea4 ("Bluetooth: btmtksdio: move interrupt service to work")
Cc: stable@vger.kernel.org
Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
drivers/bluetooth/btmtksdio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/bluetooth/btmtksdio.c b/drivers/bluetooth/btmtksdio.c
index 5b0fab7b89b5..c6f80c419e90 100644
--- a/drivers/bluetooth/btmtksdio.c
+++ b/drivers/bluetooth/btmtksdio.c
@@ -620,7 +620,7 @@ static void btmtksdio_txrx_work(struct work_struct *work)
if (btmtksdio_rx_packet(bdev, rx_size) < 0)
bdev->hdev->stat.err_rx++;
}
- } while (int_status || time_is_before_jiffies(txrx_timeout));
+ } while (int_status && time_is_after_jiffies(txrx_timeout));
/* Enable interrupt */
if (bdev->func->irq_handler)
--
2.54.0.1064.gd145956f57-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread* RE: Bluetooth: btmtksdio: fix infinite loop in btmtksdio_txrx_work() 2026-06-09 12:10 [PATCH] Bluetooth: btmtksdio: fix infinite loop in btmtksdio_txrx_work() Sergey Senozhatsky @ 2026-06-09 14:38 ` bluez.test.bot 2026-06-10 6:52 ` [PATCH] " Sean Wang ` (2 subsequent siblings) 3 siblings, 0 replies; 9+ messages in thread From: bluez.test.bot @ 2026-06-09 14:38 UTC (permalink / raw) To: linux-bluetooth, senozhatsky [-- Attachment #1: Type: text/plain, Size: 988 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=1108559 ---Test result--- Test Summary: CheckPatch PASS 0.74 seconds VerifyFixes PASS 0.13 seconds VerifySignedoff PASS 0.13 seconds GitLint PASS 0.37 seconds SubjectPrefix PASS 0.13 seconds BuildKernel PASS 26.65 seconds CheckAllWarning PASS 29.25 seconds CheckSparse PASS 27.91 seconds BuildKernel32 PASS 26.67 seconds TestRunnerSetup PASS 550.04 seconds IncrementalBuild PASS 25.31 seconds https://github.com/bluez/bluetooth-next/pull/299 --- Regards, Linux Bluetooth ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Bluetooth: btmtksdio: fix infinite loop in btmtksdio_txrx_work() 2026-06-09 12:10 [PATCH] Bluetooth: btmtksdio: fix infinite loop in btmtksdio_txrx_work() Sergey Senozhatsky 2026-06-09 14:38 ` bluez.test.bot @ 2026-06-10 6:52 ` Sean Wang 2026-06-19 13:27 ` Takashi Iwai 2026-06-10 8:51 ` Sergey Senozhatsky 2026-06-11 17:58 ` patchwork-bot+bluetooth 3 siblings, 1 reply; 9+ messages in thread From: Sean Wang @ 2026-06-10 6:52 UTC (permalink / raw) To: Sergey Senozhatsky Cc: Marcel Holtmann, Luiz Augusto von Dentz, Mark-yw Chen, Sean Wang, Tomasz Figa, linux-bluetooth, linux-kernel, linux-arm-kernel, linux-mediatek, stable Hi, On Tue, Jun 9, 2026 at 7:19 AM Sergey Senozhatsky <senozhatsky@chromium.org> wrote: > > Every once in a while we see a hung btmtksdio_flush() task: > > INFO: task kworker/u17:0:189 blocked for more than 122 seconds. > __cancel_work_timer+0x3f4/0x460 > cancel_work_sync+0x1c/0x2c > btmtksdio_flush+0x2c/0x40 > hci_dev_open_sync+0x10c4/0x2190 > [..] > > It all boils down to incorrect time_is_before_jiffies() usage in > btmtksdio_txrx_work(). The btmtksdio_txrx_work() loop is expected > to be terminated if running for longer than 5*HZ. However the > timeout check is twisted: time_is_before_jiffies(old_jiffies + 5*HZ) > evaluates to true when old_jiffies + 5*HZ is in the past i.e. when a > timeout has occurred. Using OR with time_is_before_jiffies(txrx_timeout) > means that: > - before the 5-second timeout: the condition is `int_status || false`, > so it loops as long as there are pending interrupts. > - after the 5-second timeout: the condition becomes `int_status || true`, > which is always true. > > When the loop becomes infinite btmtksdio_txrx_work() loop never > terminates and never releases the SDIO host. > > Fix loop termination condition to actually enforce a 5*HZ timeout. > > Fixes: 26270bc189ea4 ("Bluetooth: btmtksdio: move interrupt service to work") > Cc: stable@vger.kernel.org > Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org> > --- > drivers/bluetooth/btmtksdio.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/bluetooth/btmtksdio.c b/drivers/bluetooth/btmtksdio.c > index 5b0fab7b89b5..c6f80c419e90 100644 > --- a/drivers/bluetooth/btmtksdio.c > +++ b/drivers/bluetooth/btmtksdio.c > @@ -620,7 +620,7 @@ static void btmtksdio_txrx_work(struct work_struct *work) > if (btmtksdio_rx_packet(bdev, rx_size) < 0) > bdev->hdev->stat.err_rx++; > } > - } while (int_status || time_is_before_jiffies(txrx_timeout)); > + } while (int_status && time_is_after_jiffies(txrx_timeout)); yes, loop continues only while there is interrupt work and the timeout deadline is still in the future Reviewed-by: Sean Wang <sean.wang@mediatek.com> Thanks for fixing this long-standing issue. > > /* Enable interrupt */ > if (bdev->func->irq_handler) > -- > 2.54.0.1064.gd145956f57-goog > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Bluetooth: btmtksdio: fix infinite loop in btmtksdio_txrx_work() 2026-06-10 6:52 ` [PATCH] " Sean Wang @ 2026-06-19 13:27 ` Takashi Iwai 2026-06-19 14:20 ` Tomasz Figa [not found] ` <CAAFQd5DyDQo9vBH80YYQBW7Bgf64F1m9q44-jhf1cc75XYpftA@mail.gmail.com> 0 siblings, 2 replies; 9+ messages in thread From: Takashi Iwai @ 2026-06-19 13:27 UTC (permalink / raw) To: Sean Wang Cc: Sergey Senozhatsky, Marcel Holtmann, Luiz Augusto von Dentz, Mark-yw Chen, Sean Wang, Tomasz Figa, linux-bluetooth, linux-kernel, linux-arm-kernel, linux-mediatek, stable On Wed, 10 Jun 2026 08:52:31 +0200, Sean Wang wrote: > > Hi, > > On Tue, Jun 9, 2026 at 7:19 AM Sergey Senozhatsky > <senozhatsky@chromium.org> wrote: > > > > Every once in a while we see a hung btmtksdio_flush() task: > > > > INFO: task kworker/u17:0:189 blocked for more than 122 seconds. > > __cancel_work_timer+0x3f4/0x460 > > cancel_work_sync+0x1c/0x2c > > btmtksdio_flush+0x2c/0x40 > > hci_dev_open_sync+0x10c4/0x2190 > > [..] > > > > It all boils down to incorrect time_is_before_jiffies() usage in > > btmtksdio_txrx_work(). The btmtksdio_txrx_work() loop is expected > > to be terminated if running for longer than 5*HZ. However the > > timeout check is twisted: time_is_before_jiffies(old_jiffies + 5*HZ) > > evaluates to true when old_jiffies + 5*HZ is in the past i.e. when a > > timeout has occurred. Using OR with time_is_before_jiffies(txrx_timeout) > > means that: > > - before the 5-second timeout: the condition is `int_status || false`, > > so it loops as long as there are pending interrupts. > > - after the 5-second timeout: the condition becomes `int_status || true`, > > which is always true. > > > > When the loop becomes infinite btmtksdio_txrx_work() loop never > > terminates and never releases the SDIO host. > > > > Fix loop termination condition to actually enforce a 5*HZ timeout. > > > > Fixes: 26270bc189ea4 ("Bluetooth: btmtksdio: move interrupt service to work") > > Cc: stable@vger.kernel.org > > Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org> > > --- > > drivers/bluetooth/btmtksdio.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/bluetooth/btmtksdio.c b/drivers/bluetooth/btmtksdio.c > > index 5b0fab7b89b5..c6f80c419e90 100644 > > --- a/drivers/bluetooth/btmtksdio.c > > +++ b/drivers/bluetooth/btmtksdio.c > > @@ -620,7 +620,7 @@ static void btmtksdio_txrx_work(struct work_struct *work) > > if (btmtksdio_rx_packet(bdev, rx_size) < 0) > > bdev->hdev->stat.err_rx++; > > } > > - } while (int_status || time_is_before_jiffies(txrx_timeout)); > > + } while (int_status && time_is_after_jiffies(txrx_timeout)); > > yes, loop continues only while there is interrupt work and the timeout > deadline is still in the future I stumbled on this while backporting to distro kernels, and I wonder whether this change is correct. IIUC, this essentially makes the loop exiting right after the first cycle; the patch changed from time_is_before_jiffies() to *_after_*(), not only the logical OR to AND, and *_after_*() returns false, so the whole condition becomes false, too. thanks, Takashi ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Bluetooth: btmtksdio: fix infinite loop in btmtksdio_txrx_work() 2026-06-19 13:27 ` Takashi Iwai @ 2026-06-19 14:20 ` Tomasz Figa [not found] ` <CAAFQd5DyDQo9vBH80YYQBW7Bgf64F1m9q44-jhf1cc75XYpftA@mail.gmail.com> 1 sibling, 0 replies; 9+ messages in thread From: Tomasz Figa @ 2026-06-19 14:20 UTC (permalink / raw) To: Takashi Iwai Cc: Sean Wang, Sergey Senozhatsky, Marcel Holtmann, Luiz Augusto von Dentz, Mark-yw Chen, Sean Wang, linux-bluetooth, linux-kernel, linux-arm-kernel, linux-mediatek, stable [Resending without HTML... Damn Gmail] On Fri, Jun 19, 2026 at 10:27 PM Takashi Iwai <tiwai@suse.de> wrote: > > On Wed, 10 Jun 2026 08:52:31 +0200, > Sean Wang wrote: > > > > Hi, > > > > On Tue, Jun 9, 2026 at 7:19 AM Sergey Senozhatsky > > <senozhatsky@chromium.org> wrote: > > > > > > Every once in a while we see a hung btmtksdio_flush() task: > > > > > > INFO: task kworker/u17:0:189 blocked for more than 122 seconds. > > > __cancel_work_timer+0x3f4/0x460 > > > cancel_work_sync+0x1c/0x2c > > > btmtksdio_flush+0x2c/0x40 > > > hci_dev_open_sync+0x10c4/0x2190 > > > [..] > > > > > > It all boils down to incorrect time_is_before_jiffies() usage in > > > btmtksdio_txrx_work(). The btmtksdio_txrx_work() loop is expected > > > to be terminated if running for longer than 5*HZ. However the > > > timeout check is twisted: time_is_before_jiffies(old_jiffies + 5*HZ) > > > evaluates to true when old_jiffies + 5*HZ is in the past i.e. when a > > > timeout has occurred. Using OR with time_is_before_jiffies(txrx_timeout) > > > means that: > > > - before the 5-second timeout: the condition is `int_status || false`, > > > so it loops as long as there are pending interrupts. > > > - after the 5-second timeout: the condition becomes `int_status || true`, > > > which is always true. > > > > > > When the loop becomes infinite btmtksdio_txrx_work() loop never > > > terminates and never releases the SDIO host. > > > > > > Fix loop termination condition to actually enforce a 5*HZ timeout. > > > > > > Fixes: 26270bc189ea4 ("Bluetooth: btmtksdio: move interrupt service to work") > > > Cc: stable@vger.kernel.org > > > Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org> > > > --- > > > drivers/bluetooth/btmtksdio.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/bluetooth/btmtksdio.c b/drivers/bluetooth/btmtksdio.c > > > index 5b0fab7b89b5..c6f80c419e90 100644 > > > --- a/drivers/bluetooth/btmtksdio.c > > > +++ b/drivers/bluetooth/btmtksdio.c > > > @@ -620,7 +620,7 @@ static void btmtksdio_txrx_work(struct work_struct *work) > > > if (btmtksdio_rx_packet(bdev, rx_size) < 0) > > > bdev->hdev->stat.err_rx++; > > > } > > > - } while (int_status || time_is_before_jiffies(txrx_timeout)); > > > + } while (int_status && time_is_after_jiffies(txrx_timeout)); > > > > yes, loop continues only while there is interrupt work and the timeout > > deadline is still in the future > > I stumbled on this while backporting to distro kernels, and I wonder > whether this change is correct. > > IIUC, this essentially makes the loop exiting right after the first > cycle; the patch changed from time_is_before_jiffies() to *_after_*(), > not only the logical OR to AND, and *_after_*() returns false, so the > whole condition becomes false, too. The intention is for the loop to keep running as long as there is still an interrupt left to handle (int_status != 0) and the timeout has not elapsed (jiffies < txrx_timeout). Note that time_is_after_jiffies(x) returns true if x > jiffies (or jiffies < x): /** * time_is_after_jiffies - return true if a is after jiffies * @a: time (unsigned long) to compare to jiffies * * Return: %true is time a is after jiffies, otherwise %false. */ #define time_is_after_jiffies(a) time_before(jiffies, a) Or am I missing something? Best, Tomasz ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <CAAFQd5DyDQo9vBH80YYQBW7Bgf64F1m9q44-jhf1cc75XYpftA@mail.gmail.com>]
* Re: [PATCH] Bluetooth: btmtksdio: fix infinite loop in btmtksdio_txrx_work() [not found] ` <CAAFQd5DyDQo9vBH80YYQBW7Bgf64F1m9q44-jhf1cc75XYpftA@mail.gmail.com> @ 2026-06-19 14:35 ` Takashi Iwai 2026-06-19 14:53 ` Tomasz Figa 0 siblings, 1 reply; 9+ messages in thread From: Takashi Iwai @ 2026-06-19 14:35 UTC (permalink / raw) To: Tomasz Figa Cc: Takashi Iwai, Sean Wang, Sergey Senozhatsky, Marcel Holtmann, Luiz Augusto von Dentz, Mark-yw Chen, Sean Wang, linux-bluetooth, linux-kernel, linux-arm-kernel, linux-mediatek, stable On Fri, 19 Jun 2026 16:17:31 +0200, Tomasz Figa wrote: > > > On Fri, Jun 19, 2026 at 10:27 PM Takashi Iwai <tiwai@suse.de> wrote: > > > > On Wed, 10 Jun 2026 08:52:31 +0200, > > Sean Wang wrote: > > > > > > Hi, > > > > > > On Tue, Jun 9, 2026 at 7:19 AM Sergey Senozhatsky > > > <senozhatsky@chromium.org> wrote: > > > > > > > > Every once in a while we see a hung btmtksdio_flush() task: > > > > > > > > INFO: task kworker/u17:0:189 blocked for more than 122 seconds. > > > > __cancel_work_timer+0x3f4/0x460 > > > > cancel_work_sync+0x1c/0x2c > > > > btmtksdio_flush+0x2c/0x40 > > > > hci_dev_open_sync+0x10c4/0x2190 > > > > [..] > > > > > > > > It all boils down to incorrect time_is_before_jiffies() usage in > > > > btmtksdio_txrx_work(). The btmtksdio_txrx_work() loop is expected > > > > to be terminated if running for longer than 5*HZ. However the > > > > timeout check is twisted: time_is_before_jiffies(old_jiffies + 5*HZ) > > > > evaluates to true when old_jiffies + 5*HZ is in the past i.e. when a > > > > timeout has occurred. Using OR with time_is_before_jiffies > (txrx_timeout) > > > > means that: > > > > - before the 5-second timeout: the condition is `int_status || false`, > > > > so it loops as long as there are pending interrupts. > > > > - after the 5-second timeout: the condition becomes `int_status || true > `, > > > > which is always true. > > > > > > > > When the loop becomes infinite btmtksdio_txrx_work() loop never > > > > terminates and never releases the SDIO host. > > > > > > > > Fix loop termination condition to actually enforce a 5*HZ timeout. > > > > > > > > Fixes: 26270bc189ea4 ("Bluetooth: btmtksdio: move interrupt service to > work") > > > > Cc: stable@vger.kernel.org > > > > Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org> > > > > --- > > > > drivers/bluetooth/btmtksdio.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/bluetooth/btmtksdio.c b/drivers/bluetooth/ > btmtksdio.c > > > > index 5b0fab7b89b5..c6f80c419e90 100644 > > > > --- a/drivers/bluetooth/btmtksdio.c > > > > +++ b/drivers/bluetooth/btmtksdio.c > > > > @@ -620,7 +620,7 @@ static void btmtksdio_txrx_work(struct work_struct > *work) > > > > if (btmtksdio_rx_packet(bdev, rx_size) < 0) > > > > bdev->hdev->stat.err_rx++; > > > > } > > > > - } while (int_status || time_is_before_jiffies(txrx_timeout)); > > > > + } while (int_status && time_is_after_jiffies(txrx_timeout)); > > > > > > yes, loop continues only while there is interrupt work and the timeout > > > deadline is still in the future > > > > I stumbled on this while backporting to distro kernels, and I wonder > > whether this change is correct. > > > > IIUC, this essentially makes the loop exiting right after the first > > cycle; the patch changed from time_is_before_jiffies() to *_after_*(), > > not only the logical OR to AND, and *_after_*() returns false, so the > > whole condition becomes false, too. > > The intention is for the loop to keep running as long as there is still an > interrupt left to handle (int_status != 0) and the timeout has not elapsed > (jiffies < txrx_timeout). > > Note that time_is_after_jiffies(x) returns true if x > jiffies (or jiffies < > x): > > /** > * time_is_after_jiffies - return true if a is after jiffies > * @a: time (unsigned long) to compare to jiffies > * > * Return: %true is time a is after jiffies, otherwise %false. > */ > #define time_is_after_jiffies(a) time_before(jiffies, a) > > Or am I missing something? Doh, scratch my comment. It's enough confusing about time_after() vs time_is_after_jiffies(). Too hot here to review something today :-< Sorry for the noise! Takashi ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Bluetooth: btmtksdio: fix infinite loop in btmtksdio_txrx_work() 2026-06-19 14:35 ` Takashi Iwai @ 2026-06-19 14:53 ` Tomasz Figa 0 siblings, 0 replies; 9+ messages in thread From: Tomasz Figa @ 2026-06-19 14:53 UTC (permalink / raw) To: Takashi Iwai Cc: Sean Wang, Sergey Senozhatsky, Marcel Holtmann, Luiz Augusto von Dentz, Mark-yw Chen, Sean Wang, linux-bluetooth, linux-kernel, linux-arm-kernel, linux-mediatek, stable On Fri, Jun 19, 2026 at 11:36 PM Takashi Iwai <tiwai@suse.de> wrote: > > On Fri, 19 Jun 2026 16:17:31 +0200, > Tomasz Figa wrote: > > > > > > On Fri, Jun 19, 2026 at 10:27 PM Takashi Iwai <tiwai@suse.de> wrote: > > > > > > On Wed, 10 Jun 2026 08:52:31 +0200, > > > Sean Wang wrote: > > > > > > > > Hi, > > > > > > > > On Tue, Jun 9, 2026 at 7:19 AM Sergey Senozhatsky > > > > <senozhatsky@chromium.org> wrote: > > > > > > > > > > Every once in a while we see a hung btmtksdio_flush() task: > > > > > > > > > > INFO: task kworker/u17:0:189 blocked for more than 122 seconds. > > > > > __cancel_work_timer+0x3f4/0x460 > > > > > cancel_work_sync+0x1c/0x2c > > > > > btmtksdio_flush+0x2c/0x40 > > > > > hci_dev_open_sync+0x10c4/0x2190 > > > > > [..] > > > > > > > > > > It all boils down to incorrect time_is_before_jiffies() usage in > > > > > btmtksdio_txrx_work(). The btmtksdio_txrx_work() loop is expected > > > > > to be terminated if running for longer than 5*HZ. However the > > > > > timeout check is twisted: time_is_before_jiffies(old_jiffies + 5*HZ) > > > > > evaluates to true when old_jiffies + 5*HZ is in the past i.e. when a > > > > > timeout has occurred. Using OR with time_is_before_jiffies > > (txrx_timeout) > > > > > means that: > > > > > - before the 5-second timeout: the condition is `int_status || false`, > > > > > so it loops as long as there are pending interrupts. > > > > > - after the 5-second timeout: the condition becomes `int_status || true > > `, > > > > > which is always true. > > > > > > > > > > When the loop becomes infinite btmtksdio_txrx_work() loop never > > > > > terminates and never releases the SDIO host. > > > > > > > > > > Fix loop termination condition to actually enforce a 5*HZ timeout. > > > > > > > > > > Fixes: 26270bc189ea4 ("Bluetooth: btmtksdio: move interrupt service to > > work") > > > > > Cc: stable@vger.kernel.org > > > > > Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org> > > > > > --- > > > > > drivers/bluetooth/btmtksdio.c | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/bluetooth/btmtksdio.c b/drivers/bluetooth/ > > btmtksdio.c > > > > > index 5b0fab7b89b5..c6f80c419e90 100644 > > > > > --- a/drivers/bluetooth/btmtksdio.c > > > > > +++ b/drivers/bluetooth/btmtksdio.c > > > > > @@ -620,7 +620,7 @@ static void btmtksdio_txrx_work(struct work_struct > > *work) > > > > > if (btmtksdio_rx_packet(bdev, rx_size) < 0) > > > > > bdev->hdev->stat.err_rx++; > > > > > } > > > > > - } while (int_status || time_is_before_jiffies(txrx_timeout)); > > > > > + } while (int_status && time_is_after_jiffies(txrx_timeout)); > > > > > > > > yes, loop continues only while there is interrupt work and the timeout > > > > deadline is still in the future > > > > > > I stumbled on this while backporting to distro kernels, and I wonder > > > whether this change is correct. > > > > > > IIUC, this essentially makes the loop exiting right after the first > > > cycle; the patch changed from time_is_before_jiffies() to *_after_*(), > > > not only the logical OR to AND, and *_after_*() returns false, so the > > > whole condition becomes false, too. > > > > The intention is for the loop to keep running as long as there is still an > > interrupt left to handle (int_status != 0) and the timeout has not elapsed > > (jiffies < txrx_timeout). > > > > Note that time_is_after_jiffies(x) returns true if x > jiffies (or jiffies < > > x): > > > > /** > > * time_is_after_jiffies - return true if a is after jiffies > > * @a: time (unsigned long) to compare to jiffies > > * > > * Return: %true is time a is after jiffies, otherwise %false. > > */ > > #define time_is_after_jiffies(a) time_before(jiffies, a) > > > > Or am I missing something? > > Doh, scratch my comment. It's enough confusing about time_after() vs > time_is_after_jiffies(). Too hot here to review something today :-< > > Sorry for the noise! Haha, no worries, it got me too! (In our internal discussion with Sergey) I had to look up the definition and think about it for quite a while to ensure it was really what we needed. ;) Best, Tomasz ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Bluetooth: btmtksdio: fix infinite loop in btmtksdio_txrx_work() 2026-06-09 12:10 [PATCH] Bluetooth: btmtksdio: fix infinite loop in btmtksdio_txrx_work() Sergey Senozhatsky 2026-06-09 14:38 ` bluez.test.bot 2026-06-10 6:52 ` [PATCH] " Sean Wang @ 2026-06-10 8:51 ` Sergey Senozhatsky 2026-06-11 17:58 ` patchwork-bot+bluetooth 3 siblings, 0 replies; 9+ messages in thread From: Sergey Senozhatsky @ 2026-06-10 8:51 UTC (permalink / raw) To: Marcel Holtmann, Luiz Augusto von Dentz, Mark-yw Chen Cc: Sean Wang, Tomasz Figa, linux-bluetooth, linux-kernel, linux-arm-kernel, linux-mediatek, stable, Sergey Senozhatsky On (26/06/09 21:10), Sergey Senozhatsky wrote: > Every once in a while we see a hung btmtksdio_flush() task: > > INFO: task kworker/u17:0:189 blocked for more than 122 seconds. > __cancel_work_timer+0x3f4/0x460 > cancel_work_sync+0x1c/0x2c > btmtksdio_flush+0x2c/0x40 > hci_dev_open_sync+0x10c4/0x2190 > [..] > > It all boils down to incorrect time_is_before_jiffies() usage in > btmtksdio_txrx_work(). The btmtksdio_txrx_work() loop is expected > to be terminated if running for longer than 5*HZ. However the > timeout check is twisted: time_is_before_jiffies(old_jiffies + 5*HZ) > evaluates to true when old_jiffies + 5*HZ is in the past i.e. when a > timeout has occurred. Using OR with time_is_before_jiffies(txrx_timeout) > means that: > - before the 5-second timeout: the condition is `int_status || false`, > so it loops as long as there are pending interrupts. > - after the 5-second timeout: the condition becomes `int_status || true`, > which is always true. > > When the loop becomes infinite btmtksdio_txrx_work() loop never > terminates and never releases the SDIO host. > > Fix loop termination condition to actually enforce a 5*HZ timeout. Please hold off this patch, this change alone might not be enough. Let us look closer into it, we'll come back you. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Bluetooth: btmtksdio: fix infinite loop in btmtksdio_txrx_work() 2026-06-09 12:10 [PATCH] Bluetooth: btmtksdio: fix infinite loop in btmtksdio_txrx_work() Sergey Senozhatsky ` (2 preceding siblings ...) 2026-06-10 8:51 ` Sergey Senozhatsky @ 2026-06-11 17:58 ` patchwork-bot+bluetooth 3 siblings, 0 replies; 9+ messages in thread From: patchwork-bot+bluetooth @ 2026-06-11 17:58 UTC (permalink / raw) To: Sergey Senozhatsky Cc: marcel, luiz.dentz, mark-yw.chen, sean.wang, tfiga, linux-bluetooth, linux-kernel, linux-arm-kernel, linux-mediatek, stable Hello: This patch was applied to bluetooth/bluetooth-next.git (master) by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>: On Tue, 9 Jun 2026 21:10:06 +0900 you wrote: > Every once in a while we see a hung btmtksdio_flush() task: > > INFO: task kworker/u17:0:189 blocked for more than 122 seconds. > __cancel_work_timer+0x3f4/0x460 > cancel_work_sync+0x1c/0x2c > btmtksdio_flush+0x2c/0x40 > hci_dev_open_sync+0x10c4/0x2190 > [..] > > [...] Here is the summary with links: - Bluetooth: btmtksdio: fix infinite loop in btmtksdio_txrx_work() https://git.kernel.org/bluetooth/bluetooth-next/c/a4263306d01d 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] 9+ messages in thread
end of thread, other threads:[~2026-06-19 14:53 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-09 12:10 [PATCH] Bluetooth: btmtksdio: fix infinite loop in btmtksdio_txrx_work() Sergey Senozhatsky
2026-06-09 14:38 ` bluez.test.bot
2026-06-10 6:52 ` [PATCH] " Sean Wang
2026-06-19 13:27 ` Takashi Iwai
2026-06-19 14:20 ` Tomasz Figa
[not found] ` <CAAFQd5DyDQo9vBH80YYQBW7Bgf64F1m9q44-jhf1cc75XYpftA@mail.gmail.com>
2026-06-19 14:35 ` Takashi Iwai
2026-06-19 14:53 ` Tomasz Figa
2026-06-10 8:51 ` Sergey Senozhatsky
2026-06-11 17:58 ` 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