Message ID | 20220926225107.3226470-1-luiz.dentz@gmail.com |
---|---|
State | New |
Headers | show |
Series | Bluetooth: hci_core: Fix not handling link timeouts propertly | expand |
On 2022-09-27 00:51, Luiz Augusto von Dentz wrote: > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > 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 <luiz.von.dentz@intel.com> Tested-by: David Beinder <david@beinder.at> 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 <luiz.von.dentz@intel.com>: On Mon, 26 Sep 2022 15:51:07 -0700 you wrote: > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > 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!
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))) {