Skip to content

Commit f2ed1b3

Browse files
miaoqing-panksacilotto
authored andcommitted
ath10k: fix wmi mgmt tx queue full due to race condition
BugLink: https://bugs.launchpad.net/bugs/1920235 [ Upstream commit b55379e ] Failed to transmit wmi management frames: [84977.840894] ath10k_snoc a000000.wifi: wmi mgmt tx queue is full [84977.840913] ath10k_snoc a000000.wifi: failed to transmit packet, dropping: -28 [84977.840924] ath10k_snoc a000000.wifi: failed to submit frame: -28 [84977.840932] ath10k_snoc a000000.wifi: failed to transmit frame: -28 This issue is caused by race condition between skb_dequeue and __skb_queue_tail. The queue of ‘wmi_mgmt_tx_queue’ is protected by a different lock: ar->data_lock vs list->lock, the result is no protection. So when ath10k_mgmt_over_wmi_tx_work() and ath10k_mac_tx_wmi_mgmt() running concurrently on different CPUs, there appear to be a rare corner cases when the queue length is 1, CPUx (skb_deuque) CPUy (__skb_queue_tail) next=list prev=list struct sk_buff *skb = skb_peek(list); WRITE_ONCE(newsk->next, next); WRITE_ONCE(list->qlen, list->qlen - 1);WRITE_ONCE(newsk->prev, prev); next = skb->next; WRITE_ONCE(next->prev, newsk); prev = skb->prev; WRITE_ONCE(prev->next, newsk); skb->next = skb->prev = NULL; list->qlen++; WRITE_ONCE(next->prev, prev); WRITE_ONCE(prev->next, next); If the instruction ‘next = skb->next’ is executed before ‘WRITE_ONCE(prev->next, newsk)’, newsk will be lost, as CPUx get the old ‘next’ pointer, but the length is still added by one. The final result is the length of the queue will reach the maximum value but the queue is empty. So remove ar->data_lock, and use 'skb_queue_tail' instead of '__skb_queue_tail' to prevent the potential race condition. Also switch to use skb_queue_len_lockless, in case we queue a few SKBs simultaneously. Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1.c2-00033-QCAHLSWMTPLZ-1 Signed-off-by: Miaoqing Pan <miaoqing@codeaurora.org> Reviewed-by: Brian Norris <briannorris@chromium.org> Signed-off-by: Kalle Valo <kvalo@codeaurora.org> Link: https://lore.kernel.org/r/1608618887-8857-1-git-send-email-miaoqing@codeaurora.org Signed-off-by: Sasha Levin <sashal@kernel.org> Signed-off-by: Kamal Mostafa <kamal@canonical.com> Signed-off-by: Kelsey Skunberg <kelsey.skunberg@canonical.com>
1 parent 15ceaae commit f2ed1b3

File tree

1 file changed

+4
-11
lines changed
  • drivers/net/wireless/ath/ath10k

1 file changed

+4
-11
lines changed

drivers/net/wireless/ath/ath10k/mac.c

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3625,23 +3625,16 @@ bool ath10k_mac_tx_frm_has_freq(struct ath10k *ar)
36253625
static int ath10k_mac_tx_wmi_mgmt(struct ath10k *ar, struct sk_buff *skb)
36263626
{
36273627
struct sk_buff_head *q = &ar->wmi_mgmt_tx_queue;
3628-
int ret = 0;
3629-
3630-
spin_lock_bh(&ar->data_lock);
36313628

3632-
if (skb_queue_len(q) == ATH10K_MAX_NUM_MGMT_PENDING) {
3629+
if (skb_queue_len_lockless(q) >= ATH10K_MAX_NUM_MGMT_PENDING) {
36333630
ath10k_warn(ar, "wmi mgmt tx queue is full\n");
3634-
ret = -ENOSPC;
3635-
goto unlock;
3631+
return -ENOSPC;
36363632
}
36373633

3638-
__skb_queue_tail(q, skb);
3634+
skb_queue_tail(q, skb);
36393635
ieee80211_queue_work(ar->hw, &ar->wmi_mgmt_tx_work);
36403636

3641-
unlock:
3642-
spin_unlock_bh(&ar->data_lock);
3643-
3644-
return ret;
3637+
return 0;
36453638
}
36463639

36473640
static enum ath10k_mac_tx_path

0 commit comments

Comments
 (0)