From: Luiz Augusto von Dentz <[email protected]>
Change that introduced the use of __check_timeout did not account for
link types properly, it always assumes ACL_LINK is used thus causing
hdev->acl_last_tx to be used even in case of LE_LINK and then again
uses ACL_LINK with hci_link_tx_to.
To fix this __check_timeout now takes the link type as parameter and
then procedure to use the right last_tx based on the link type and pass
it to hci_link_tx_to.
Fixes: 1b1d29e51499 ("Bluetooth: Make use of __check_timeout on hci_sched_le")
Signed-off-by: Luiz Augusto von Dentz <[email protected]>
---
net/bluetooth/hci_core.c | 34 +++++++++++++++++++++++-----------
1 file changed, 23 insertions(+), 11 deletions(-)
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 66c7cdba0d32..063fbb8e07ca 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3485,15 +3485,27 @@ static inline int __get_blocks(struct hci_dev *hdev, struct sk_buff *skb)
return DIV_ROUND_UP(skb->len - HCI_ACL_HDR_SIZE, hdev->block_len);
}
-static void __check_timeout(struct hci_dev *hdev, unsigned int cnt)
+static void __check_timeout(struct hci_dev *hdev, unsigned int cnt, u8 type)
{
- if (!hci_dev_test_flag(hdev, HCI_UNCONFIGURED)) {
- /* ACL tx timeout must be longer than maximum
- * link supervision timeout (40.9 seconds) */
- if (!cnt && time_after(jiffies, hdev->acl_last_tx +
- HCI_ACL_TX_TIMEOUT))
- hci_link_tx_to(hdev, ACL_LINK);
+ unsigned long last_tx;
+
+ if (hci_dev_test_flag(hdev, HCI_UNCONFIGURED))
+ return;
+
+ switch (type) {
+ case LE_LINK:
+ last_tx = hdev->le_last_tx;
+ break;
+ default:
+ last_tx = hdev->acl_last_tx;
+ break;
}
+
+ /* tx timeout must be longer than maximum link supervision timeout
+ * (40.9 seconds)
+ */
+ if (!cnt && time_after(jiffies, last_tx + HCI_ACL_TX_TIMEOUT))
+ hci_link_tx_to(hdev, type);
}
/* Schedule SCO */
@@ -3551,7 +3563,7 @@ static void hci_sched_acl_pkt(struct hci_dev *hdev)
struct sk_buff *skb;
int quote;
- __check_timeout(hdev, cnt);
+ __check_timeout(hdev, cnt, ACL_LINK);
while (hdev->acl_cnt &&
(chan = hci_chan_sent(hdev, ACL_LINK, "e))) {
@@ -3594,8 +3606,6 @@ static void hci_sched_acl_blk(struct hci_dev *hdev)
int quote;
u8 type;
- __check_timeout(hdev, cnt);
-
BT_DBG("%s", hdev->name);
if (hdev->dev_type == HCI_AMP)
@@ -3603,6 +3613,8 @@ static void hci_sched_acl_blk(struct hci_dev *hdev)
else
type = ACL_LINK;
+ __check_timeout(hdev, cnt, type);
+
while (hdev->block_cnt > 0 &&
(chan = hci_chan_sent(hdev, type, "e))) {
u32 priority = (skb_peek(&chan->data_q))->priority;
@@ -3676,7 +3688,7 @@ static void hci_sched_le(struct hci_dev *hdev)
cnt = hdev->le_pkts ? hdev->le_cnt : hdev->acl_cnt;
- __check_timeout(hdev, cnt);
+ __check_timeout(hdev, cnt, LE_LINK);
tmp = cnt;
while (cnt && (chan = hci_chan_sent(hdev, LE_LINK, "e))) {
--
2.37.3
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=680748
---Test result---
Test Summary:
CheckPatch PASS 1.76 seconds
GitLint PASS 1.07 seconds
SubjectPrefix PASS 0.87 seconds
BuildKernel PASS 46.19 seconds
BuildKernel32 PASS 40.66 seconds
Incremental Build with patchesPASS 56.67 seconds
TestRunner: Setup PASS 677.05 seconds
TestRunner: l2cap-tester PASS 21.35 seconds
TestRunner: iso-tester PASS 21.64 seconds
TestRunner: bnep-tester PASS 8.50 seconds
TestRunner: mgmt-tester PASS 130.54 seconds
TestRunner: rfcomm-tester PASS 12.74 seconds
TestRunner: sco-tester PASS 12.23 seconds
TestRunner: ioctl-tester PASS 14.32 seconds
TestRunner: smp-tester PASS 11.85 seconds
TestRunner: userchan-tester PASS 8.89 seconds
---
Regards,
Linux Bluetooth
On 2022-09-27 00:51, Luiz Augusto von Dentz wrote:
> From: Luiz Augusto von Dentz <[email protected]>
>
> Change that introduced the use of __check_timeout did not account for
> link types properly, it always assumes ACL_LINK is used thus causing
> hdev->acl_last_tx to be used even in case of LE_LINK and then again
> uses ACL_LINK with hci_link_tx_to.
>
> To fix this __check_timeout now takes the link type as parameter and
> then procedure to use the right last_tx based on the link type and pass
> it to hci_link_tx_to.
>
> Fixes: 1b1d29e51499 ("Bluetooth: Make use of __check_timeout on hci_sched_le")
> Signed-off-by: Luiz Augusto von Dentz <[email protected]>
Tested-by: David Beinder <[email protected]>
Patch tested on 5.10.136-cip14 (sunxi / armv7l), with a RTL8821CU
adapter in LE-only mode. Spurious "link tx timeout" errors during LE
data transfers are now gone.
> ---
> net/bluetooth/hci_core.c | 34 +++++++++++++++++++++++-----------
> 1 file changed, 23 insertions(+), 11 deletions(-)
>
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 66c7cdba0d32..063fbb8e07ca 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -3485,15 +3485,27 @@ static inline int __get_blocks(struct hci_dev *hdev, struct sk_buff *skb)
> return DIV_ROUND_UP(skb->len - HCI_ACL_HDR_SIZE, hdev->block_len);
> }
>
> -static void __check_timeout(struct hci_dev *hdev, unsigned int cnt)
> +static void __check_timeout(struct hci_dev *hdev, unsigned int cnt, u8 type)
> {
> - if (!hci_dev_test_flag(hdev, HCI_UNCONFIGURED)) {
> - /* ACL tx timeout must be longer than maximum
> - * link supervision timeout (40.9 seconds) */
> - if (!cnt && time_after(jiffies, hdev->acl_last_tx +
> - HCI_ACL_TX_TIMEOUT))
> - hci_link_tx_to(hdev, ACL_LINK);
> + unsigned long last_tx;
> +
> + if (hci_dev_test_flag(hdev, HCI_UNCONFIGURED))
> + return;
> +
> + switch (type) {
> + case LE_LINK:
> + last_tx = hdev->le_last_tx;
> + break;
> + default:
> + last_tx = hdev->acl_last_tx;
> + break;
> }
> +
> + /* tx timeout must be longer than maximum link supervision timeout
> + * (40.9 seconds)
> + */
> + if (!cnt && time_after(jiffies, last_tx + HCI_ACL_TX_TIMEOUT))
> + hci_link_tx_to(hdev, type);
> }
>
> /* Schedule SCO */
> @@ -3551,7 +3563,7 @@ static void hci_sched_acl_pkt(struct hci_dev *hdev)
> struct sk_buff *skb;
> int quote;
>
> - __check_timeout(hdev, cnt);
> + __check_timeout(hdev, cnt, ACL_LINK);
>
> while (hdev->acl_cnt &&
> (chan = hci_chan_sent(hdev, ACL_LINK, "e))) {
> @@ -3594,8 +3606,6 @@ static void hci_sched_acl_blk(struct hci_dev *hdev)
> int quote;
> u8 type;
>
> - __check_timeout(hdev, cnt);
> -
> BT_DBG("%s", hdev->name);
>
> if (hdev->dev_type == HCI_AMP)
> @@ -3603,6 +3613,8 @@ static void hci_sched_acl_blk(struct hci_dev *hdev)
> else
> type = ACL_LINK;
>
> + __check_timeout(hdev, cnt, type);
> +
> while (hdev->block_cnt > 0 &&
> (chan = hci_chan_sent(hdev, type, "e))) {
> u32 priority = (skb_peek(&chan->data_q))->priority;
> @@ -3676,7 +3688,7 @@ static void hci_sched_le(struct hci_dev *hdev)
>
> cnt = hdev->le_pkts ? hdev->le_cnt : hdev->acl_cnt;
>
> - __check_timeout(hdev, cnt);
> + __check_timeout(hdev, cnt, LE_LINK);
>
> tmp = cnt;
> while (cnt && (chan = hci_chan_sent(hdev, LE_LINK, "e))) {
Hello:
This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <[email protected]>:
On Mon, 26 Sep 2022 15:51:07 -0700 you wrote:
> From: Luiz Augusto von Dentz <[email protected]>
>
> Change that introduced the use of __check_timeout did not account for
> link types properly, it always assumes ACL_LINK is used thus causing
> hdev->acl_last_tx to be used even in case of LE_LINK and then again
> uses ACL_LINK with hci_link_tx_to.
>
> [...]
Here is the summary with links:
- Bluetooth: hci_core: Fix not handling link timeouts propertly
https://git.kernel.org/bluetooth/bluetooth-next/c/116523c8fac0
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html