[PATCH] Bluetooth: btmtksdio: fix infinite loop in btmtksdio_txrx_work()

[PATCH] Bluetooth: btmtksdio: fix infinite loop in btmtksdio_txrx_work()

[Sergey Senozhatsky]

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

Re: [PATCH] Bluetooth: btmtksdio: fix infinite loop in btmtksdio_txrx_work()

[Sean Wang]

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
>
>

Re: [PATCH] Bluetooth: btmtksdio: fix infinite loop in btmtksdio_txrx_work()

[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.

Re: [PATCH] Bluetooth: btmtksdio: fix infinite loop in btmtksdio_txrx_work()

[patchwork-bot+bluetooth]

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

Re: [PATCH] Bluetooth: btmtksdio: fix infinite loop in btmtksdio_txrx_work()

[Takashi Iwai]

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

Re: [PATCH] Bluetooth: btmtksdio: fix infinite loop in btmtksdio_txrx_work()

[Tomasz Figa]

[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

Re: [PATCH] Bluetooth: btmtksdio: fix infinite loop in btmtksdio_txrx_work()

[Takashi Iwai]

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

Re: [PATCH] Bluetooth: btmtksdio: fix infinite loop in btmtksdio_txrx_work()

[Tomasz Figa]

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