ath10k: tx flow control fixes
cover ath10k: tx flow control fixes
This addresses some tx failed issues and minor
issues I've found along.
This is a resend as a PATCH rather than an RFC.
Updates include:
* added ath10k: detect htt pending tx limit at runtime
* dropped htc tx polling with a timer patch
* dropped explicit htt tx polling from
ath10k: introduce proper htt tx flow control (this was deadlocking, and is no longer necessary anyway)
Michal Kazior (7):
ath10k: change errno if we run out of msdu_ids
ath10k: ath10k_htc_prepare_tx_skb() never fails
ath10k: add lockdep asserts to htc skb dequeuing
ath10k: simplify htc flow control
ath10k: remove unused queue limit
ath10k: introduce proper htt tx flow control
ath10k: detect htt pending tx limit at runtime
drivers/net/wireless/ath/ath10k/htc.c | 106 +++++++----------------------- drivers/net/wireless/ath/ath10k/htc.h | 4 -- drivers/net/wireless/ath/ath10k/htt.c | 25 ++----- drivers/net/wireless/ath/ath10k/htt.h | 26 ++------ drivers/net/wireless/ath/ath10k/htt_tx.c | 83 +++++++++++++++++++++-- drivers/net/wireless/ath/ath10k/mac.c | 2 +- drivers/net/wireless/ath/ath10k/txrx.c | 5 +- 7 files changed, 116 insertions(+), 135 deletions(-)
Signature · 1 line
1.7.9.5
PATCH 1/7 ath10k: change errno if we run out of msdu_ids
Since these are limited resources not strictly by
memory we should not report it as such. It's also
easier to distinguish tx failed warnings this
way.
--- drivers/net/wireless/ath/ath10k/htt_tx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/wireless/ath/ath10k/htt_tx.c +1 -1
@@ -30,7 +30,7 @@ int ath10k_htt_tx_alloc_msdu_id(struct ath10k_htt *htt) msdu_id =find_first_zero_bit (htt->used_msdu_ids,HTT_MAX_NUM_PENDING_TX );if (msdu_id ==HTT_MAX_NUM_PENDING_TX ) -return -ENOMEM ; +return -ENOBUFS ;ath10k_dbg (ATH10K_DBG_HTT ,"htt tx alloc msdu_id %d\n" , msdu_id);__set_bit (msdu_id, htt->used_msdu_ids);
Signature · 1 line
1.7.9.5
PATCH 2/7 ath10k: ath10k_htc_prepare_tx_skb() never fails
In reply to Michal KaziorIt shouldn't report any result.ath10k_htc_restore_tx_skb() assumesath10k_htc_prepare_tx_skb() always succeeds. Ifath10k_htc_prepare_tx_skb() was to fail we'd
errornously skb_pull(). This patch is for clarity
and sanity.
--- drivers/net/wireless/ath/ath10k/htc.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-)
drivers/net/wireless/ath/ath10k/htc.c +3 -7
@@ -139,8 +139,8 @@ static bool ath10k_htc_ep_need_credit_update(struct ath10k_htc_ep *ep)return true; } -static int ath10k_htc_prepare_tx_skb (struct ath10k_htc_ep *ep, -struct sk_buff *skb) +static void ath10k_htc_prepare_tx_skb (struct ath10k_htc_ep *ep, +struct sk_buff *skb) {struct ath10k_htc_hdr *hdr;
@@ -157,8 +157,6 @@ static int ath10k_htc_prepare_tx_skb(struct ath10k_htc_ep *ep, hdr->flags |=ATH10K_HTC_FLAG_NEED_CREDIT_UPDATE ;spin_unlock_bh (&ep->htc->tx_lock); - -return 0 ; } static int ath10k_htc_issue_skb(struct ath10k_htc *htc,
@@ -172,9 +170,7 @@ static int ath10k_htc_issue_skb(struct ath10k_htc *htc,ath10k_dbg (ATH10K_DBG_HTC ,"%s: ep %d skb %p\n" , __func__, ep->eid, skb); - ret =ath10k_htc_prepare_tx_skb (ep, skb); -if (ret) - goto err; +ath10k_htc_prepare_tx_skb (ep, skb); ret =ath10k_skb_map (htc->ar->dev, skb);if (ret)
Signature · 1 line
1.7.9.5
PATCH 3/7 ath10k: add lockdep asserts to htc skb dequeuing
In reply to Michal Kazior--- drivers/net/wireless/ath/ath10k/htc.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/net/wireless/ath/ath10k/htc.c +4 -2
@@ -198,7 +198,6 @@ err:return ret; } -/* assumes tx_lock is held */ static struct sk_buff *ath10k_htc_get_skb_credit_based (struct ath10k_htc *htc,struct ath10k_htc_ep *ep,u8 *credits)
@@ -209,6 +208,8 @@ static struct sk_buff *ath10k_htc_get_skb_credit_based(struct ath10k_htc *htc,int remainder;unsigned int transfer_len; +lockdep_assert_held (&htc->tx_lock); + skb =__skb_dequeue (&ep->tx_queue);if (!skb)return NULL ;
@@ -241,7 +242,6 @@ static struct sk_buff *ath10k_htc_get_skb_credit_based(struct ath10k_htc *htc,return skb; } -/* assumes tx_lock is held */ static struct sk_buff *ath10k_htc_get_skb (struct ath10k_htc *htc,struct ath10k_htc_ep *ep,int resources)
@@ -249,6 +249,8 @@ static struct sk_buff *ath10k_htc_get_skb(struct ath10k_htc *htc,struct sk_buff *skb;struct ath10k_skb_cb *skb_cb; +lockdep_assert_held (&htc->tx_lock); +if (!resources)return NULL ;
Signature · 1 line
1.7.9.5
PATCH 4/7 ath10k: simplify htc flow control
In reply to Michal KaziorCredit-based flow is designed around HTC, and HTC is supposed to manage the credit count and reservation.
For non credit-based endpoints we must depend on the HIF pipe tx resources. This is CE ringbuffer size/usage for PCI backend. This is not managed by HTC but by the HIF. We could mirror the tx_resources and sync it in HTC and perform mirror-reservation in HTC but it doesn't really make much sense. It's far simpler to reinsert skb if we're out of resources.
Queue control in HTC is ineffective and does virtually nothing. This will be replaced in the future.
--- drivers/net/wireless/ath/ath10k/htc.c | 94 ++++++--------------------------- drivers/net/wireless/ath/ath10k/htc.h | 4 -- drivers/net/wireless/ath/ath10k/htt.c | 12 ----- 3 files changed, 17 insertions(+), 93 deletions(-)
drivers/net/wireless/ath/ath10k/htc.c +17 -77
@@ -23,48 +23,6 @@/* Send */ /********/ -static inline void ath10k_htc_stop_queue (struct ath10k_htc_ep *ep) -{ -if (ep->tx_queue_stopped) -return ; - -if (ep->ep_ops.stop_queue) - ep->ep_ops.stop_queue (ep->htc->ar); - -ath10k_dbg (ATH10K_DBG_HTC ,"ep %d stop\n" , ep->eid); - ep->tx_queue_stopped = true; -} - -static inline void ath10k_htc_wake_queue (struct ath10k_htc_ep *ep) -{ -if (!ep->tx_queue_stopped) -return ; - -if (ep->ep_ops.wake_queue) - ep->ep_ops.wake_queue (ep->htc->ar); - -ath10k_dbg (ATH10K_DBG_HTC ,"ep %d wake\n" , ep->eid); - ep->tx_queue_stopped = false; -} - -static inline void ath10k_htc_recalc_queue (struct ath10k_htc_ep *ep,int delta) -{ -ath10k_dbg (ATH10K_DBG_HTC ,"ep %d queue len %d +%d max %d\n" , - ep->eid, ep->tx_queue_len, delta, ep->max_tx_queue_depth); - -if (ep->max_tx_queue_depth ==0 ) -return ; - - ep->tx_queue_len += delta; - -if (ep->tx_queue_stopped) { -if (ep->tx_queue_len <= ep->max_tx_queue_depth/2 ) -ath10k_htc_wake_queue (ep); - }else if (ep->tx_queue_len >= ep->max_tx_queue_depth) { -ath10k_htc_stop_queue (ep); - } -} -static inline void ath10k_htc_send_complete_check (struct ath10k_htc_ep *ep,int force) {
@@ -192,6 +150,17 @@ err: ep->tx_credits += credits;spin_unlock_bh (&htc->tx_lock); +/* this is the simplest way to handle out-of-resources for non-credit +* based endpoints. credit based endpoints can still get -ENOSR, but +* this is highly unlikely as credit reservation should prevent that */ +if (ret == -ENOSR ) { +spin_lock_bh (&htc->tx_lock); +__skb_queue_head (&ep->tx_queue, skb); +spin_unlock_bh (&htc->tx_lock); + +return ret; + } + skb_cb->is_aborted = true;ath10k_htc_notify_tx_completion (ep, skb);
@@ -233,7 +202,6 @@ static struct sk_buff *ath10k_htc_get_skb_credit_based(struct ath10k_htc *htc,if (ep->tx_credits < credits_required) {__skb_queue_head (&ep->tx_queue, skb); -ath10k_htc_recalc_queue (ep,1 );return NULL ; }
@@ -242,59 +210,33 @@ static struct sk_buff *ath10k_htc_get_skb_credit_based(struct ath10k_htc *htc,return skb; } -static struct sk_buff *ath10k_htc_get_skb (struct ath10k_htc *htc, -struct ath10k_htc_ep *ep, -int resources) -{ -struct sk_buff *skb; -struct ath10k_skb_cb *skb_cb; - -lockdep_assert_held (&htc->tx_lock); - -if (!resources) -return NULL ; - - skb =__skb_dequeue (&ep->tx_queue); -if (!skb) -return NULL ; - - skb_cb =ATH10K_SKB_CB (skb); -ath10k_htc_recalc_queue (ep,-1 ); - -return skb; -} -static void ath10k_htc_send_work (struct work_struct *work) {struct ath10k_htc_ep *ep =container_of (work, struct ath10k_htc_ep, send_work);struct ath10k_htc *htc = ep->htc;struct sk_buff *skb; -int tx_resources =0 ;u8 credits =0 ; +int ret;while (true) { -if (!ep->tx_credit_flow_enabled) - tx_resources =ath10k_hif_get_free_queue_number - (htc->ar, ep->ul_pipe_id); -if (ep->ul_is_polled)ath10k_htc_send_complete_check (ep,0 );spin_lock_bh (&htc->tx_lock); -if (ep->tx_credit_flow_enabled) skb =ath10k_htc_get_skb_credit_based (htc, ep, &credits);else - skb =ath10k_htc_get_skb (htc, ep, tx_resources); - + skb =__skb_dequeue (&ep->tx_queue);spin_unlock_bh (&htc->tx_lock);if (!skb) -break ;/* tx_queue empty or out of resources */ +break ; -ath10k_htc_issue_skb (htc, ep, skb, credits); + ret =ath10k_htc_issue_skb (htc, ep, skb, credits); +if (ret == -ENOSR ) +break ; } }
@@ -313,7 +255,6 @@ int ath10k_htc_send(struct ath10k_htc *htc,spin_lock_bh (&htc->tx_lock);__skb_queue_tail (&ep->tx_queue, skb); -ath10k_htc_recalc_queue (ep,1 );spin_unlock_bh (&htc->tx_lock);queue_work (htc->ar->workqueue, &ep->send_work);
@@ -658,7 +599,6 @@ static void ath10k_htc_reset_endpoint_states(struct ath10k_htc *htc) ep->max_tx_queue_depth =0 ; ep->eid = i;skb_queue_head_init (&ep->tx_queue); - ep->tx_queue_len =0 ; ep->htc = htc; ep->tx_credit_flow_enabled = true;INIT_WORK (&ep->send_work, ath10k_htc_send_work);
drivers/net/wireless/ath/ath10k/htc.h +0 -4
@@ -276,8 +276,6 @@ struct ath10k_htc_ops {struct ath10k_htc_ep_ops {void (*ep_tx_complete)(struct ath10k *,struct sk_buff *);void (*ep_rx_complete)(struct ath10k *,struct sk_buff *); -void (*stop_queue)(struct ath10k *); -void (*wake_queue)(struct ath10k *); };/* service connection information */
@@ -318,8 +316,6 @@ struct ath10k_htc_ep {int dl_is_polled;/* call HIF to fetch rx (not implemented) */ struct sk_buff_head tx_queue; -int tx_queue_len; -bool tx_queue_stopped;u8 seq_no;/* for debugging */ int tx_credits;
drivers/net/wireless/ath/ath10k/htt.c +0 -12
@@ -21,16 +21,6 @@#include "core.h" #include "debug.h" -static void ath10k_htt_stop_queue (struct ath10k *ar) -{ -ieee80211_stop_queues (ar->hw); -} - -static void ath10k_htt_wake_queue (struct ath10k *ar) -{ -ieee80211_wake_queues (ar->hw); -} -static int ath10k_htt_htc_attach (struct ath10k_htt *htt) {struct ath10k_htc_svc_conn_req conn_req;
@@ -42,8 +32,6 @@ static int ath10k_htt_htc_attach(struct ath10k_htt *htt) conn_req.ep_ops.ep_tx_complete = ath10k_htt_htc_tx_complete; conn_req.ep_ops.ep_rx_complete = ath10k_htt_t2h_msg_handler; - conn_req.ep_ops.stop_queue = ath10k_htt_stop_queue; - conn_req.ep_ops.wake_queue = ath10k_htt_wake_queue;/* * Specify how deep to let a queue get before ath10k_htc_send will
Signature · 1 line
1.7.9.5
PATCH 5/7 ath10k: remove unused queue limit
In reply to Michal KaziorHTT isn't credit-based and we don't need to have
an artificial limit here.
--- drivers/net/wireless/ath/ath10k/htt.c | 6 ------ drivers/net/wireless/ath/ath10k/htt.h | 15 --------------- 2 files changed, 21 deletions(-)
drivers/net/wireless/ath/ath10k/htt.c +0 -6
@@ -33,12 +33,6 @@ static int ath10k_htt_htc_attach(struct ath10k_htt *htt) conn_req.ep_ops.ep_tx_complete = ath10k_htt_htc_tx_complete; conn_req.ep_ops.ep_rx_complete = ath10k_htt_t2h_msg_handler; -/* -* Specify how deep to let a queue get before ath10k_htc_send will -* call the ep_send_full function due to excessive send queue depth. -*/ - conn_req.max_send_queue_depth =HTT_MAX_SEND_QUEUE_DEPTH ; -/* connect to control service */ conn_req.service_id =ATH10K_HTC_SVC_ID_HTT_DATA_MSG ;
drivers/net/wireless/ath/ath10k/htt.h +0 -15
@@ -1302,21 +1302,6 @@ struct htt_rx_desc {#define HTT_MAC_ADDR_LEN 6/* -* HTT_MAX_SEND_QUEUE_DEPTH - -* How many packets HTC should allow to accumulate in a send queue -* before calling the ep_send_full callback to see whether to retain -* or drop packets. -* This is not relevant for LL, where tx descriptors should be immediately -* downloaded to the target. -* This is not very relevant for HL either, since it is anticipated that -* the HL tx download scheduler will not work this far in advance - rather, -* it will make its decisions just-in-time, so it can be responsive to -* changing conditions. -* Hence, this queue depth threshold spec is mostly just a formality. -*/ -#define HTT_MAX_SEND_QUEUE_DEPTH (HTT_MAX_NUM_PENDING_TX) - -/* * FIX THIS * Should be: sizeof(struct htt_host_rx_desc) + max rx MSDU size, * rounded up to a cache line size.
Signature · 1 line
1.7.9.5
PATCH 6/7 ath10k: introduce proper htt tx flow control
In reply to Michal KaziorThere were cases where we'd run out of msdu_ids. This can be fixed by telling mac80211 to stop sending us more frames if we run out of resources.
It was possible to trigger with massive TCP RX
~300mbps (e.g. using iperf).
--- drivers/net/wireless/ath/ath10k/htt.h | 2 ++ drivers/net/wireless/ath/ath10k/htt_tx.c | 46 ++++++++++++++++++++++++++++++ drivers/net/wireless/ath/ath10k/txrx.c | 1 + 3 files changed, 49 insertions(+)
drivers/net/wireless/ath/ath10k/htt.h +2 -0
@@ -1262,6 +1262,7 @@ struct ath10k_htt {/* Protects access to %pending_tx, %used_msdu_ids */ spinlock_t tx_lock; +int num_pending_tx;struct sk_buff *pending_tx[HTT_MAX_NUM_PENDING_TX ];DECLARE_BITMAP (used_msdu_ids,HTT_MAX_NUM_PENDING_TX );wait_queue_head_t empty_tx_wq;
@@ -1330,6 +1331,7 @@ void ath10k_htt_t2h_msg_handler(struct ath10k *ar, struct sk_buff *skb);int ath10k_htt_h2t_ver_req_msg (struct ath10k_htt *htt);int ath10k_htt_send_rx_ring_cfg_ll (struct ath10k_htt *htt); +void __ath10k_htt_tx_dec_pending (struct ath10k_htt *htt);int ath10k_htt_tx_alloc_msdu_id (struct ath10k_htt *htt);void ath10k_htt_tx_free_msdu_id (struct ath10k_htt *htt,u16 msdu_id);int ath10k_htt_mgmt_tx (struct ath10k_htt *htt,struct sk_buff *);
drivers/net/wireless/ath/ath10k/htt_tx.c +46 -0
79 lines not on this page · raw message
drivers/net/wireless/ath/ath10k/txrx.c +1 -0
@@ -95,6 +95,7 @@ exit:spin_lock_bh (&htt->tx_lock); htt->pending_tx[ATH10K_SKB_CB (txdesc)->htt.msdu_id] =NULL ;ath10k_htt_tx_free_msdu_id (htt,ATH10K_SKB_CB (txdesc)->htt.msdu_id); +__ath10k_htt_tx_dec_pending (htt);if (bitmap_empty (htt->used_msdu_ids,HTT_MAX_NUM_PENDING_TX ))wake_up (&htt->empty_tx_wq);spin_unlock_bh (&htt->tx_lock);
Signature · 1 line
1.7.9.5
PATCH 6/7 ath10k: introduce proper htt tx flow control
Quoted 8 lines · Michal Kazior
There were cases where we'd run out of
msdu_ids. This can be fixed by telling mac80211 to stop sending us more frames if we run out of resources.It was possible to trigger with massive TCP RX
~300mbps (e.g. using iperf).Signed-off-by Michal Kazior <[email protected]>
[...]
drivers/net/wireless/ath/ath10k/htt_tx.c line 30
Quoted 6 lines
drivers/net/wireless/ath/ath10k/htt_tx.c +6
-0+void __ath10k_htt_tx_dec_pending (struct ath10k_htt *htt) +{ + htt->num_pending_tx--; +if (htt->num_pending_tx ==HTT_MAX_NUM_PENDING_TX -1 ) +ieee80211_wake_queues (htt->ar->hw); +}
How often is this function called? I'm wondering if it should be an inline function.
But that can be done in a followup patch (if needed).
Signature · 1 line
Kalle Valo
PATCH 6/7 ath10k: introduce proper htt tx flow control
Quoted 23 lines · Kalle Valo
Quoted 8 lines · Michal Kazior
There were cases where we'd run out of
msdu_ids. This can be fixed by telling mac80211 to stop sending us more frames if we run out of resources.It was possible to trigger with massive TCP RX
~300mbps (e.g. using iperf).Signed-off-by Michal Kazior <[email protected]>[...]
drivers/net/wireless/ath/ath10k/htt_tx.c line 30
Quoted 6 lines
drivers/net/wireless/ath/ath10k/htt_tx.c +6
-0+void __ath10k_htt_tx_dec_pending (struct ath10k_htt *htt) +{ + htt->num_pending_tx--; +if (htt->num_pending_tx ==HTT_MAX_NUM_PENDING_TX -1 ) +ieee80211_wake_queues (htt->ar->hw); +}How often is this function called? I'm wondering if it should be an inline function.
Quoted 1 line
But that can be done in a followup patch (if needed).
It's called after each tx completion, so it might be a good idea to make it an inline function.
-- Pozdrawiam / Best regards, Michal Kazior.
PATCH 7/7 ath10k: detect htt pending tx limit at runtime
In reply to Michal KaziorThe real limit for sending htt tx is either msdu_id storage type (u16 - 65536 different values) or the HIF tx pipe queue length (2047 in case of our PCI HIF).
The htt tx pipe does not use interrupts - it must be polled. It is polled either on RX unconditionally or on TX if HIF tx pipe has used over 50% of the resources.
With this patch we should finally trigger the TX polling properly. What this really means we should have a smoother tx. Not because the tx limit is greater, but simply because we'll be triggering the polling properly in case of heavy tx.
--- drivers/net/wireless/ath/ath10k/htt.c | 7 ++++- drivers/net/wireless/ath/ath10k/htt.h | 9 +++---- drivers/net/wireless/ath/ath10k/htt_tx.c | 41 +++++++++++++++++++++++++----- drivers/net/wireless/ath/ath10k/mac.c | 2 +- drivers/net/wireless/ath/ath10k/txrx.c | 4 +-- 5 files changed, 47 insertions(+), 16 deletions(-)
drivers/net/wireless/ath/ath10k/htt.c +6 -1
@@ -50,6 +50,7 @@ static int ath10k_htt_htc_attach(struct ath10k_htt *htt)struct ath10k_htt *ath10k_htt_attach (struct ath10k *ar) {struct ath10k_htt *htt; +int ret; htt =kzalloc (sizeof (*htt),GFP_KERNEL );if (!htt)
@@ -67,7 +68,11 @@ struct ath10k_htt *ath10k_htt_attach(struct ath10k *ar)if (ath10k_htt_htc_attach (htt)) goto err_htc_attach; -ath10k_htt_tx_attach (htt); + ret =ath10k_htt_tx_attach (htt); +if (ret) { +ath10k_err ("could not attach htt tx (%d)\n" , ret); + goto err_htc_attach; + }if (ath10k_htt_rx_attach (htt)) goto err_rx_attach;
drivers/net/wireless/ath/ath10k/htt.h +4 -5
@@ -1184,8 +1184,6 @@ struct htt_rx_info {bool fcs_err; }; -#define HTT_MAX_NUM_PENDING_TX 512/* FIXME: find proper value? */ -struct ath10k_htt {struct ath10k *ar;enum ath10k_htc_ep_id eid;
@@ -1262,9 +1260,10 @@ struct ath10k_htt {/* Protects access to %pending_tx, %used_msdu_ids */ spinlock_t tx_lock; +int max_num_pending_tx;int num_pending_tx; -struct sk_buff *pending_tx[HTT_MAX_NUM_PENDING_TX ]; -DECLARE_BITMAP (used_msdu_ids,HTT_MAX_NUM_PENDING_TX ); +struct sk_buff **pending_tx; +unsigned long *used_msdu_ids;/* bitmap */ wait_queue_head_t empty_tx_wq;/* set if host-fw communication goes haywire
@@ -1322,7 +1321,7 @@ struct ath10k_htt *ath10k_htt_attach(struct ath10k *ar);int ath10k_htt_attach_target (struct ath10k_htt *htt);void ath10k_htt_detach (struct ath10k_htt *htt); -void ath10k_htt_tx_attach (struct ath10k_htt *htt); +int ath10k_htt_tx_attach (struct ath10k_htt *htt);void ath10k_htt_tx_detach (struct ath10k_htt *htt);int ath10k_htt_rx_attach (struct ath10k_htt *htt);void ath10k_htt_rx_detach (struct ath10k_htt *htt);
drivers/net/wireless/ath/ath10k/htt_tx.c +34 -7
85 lines not on this page · raw message
drivers/net/wireless/ath/ath10k/mac.c +1 -1
@@ -2582,7 +2582,7 @@ static void ath10k_flush(struct ieee80211_hw *hw, u32 queues, bool drop)bool empty;spin_lock_bh (&ar->htt->tx_lock); empty =bitmap_empty (ar->htt->used_msdu_ids, -HTT_MAX_NUM_PENDING_TX ); + ar->htt->max_num_pending_tx);spin_unlock_bh (&ar->htt->tx_lock); (empty); }), ATH10K_FLUSH_TIMEOUT_HZ);
drivers/net/wireless/ath/ath10k/txrx.c +2 -2
@@ -96,7 +96,7 @@ exit: htt->pending_tx[ATH10K_SKB_CB (txdesc)->htt.msdu_id] =NULL ;ath10k_htt_tx_free_msdu_id (htt,ATH10K_SKB_CB (txdesc)->htt.msdu_id);__ath10k_htt_tx_dec_pending (htt); -if (bitmap_empty (htt->used_msdu_ids,HTT_MAX_NUM_PENDING_TX )) +if (bitmap_empty (htt->used_msdu_ids, htt->max_num_pending_tx))wake_up (&htt->empty_tx_wq);spin_unlock_bh (&htt->tx_lock);
@@ -111,7 +111,7 @@ void ath10k_txrx_tx_completed(struct ath10k_htt *htt,ath10k_dbg (ATH10K_DBG_HTT ,"htt tx completion msdu_id %u discard %d no_ack %d\n" , tx_done->msdu_id, !!tx_done->discard, !!tx_done->no_ack); -if (tx_done->msdu_id >=ARRAY_SIZE (htt->pending_tx)) { +if (tx_done->msdu_id >= htt->max_num_pending_tx) {ath10k_warn ("warning: msdu_id %d too big, ignoring\n" , tx_done->msdu_id);return ;
Signature · 1 line
1.7.9.5
PATCH 7/7 ath10k: detect htt pending tx limit at runtime
Quoted 10 lines · Michal Kazior
The real limit for sending htt tx is either
msdu_idstorage type (u16 - 65536 different values) or the HIF tx pipe queue length (2047 in case of our PCI HIF).The htt tx pipe does not use interrupts - it must be polled. It is polled either on RX unconditionally or on TX if HIF tx pipe has used over 50% of the resources.
Quoted 7 lines
With this patch we should finally trigger the TX polling properly. What this really means we should have a smoother tx. Not because the tx limit is greater, but simply because we'll be triggering the polling properly in case of heavy tx.
Signed-off-by Michal Kazior <[email protected]>
I think a queue length of 2047 is completely excessive. It causes way too much buffering and latency under load, and I suspect it may be significantly hurting TCP throughput as well (if TCP manages to get the queue filled up).
As long as there is no way to dynamically determine a *reasonable* queue size, introducing an arbitrary limit is *much* better than just letting the firmware gobble up as much as it wants.
For reference: ath9k limits the number of pending tx packets to 128 per WMM queue, and even at the highest MCS rates with 3x3 and HT40 this is much more than what's actually needed.
- Felix
cover ath10k: tx flow control fixes
In reply to Michal KaziorQuoted 12 lines · Michal Kazior
This addresses some
tx failedissues and minor
issues I've found along.This is a resend as a PATCH rather than an RFC.
Updates include:
* addedath10k: detect htt pending tx limit at runtime
* dropped htc tx polling with a timer patch
* dropped explicit htt tx polling from
ath10k: introduce proper htt tx flow control(this was deadlocking, and is no longer necessary anyway)
Quoted 8 lines
Michal Kazior (7):
ath10k: change errno if we run out ofmsdu_ids
ath10k:ath10k_htc_prepare_tx_skb()never fails
ath10k: add lockdep asserts to htc skb dequeuing
ath10k: simplify htc flow control
ath10k: remove unused queue limit
ath10k: introduce proper htt tx flow control
ath10k: detect htt pending tx limit at runtime
Thanks, all patches applied.
Signature · 1 line
Kalle Valo
cover ath10k: tx flow control fixes
Quoted 23 lines · Kalle Valo
Quoted 12 lines · Michal Kazior
This addresses some
tx failedissues and minor
issues I've found along.This is a resend as a PATCH rather than an RFC.
Updates include:
* addedath10k: detect htt pending tx limit at runtime
* dropped htc tx polling with a timer patch
* dropped explicit htt tx polling from
ath10k: introduce proper htt tx flow control(this was deadlocking, and is no longer necessary anyway)Quoted 8 lines
Michal Kazior (7):
ath10k: change errno if we run out ofmsdu_ids
ath10k:ath10k_htc_prepare_tx_skb()never fails
ath10k: add lockdep asserts to htc skb dequeuing
ath10k: simplify htc flow control
ath10k: remove unused queue limit
ath10k: introduce proper htt tx flow control
ath10k: detect htt pending tx limit at runtime
Quoted 1 line
Thanks, all patches applied.
I got a new warning, can you please send a followup patch for that:
drivers/net/wireless/ath/ath10k/htt_tx.c:96: WARNING: line over 80 characters
Signature · 1 line
Kalle Valo
PATCH 1/1 ath10k: fix sparse warning
The line was over 80 characters long.
--- drivers/net/wireless/ath/ath10k/htt_tx.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/net/wireless/ath/ath10k/htt_tx.c +2 -1
@@ -93,7 +93,8 @@ int ath10k_htt_tx_attach(struct ath10k_htt *htt)/* At the beginning free queue number should hint us the maximum * queue length */ pipe = htt->ar->htc->endpoint[htt->eid].ul_pipe_id; - htt->max_num_pending_tx =ath10k_hif_get_free_queue_number (htt->ar, pipe); + htt->max_num_pending_tx =ath10k_hif_get_free_queue_number (htt->ar, + pipe);ath10k_dbg (ATH10K_DBG_HTT ,"htt tx max num pending tx %d\n" , htt->max_num_pending_tx);
Signature · 1 line
1.7.9.5
Kalle Valo
AppliedPATCH 1/1 ath10k: fix sparse warning
Quoted 3 lines · Michal Kazior
The line was over 80 characters long.
Signed-off-by Michal Kazior <[email protected]>
Thanks, applied.
Signature · 1 line
Kalle Valo