2023-03-23 17:18:42

by Alexander Wetzel

[permalink] [raw]
Subject: [PATCH] wifi: ath10k: Serialize wake_tx_queue ops

Serialize the ath10k implementation of the wake_tx_queue ops.
ath10k_mac_op_wake_tx_queue() must not run concurrent since it's using
ieee80211_txq_schedule_start().

Fixes: bb2edb733586 ("ath10k: migrate to mac80211 txq scheduling")
Reported-by: Felix Fietkau <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
CC: <[email protected]>
Signed-off-by: Alexander Wetzel <[email protected]>
---

The intend of this patch is to sort out an issue discovered in the discussion
referred to by the Link tag.

I can't test it with real HW and thus just implemented the per-ac queue lock
Felix suggested. One obvious alternative to the per-ac lock would be to
bring back the txqs_lock commit bb2edb733586 ("ath10k: migrate to mac80211 txq
scheduling") dropped.

Alexander

---
drivers/net/wireless/ath/ath10k/core.c | 3 +++
drivers/net/wireless/ath/ath10k/core.h | 3 +++
drivers/net/wireless/ath/ath10k/mac.c | 6 ++++--
3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 5eb131ab916f..533ed7169e11 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -3643,6 +3643,9 @@ struct ath10k *ath10k_core_create(size_t priv_size, struct device *dev,
mutex_init(&ar->dump_mutex);
spin_lock_init(&ar->data_lock);

+ for (int ac = 0; ac < IEEE80211_NUM_ACS; ac++)
+ spin_lock_init(&ar->queue_lock[ac]);
+
INIT_LIST_HEAD(&ar->peers);
init_waitqueue_head(&ar->peer_mapping_wq);
init_waitqueue_head(&ar->htt.empty_tx_wq);
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index f5de8ce8fb45..4b5239de4018 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -1170,6 +1170,9 @@ struct ath10k {
/* protects shared structure data */
spinlock_t data_lock;

+ /* serialize wake_tx_queue calls per ac */
+ spinlock_t queue_lock[IEEE80211_NUM_ACS];
+
struct list_head arvifs;
struct list_head peers;
struct ath10k_peer *peer_map[ATH10K_MAX_NUM_PEER_IDS];
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 7675858f069b..9c4bf2fdbc0f 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -4732,13 +4732,14 @@ static void ath10k_mac_op_wake_tx_queue(struct ieee80211_hw *hw,
{
struct ath10k *ar = hw->priv;
int ret;
- u8 ac;
+ u8 ac = txq->ac;

ath10k_htt_tx_txq_update(hw, txq);
if (ar->htt.tx_q_state.mode != HTT_TX_MODE_SWITCH_PUSH)
return;

- ac = txq->ac;
+ spin_lock_bh(&ar->queue_lock[ac]);
+
ieee80211_txq_schedule_start(hw, ac);
txq = ieee80211_next_txq(hw, ac);
if (!txq)
@@ -4753,6 +4754,7 @@ static void ath10k_mac_op_wake_tx_queue(struct ieee80211_hw *hw,
ath10k_htt_tx_txq_update(hw, txq);
out:
ieee80211_txq_schedule_end(hw, ac);
+ spin_unlock_bh(&ar->queue_lock[ac]);
}

/* Must not be called with conf_mutex held as workers can use that also. */
--
2.40.0


2023-05-05 13:09:24

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] wifi: ath10k: Serialize wake_tx_queue ops

Alexander Wetzel <[email protected]> wrote:

> Serialize the ath10k implementation of the wake_tx_queue ops.
> ath10k_mac_op_wake_tx_queue() must not run concurrent since it's using
> ieee80211_txq_schedule_start().
>
> The intend of this patch is to sort out an issue discovered in the discussion
> referred to by the Link tag.
>
> I can't test it with real hardware and thus just implemented the per-ac queue
> lock Felix suggested. One obvious alternative to the per-ac lock would be to
> bring back the txqs_lock commit bb2edb733586 ("ath10k: migrate to mac80211 txq
> scheduling") dropped.
>
> Fixes: bb2edb733586 ("ath10k: migrate to mac80211 txq scheduling")
> Reported-by: Felix Fietkau <[email protected]>
> Link: https://lore.kernel.org/r/[email protected]
> CC: <[email protected]>
> Signed-off-by: Alexander Wetzel <[email protected]>
> Signed-off-by: Kalle Valo <[email protected]>

Patch applied to ath-next branch of ath.git, thanks.

b719ebc37a1e wifi: ath10k: Serialize wake_tx_queue ops

--
https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches