wifi: rtlwifi: pci: Fix condition to wake the queues
PATCH 1/1 wifi: rtlwifi: pci: Fix condition to wake the queues
The TX queues should be woken when there are at least 4 available entries, not when there are at most 4.
Tested with RTL8821AE.
--- drivers/net/wireless/realtek/rtlwifi/pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/wireless/realtek/rtlwifi/pci.c +1 -1
@@ -520,7 +520,7 @@ static void _rtl_pci_tx_isr(struct ieee80211_hw *hw, int prio)rtl_tx_ackqueue (hw, skb); } -if ((ring->entries -skb_queue_len (&ring->queue)) <=4 ) { +if ((ring->entries -skb_queue_len (&ring->queue)) >=4 ) { rtl_dbg(rtlpriv, COMP_ERR, DBG_DMESG, "more desc left, wake skb_queue@%d, ring->idx = %d, skb_queue_len = 0x%x\n", prio, ring->idx,
Signature · 1 line
2.48.1
Ping-Ke Shih
maintainerQuoted 6 lines · Bitterblue Smith
The TX queues should be woken when there are at least 4 available entries, not when there are at most 4.
Tested with RTL8821AE.
Fixes d0311314d002 ("rtlwifi: rtl8192ee: Fix handling of new style descriptors")
I agree your change looks more reasonable. Did you really meet the case and this patch solves it?
By the changes of d0311314d002, I can't really understand why the initial version and the commit do. Considering the commit "fix" some situations, I don't know if this patch would break them again. Could you look into rtlwifi PCI flow deeper and share your ideas?
Quoted 8 lines · Ping-Ke Shih
Quoted 6 lines · Bitterblue Smith
The TX queues should be woken when there are at least 4 available entries, not when there are at most 4.
Tested with RTL8821AE.
Fixes d0311314d002 ("rtlwifi: rtl8192ee: Fix handling of new style descriptors")
Quoted 8 lines
I agree your change looks more reasonable. Did you really meet the case and this patch solves it?
By the changes of
d0311314d002, I can't really understand why the initial version and the commit do. Considering the commit "fix" some situations, I don't know if this patch would break them again. Could you look into rtlwifi PCI flow deeper and share your ideas?
I saw several problems with rtl8821ae but I can't say if this patch fixes any of them:
- After a few hours it randomly stops receiving beacons and data frames for at least 10 seconds ("AP off, try to reconnect now" in dmesg). Then it may or may not be able to reconnect.
- Sometimes the TX speed measured by iperf3 is limited to ~80 Mbps. Normally it's over 250.
- Sometimes there are duplicate pings:
19 packets transmitted, 14 received, +151 duplicates, 26.3158% packet loss, time 18098ms
- Testing the RX speed with iperf3 sometimes makes the mouse cursor stutter. (It's usually ~300 Mbps.)
I don't know how to reproduce any of these.
<= seemed like a clear, simple mistake. I'm not sure about it anymore, so please drop the patch. Maybe I will look into it again some time.
Ping-Ke Shih
maintainerQuoted 18 lines · Bitterblue Smith
Quoted 8 lines · Ping-Ke Shih
Quoted 6 lines · Bitterblue Smith
The TX queues should be woken when there are at least 4 available entries, not when there are at most 4.
Tested with RTL8821AE.
Fixes d0311314d002 ("rtlwifi: rtl8192ee: Fix handling of new style descriptors")Quoted 8 lines
I agree your change looks more reasonable. Did you really meet the case and this patch solves it?
By the changes of
d0311314d002, I can't really understand why the initial version and the commit do. Considering the commit "fix" some situations, I don't know if this patch would break them again. Could you look into rtlwifi PCI flow deeper and share your ideas?
Quoted 6 lines
I saw several problems with rtl8821ae but I can't say if this patch fixes any of them:
- After a few hours it randomly stops receiving beacons and data frames for at least 10 seconds ("AP off, try to reconnect now" in dmesg). Then it may or may not be able to reconnect.
Is interrupt counter increasing in /proc/interrupt?
If not, check if driver ISR (interrupt status register) manually to see if it received data but not issue interrupt to host properly. However, if no any of these, it will be WiFi problem, not PCI.
To bisect WiFi problem, we may stop doing rtl_watchdog_wq_callback() to avoid changing registers dynamically.
Quoted 3 lines
- Sometimes the TX speed measured by iperf3 is limited to ~80 Mbps. Normally it's over 250.
I suggest to check PHY rate first and MSDU aggregation by sniffer. Not sure if rtlwifi can be monitor mode, but very sure rtw88 does.
Quoted 3 lines
- Sometimes there are duplicate pings:
19 packets transmitted, 14 received, +151 duplicates, 26.3158% packet loss, time 18098ms
I have never dug this kind of problem, but I suggest to check sniffer first.
Quoted 3 lines
- Testing the RX speed with iperf3 sometimes makes the mouse cursor stutter. (It's usually ~300 Mbps.)
If here is Bluetooth mouse, it will be a BT-coexistence problem. If not, I feel CPU becomes exhausted or huge amount of interrupts at that time.
Quoted 2 lines
I don't know how to reproduce any of these.
If users want/can help to dig problem, please try above suggestions.
Ping-Ke Shih
maintainerQuoted 13 lines · Bitterblue Smith
drivers/net/wireless/realtek/rtlwifi/pci.c +1
-1@@ -520,7 +520,7 @@ static void _rtl_pci_tx_isr(struct ieee80211_hw *hw, int prio)rtl_tx_ackqueue (hw, skb); } - if ((ring->entries - skb_queue_len(&ring->queue)) <= 4) { + if ((ring->entries - skb_queue_len(&ring->queue)) >= 4) { rtl_dbg(rtlpriv, COMP_ERR, DBG_DMESG, "more desc left, wake skb_queue@%d, ring->idx = %d, skb_queue_len = 0x%x\n",
Quoted 1 line
prio, ring->idx,
Reading the code about calling ieee80211_stop_queue() and ieee80211_wake_queue() again. It looks like
In TX path (rtl_pci_tx):
if (remaining < 2)
ieee80211_stop_queue();
In TX completion path (_rtl_pci_tx_isr):
if (remaining <= 4)
ieee80211_wake_queue();I guess it uses "<= 4" in TX completion path because origin patch doesn't want to call ieee80211_wake_queue() redundantly, but looks not so reasonable. The better way may use ieee80211_queue_stopped() instead.