* [PATCH 1/7] ath10k: simplify HTC credits calculation
2013-09-10 13:49 [PATCH 0/7] ath10k: improve TX path Michal Kazior
@ 2013-09-10 13:49 ` Michal Kazior
2013-09-10 14:14 ` Bob Copeland
2013-09-10 13:49 ` [PATCH 2/7] ath10k: add HTC TX credits replenishing notification Michal Kazior
` (7 subsequent siblings)
8 siblings, 1 reply; 21+ messages in thread
From: Michal Kazior @ 2013-09-10 13:49 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless, Michal Kazior
Credit calculation was overly complex
unnecessarily. Now skb dequeing is more unified.
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
drivers/net/wireless/ath/ath10k/htc.c | 59 ++++++---------------------------
1 file changed, 11 insertions(+), 48 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/htc.c b/drivers/net/wireless/ath/ath10k/htc.c
index 7d445d3..65c6e0c 100644
--- a/drivers/net/wireless/ath/ath10k/htc.c
+++ b/drivers/net/wireless/ath/ath10k/htc.c
@@ -167,49 +167,6 @@ err:
return ret;
}
-static struct sk_buff *ath10k_htc_get_skb_credit_based(struct ath10k_htc *htc,
- struct ath10k_htc_ep *ep,
- u8 *credits)
-{
- struct sk_buff *skb;
- struct ath10k_skb_cb *skb_cb;
- int credits_required;
- int remainder;
- unsigned int transfer_len;
-
- lockdep_assert_held(&htc->tx_lock);
-
- skb = __skb_dequeue(&ep->tx_queue);
- if (!skb)
- return NULL;
-
- skb_cb = ATH10K_SKB_CB(skb);
- transfer_len = skb->len;
-
- if (likely(transfer_len <= htc->target_credit_size)) {
- credits_required = 1;
- } else {
- /* figure out how many credits this message requires */
- credits_required = transfer_len / htc->target_credit_size;
- remainder = transfer_len % htc->target_credit_size;
-
- if (remainder)
- credits_required++;
- }
-
- ath10k_dbg(ATH10K_DBG_HTC, "Credits required %d got %d\n",
- credits_required, ep->tx_credits);
-
- if (ep->tx_credits < credits_required) {
- __skb_queue_head(&ep->tx_queue, skb);
- return NULL;
- }
-
- ep->tx_credits -= credits_required;
- *credits = credits_required;
- return skb;
-}
-
static void ath10k_htc_send_work(struct work_struct *work)
{
struct ath10k_htc_ep *ep = container_of(work,
@@ -224,11 +181,17 @@ static void ath10k_htc_send_work(struct work_struct *work)
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 = __skb_dequeue(&ep->tx_queue);
+ skb = __skb_dequeue(&ep->tx_queue);
+
+ if (ep->tx_credit_flow_enabled) {
+ /* integer division w/ round-up */
+ credits = (skb->len + htc->target_credit_size - 1) /
+ htc->target_credit_size;
+ if (ep->tx_credits < credits) {
+ __skb_queue_head(&ep->tx_queue, skb);
+ skb = NULL;
+ }
+ }
spin_unlock_bh(&htc->tx_lock);
if (!skb)
--
1.7.9.5
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 1/7] ath10k: simplify HTC credits calculation
2013-09-10 13:49 ` [PATCH 1/7] ath10k: simplify HTC credits calculation Michal Kazior
@ 2013-09-10 14:14 ` Bob Copeland
2013-09-11 5:06 ` Michal Kazior
0 siblings, 1 reply; 21+ messages in thread
From: Bob Copeland @ 2013-09-10 14:14 UTC (permalink / raw)
To: Michal Kazior; +Cc: ath10k, linux-wireless
On Tue, Sep 10, 2013 at 03:49:57PM +0200, Michal Kazior wrote:
> + skb = __skb_dequeue(&ep->tx_queue);
> +
> + if (ep->tx_credit_flow_enabled) {
> + /* integer division w/ round-up */
> + credits = (skb->len + htc->target_credit_size - 1) /
> + htc->target_credit_size;
There's a macro for that...
credits = DIV_ROUND_UP(skb->len, htc->target_credit_size);
--
Bob Copeland %% www.bobcopeland.com
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 1/7] ath10k: simplify HTC credits calculation
2013-09-10 14:14 ` Bob Copeland
@ 2013-09-11 5:06 ` Michal Kazior
0 siblings, 0 replies; 21+ messages in thread
From: Michal Kazior @ 2013-09-11 5:06 UTC (permalink / raw)
To: Bob Copeland; +Cc: ath10k, linux-wireless
On 10 September 2013 16:14, Bob Copeland <me@bobcopeland.com> wrote:
> On Tue, Sep 10, 2013 at 03:49:57PM +0200, Michal Kazior wrote:
>> + skb = __skb_dequeue(&ep->tx_queue);
>> +
>> + if (ep->tx_credit_flow_enabled) {
>> + /* integer division w/ round-up */
>> + credits = (skb->len + htc->target_credit_size - 1) /
>> + htc->target_credit_size;
>
> There's a macro for that...
>
> credits = DIV_ROUND_UP(skb->len, htc->target_credit_size);
I didn't know that. Thanks!
Michał.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 2/7] ath10k: add HTC TX credits replenishing notification
2013-09-10 13:49 [PATCH 0/7] ath10k: improve TX path Michal Kazior
2013-09-10 13:49 ` [PATCH 1/7] ath10k: simplify HTC credits calculation Michal Kazior
@ 2013-09-10 13:49 ` Michal Kazior
2013-09-10 13:49 ` [PATCH 3/7] ath10k: make WMI commands block by design Michal Kazior
` (6 subsequent siblings)
8 siblings, 0 replies; 21+ messages in thread
From: Michal Kazior @ 2013-09-10 13:49 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless, Michal Kazior
This will allow higher layers to anticipate and
act upon TX credits renewal. This will be
important for some future rework of WMI command
submission.
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
drivers/net/wireless/ath/ath10k/htc.c | 9 +++++++++
drivers/net/wireless/ath/ath10k/htc.h | 1 +
2 files changed, 10 insertions(+)
diff --git a/drivers/net/wireless/ath/ath10k/htc.c b/drivers/net/wireless/ath/ath10k/htc.c
index 65c6e0c..030748e 100644
--- a/drivers/net/wireless/ath/ath10k/htc.c
+++ b/drivers/net/wireless/ath/ath10k/htc.c
@@ -150,6 +150,9 @@ err:
ep->tx_credits += credits;
spin_unlock_bh(&htc->tx_lock);
+ if (ep->ep_ops.ep_tx_credits)
+ ep->ep_ops.ep_tx_credits(htc->ar);
+
/* 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 */
@@ -303,6 +306,12 @@ ath10k_htc_process_credit_report(struct ath10k_htc *htc,
ep = &htc->endpoint[report->eid];
ep->tx_credits += report->credits;
+ if (ep->ep_ops.ep_tx_credits) {
+ spin_unlock_bh(&htc->tx_lock);
+ ep->ep_ops.ep_tx_credits(htc->ar);
+ spin_lock_bh(&htc->tx_lock);
+ }
+
if (ep->tx_credits && !skb_queue_empty(&ep->tx_queue))
queue_work(htc->ar->workqueue, &ep->send_work);
}
diff --git a/drivers/net/wireless/ath/ath10k/htc.h b/drivers/net/wireless/ath/ath10k/htc.h
index e1dd8c7..92ca29b 100644
--- a/drivers/net/wireless/ath/ath10k/htc.h
+++ b/drivers/net/wireless/ath/ath10k/htc.h
@@ -276,6 +276,7 @@ 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 (*ep_tx_credits)(struct ath10k *);
};
/* service connection information */
--
1.7.9.5
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 3/7] ath10k: make WMI commands block by design
2013-09-10 13:49 [PATCH 0/7] ath10k: improve TX path Michal Kazior
2013-09-10 13:49 ` [PATCH 1/7] ath10k: simplify HTC credits calculation Michal Kazior
2013-09-10 13:49 ` [PATCH 2/7] ath10k: add HTC TX credits replenishing notification Michal Kazior
@ 2013-09-10 13:49 ` Michal Kazior
2013-09-10 13:50 ` [PATCH 4/7] ath10k: simplify HTC command submitting Michal Kazior
` (5 subsequent siblings)
8 siblings, 0 replies; 21+ messages in thread
From: Michal Kazior @ 2013-09-10 13:49 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless, Michal Kazior
This will be necessary for further changes in
command submission scheme.
Once HTC is cleaned up WMI commands will finally
block.
This requires for SWBA to be processed in a
non-atomic context for now. Once other necessary
changes are in this will be reverted.
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
drivers/net/wireless/ath/ath10k/core.h | 1 +
drivers/net/wireless/ath/ath10k/trace.h | 11 ++++---
drivers/net/wireless/ath/ath10k/wmi.c | 54 +++++++++++++++++++------------
3 files changed, 42 insertions(+), 24 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 22b17d6..c953a33 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -114,6 +114,7 @@ struct ath10k_wmi {
struct completion unified_ready;
atomic_t pending_tx_count;
wait_queue_head_t wq;
+ wait_queue_head_t tx_credits_wq;
struct sk_buff_head wmi_event_list;
struct work_struct wmi_event_work;
diff --git a/drivers/net/wireless/ath/ath10k/trace.h b/drivers/net/wireless/ath/ath10k/trace.h
index bf1ceb8..fd53130 100644
--- a/drivers/net/wireless/ath/ath10k/trace.h
+++ b/drivers/net/wireless/ath/ath10k/trace.h
@@ -111,26 +111,29 @@ TRACE_EVENT(ath10k_log_dbg_dump,
);
TRACE_EVENT(ath10k_wmi_cmd,
- TP_PROTO(int id, void *buf, size_t buf_len),
+ TP_PROTO(int id, void *buf, size_t buf_len, int ret),
- TP_ARGS(id, buf, buf_len),
+ TP_ARGS(id, buf, buf_len, ret),
TP_STRUCT__entry(
__field(unsigned int, id)
__field(size_t, buf_len)
__dynamic_array(u8, buf, buf_len)
+ __field(int ret)
),
TP_fast_assign(
__entry->id = id;
__entry->buf_len = buf_len;
+ __entry->ret = ret;
memcpy(__get_dynamic_array(buf), buf, buf_len);
),
TP_printk(
- "id %d len %zu",
+ "id %d len %zu ret %d",
__entry->id,
- __entry->buf_len
+ __entry->buf_len,
+ __entry->ret
)
);
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index 32fd5e7..66cd892 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -90,13 +90,12 @@ static void ath10k_wmi_htc_tx_complete(struct ath10k *ar, struct sk_buff *skb)
wake_up(&ar->wmi.wq);
}
-/* WMI command API */
-static int ath10k_wmi_cmd_send(struct ath10k *ar, struct sk_buff *skb,
- enum wmi_cmd_id cmd_id)
+static int ath10k_wmi_cmd_send_nowait(struct ath10k *ar, struct sk_buff *skb,
+ enum wmi_cmd_id cmd_id)
{
struct ath10k_skb_cb *skb_cb = ATH10K_SKB_CB(skb);
struct wmi_cmd_hdr *cmd_hdr;
- int status;
+ int ret;
u32 cmd = 0;
if (skb_push(skb, sizeof(struct wmi_cmd_hdr)) == NULL)
@@ -107,26 +106,40 @@ static int ath10k_wmi_cmd_send(struct ath10k *ar, struct sk_buff *skb,
cmd_hdr = (struct wmi_cmd_hdr *)skb->data;
cmd_hdr->cmd_id = __cpu_to_le32(cmd);
- if (atomic_add_return(1, &ar->wmi.pending_tx_count) >
- WMI_MAX_PENDING_TX_COUNT) {
- /* avoid using up memory when FW hangs */
- dev_kfree_skb(skb);
- atomic_dec(&ar->wmi.pending_tx_count);
- return -EBUSY;
- }
-
memset(skb_cb, 0, sizeof(*skb_cb));
+ ret = ath10k_htc_send(&ar->htc, ar->wmi.eid, skb);
+ trace_ath10k_wmi_cmd(cmd_id, skb->data, skb->len, ret);
- trace_ath10k_wmi_cmd(cmd_id, skb->data, skb->len);
+ if (ret)
+ goto err_pull;
- status = ath10k_htc_send(&ar->htc, ar->wmi.eid, skb);
- if (status) {
+ return 0;
+
+err_pull:
+ skb_pull(skb, sizeof(struct wmi_cmd_hdr));
+ return ret;
+}
+
+static void ath10k_wmi_op_ep_tx_credits(struct ath10k *ar,
+ enum ath10k_htc_ep_id eid)
+{
+ wake_up(&ar->wmi.tx_credits_wq);
+}
+
+static int ath10k_wmi_cmd_send(struct ath10k *ar, struct sk_buff *skb,
+ enum wmi_cmd_id cmd_id)
+{
+ int ret = -EINVAL;
+
+ wait_event_timeout(ar->wmi.tx_credits_wq, ({
+ ret = ath10k_wmi_cmd_send_nowait(ar, skb, cmd_id);
+ (ret != -EAGAIN);
+ }), 3*HZ);
+
+ if (ret)
dev_kfree_skb_any(skb);
- atomic_dec(&ar->wmi.pending_tx_count);
- return status;
- }
- return 0;
+ return ret;
}
static int ath10k_wmi_event_scan(struct ath10k *ar, struct sk_buff *skb)
@@ -1168,7 +1181,6 @@ static void ath10k_wmi_process_rx(struct ath10k *ar, struct sk_buff *skb)
/* some events require to be handled ASAP
* thus can't be defered to a worker thread */
switch (event_id) {
- case WMI_HOST_SWBA_EVENTID:
case WMI_MGMT_RX_EVENTID:
ath10k_wmi_event_process(ar, skb);
return;
@@ -1186,6 +1198,7 @@ int ath10k_wmi_attach(struct ath10k *ar)
init_completion(&ar->wmi.service_ready);
init_completion(&ar->wmi.unified_ready);
init_waitqueue_head(&ar->wmi.wq);
+ init_waitqueue_head(&ar->wmi.tx_credits_wq);
skb_queue_head_init(&ar->wmi.wmi_event_list);
INIT_WORK(&ar->wmi.wmi_event_work, ath10k_wmi_event_work);
@@ -1215,6 +1228,7 @@ int ath10k_wmi_connect_htc_service(struct ath10k *ar)
/* these fields are the same for all service endpoints */
conn_req.ep_ops.ep_tx_complete = ath10k_wmi_htc_tx_complete;
conn_req.ep_ops.ep_rx_complete = ath10k_wmi_process_rx;
+ conn_req.ep_ops.ep_tx_credits = ath10k_wmi_op_ep_tx_credits;
/* connect to control service */
conn_req.service_id = ATH10K_HTC_SVC_ID_WMI_CONTROL;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 4/7] ath10k: simplify HTC command submitting
2013-09-10 13:49 [PATCH 0/7] ath10k: improve TX path Michal Kazior
` (2 preceding siblings ...)
2013-09-10 13:49 ` [PATCH 3/7] ath10k: make WMI commands block by design Michal Kazior
@ 2013-09-10 13:50 ` Michal Kazior
2013-09-10 13:50 ` [PATCH 5/7] ath10k: improve beacon submission latency Michal Kazior
` (4 subsequent siblings)
8 siblings, 0 replies; 21+ messages in thread
From: Michal Kazior @ 2013-09-10 13:50 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless, Michal Kazior
The patch removes HTC endpoint tx workers in
favour of direct command submission. This makes a
lot more sense for data path.
mac80211 queues are effectively stopped/woken up
in a more timely fashion preventing build up of
frames. It's possible to push more traffic than
the device/system is able to handle and have no
hiccups or performance degradation with UDP
traffic.
WMI commands will now report errors properly and
possibly block as they actively can wait for tx
credits to become available.
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
drivers/net/wireless/ath/ath10k/htc.c | 177 ++++++++-------------------------
drivers/net/wireless/ath/ath10k/htc.h | 4 -
drivers/net/wireless/ath/ath10k/wmi.c | 3 +-
3 files changed, 43 insertions(+), 141 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/htc.c b/drivers/net/wireless/ath/ath10k/htc.c
index 030748e..79f17aa 100644
--- a/drivers/net/wireless/ath/ath10k/htc.c
+++ b/drivers/net/wireless/ath/ath10k/htc.c
@@ -117,100 +117,13 @@ static void ath10k_htc_prepare_tx_skb(struct ath10k_htc_ep *ep,
spin_unlock_bh(&ep->htc->tx_lock);
}
-static int ath10k_htc_issue_skb(struct ath10k_htc *htc,
- struct ath10k_htc_ep *ep,
- struct sk_buff *skb,
- u8 credits)
-{
- struct ath10k_skb_cb *skb_cb = ATH10K_SKB_CB(skb);
- int ret;
-
- ath10k_dbg(ATH10K_DBG_HTC, "%s: ep %d skb %p\n", __func__,
- ep->eid, skb);
-
- ath10k_htc_prepare_tx_skb(ep, skb);
-
- ret = ath10k_skb_map(htc->ar->dev, skb);
- if (ret)
- goto err;
-
- ret = ath10k_hif_send_head(htc->ar,
- ep->ul_pipe_id,
- ep->eid,
- skb->len,
- skb);
- if (unlikely(ret))
- goto err;
-
- return 0;
-err:
- ath10k_warn("HTC issue failed: %d\n", ret);
-
- spin_lock_bh(&htc->tx_lock);
- ep->tx_credits += credits;
- spin_unlock_bh(&htc->tx_lock);
-
- if (ep->ep_ops.ep_tx_credits)
- ep->ep_ops.ep_tx_credits(htc->ar);
-
- /* 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);
-
- return ret;
-}
-
-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;
- u8 credits = 0;
- int ret;
-
- while (true) {
- if (ep->ul_is_polled)
- ath10k_htc_send_complete_check(ep, 0);
-
- spin_lock_bh(&htc->tx_lock);
- skb = __skb_dequeue(&ep->tx_queue);
-
- if (ep->tx_credit_flow_enabled) {
- /* integer division w/ round-up */
- credits = (skb->len + htc->target_credit_size - 1) /
- htc->target_credit_size;
- if (ep->tx_credits < credits) {
- __skb_queue_head(&ep->tx_queue, skb);
- skb = NULL;
- }
- }
- spin_unlock_bh(&htc->tx_lock);
-
- if (!skb)
- break;
-
- ret = ath10k_htc_issue_skb(htc, ep, skb, credits);
- if (ret == -ENOSR)
- break;
- }
-}
-
int ath10k_htc_send(struct ath10k_htc *htc,
enum ath10k_htc_ep_id eid,
struct sk_buff *skb)
{
struct ath10k_htc_ep *ep = &htc->endpoint[eid];
+ int credits = 0;
+ int ret;
if (htc->ar->state == ATH10K_STATE_WEDGED)
return -ECOMM;
@@ -220,18 +133,55 @@ int ath10k_htc_send(struct ath10k_htc *htc,
return -ENOENT;
}
+ /* FIXME: This looks ugly, can we fix it? */
spin_lock_bh(&htc->tx_lock);
if (htc->stopped) {
spin_unlock_bh(&htc->tx_lock);
return -ESHUTDOWN;
}
+ spin_unlock_bh(&htc->tx_lock);
- __skb_queue_tail(&ep->tx_queue, skb);
skb_push(skb, sizeof(struct ath10k_htc_hdr));
- spin_unlock_bh(&htc->tx_lock);
- queue_work(htc->ar->workqueue, &ep->send_work);
+ if (ep->tx_credit_flow_enabled) {
+ /* integer division w/ round-up */
+ credits = (skb->len + htc->target_credit_size - 1) /
+ htc->target_credit_size;
+ spin_lock_bh(&htc->tx_lock);
+ if (ep->tx_credits < credits) {
+ spin_unlock_bh(&htc->tx_lock);
+ ret = -EAGAIN;
+ goto err_pull;
+ }
+ ep->tx_credits -= credits;
+ spin_unlock_bh(&htc->tx_lock);
+ }
+
+ ath10k_htc_prepare_tx_skb(ep, skb);
+
+ ret = ath10k_skb_map(htc->ar->dev, skb);
+ if (ret)
+ goto err_credits;
+
+ ret = ath10k_hif_send_head(htc->ar, ep->ul_pipe_id, ep->eid,
+ skb->len, skb);
+ if (ret)
+ goto err_unmap;
+
return 0;
+
+err_unmap:
+ ath10k_skb_unmap(htc->ar->dev, skb);
+err_credits:
+ spin_lock_bh(&htc->tx_lock);
+ ep->tx_credits += credits;
+ spin_unlock_bh(&htc->tx_lock);
+
+ if (ep->ep_ops.ep_tx_credits)
+ ep->ep_ops.ep_tx_credits(htc->ar);
+err_pull:
+ skb_pull(skb, sizeof(struct ath10k_htc_hdr));
+ return ret;
}
static int ath10k_htc_tx_completion_handler(struct ath10k *ar,
@@ -244,39 +194,9 @@ static int ath10k_htc_tx_completion_handler(struct ath10k *ar,
ath10k_htc_notify_tx_completion(ep, skb);
/* the skb now belongs to the completion handler */
- /* note: when using TX credit flow, the re-checking of queues happens
- * when credits flow back from the target. in the non-TX credit case,
- * we recheck after the packet completes */
- spin_lock_bh(&htc->tx_lock);
- if (!ep->tx_credit_flow_enabled && !htc->stopped)
- queue_work(ar->workqueue, &ep->send_work);
- spin_unlock_bh(&htc->tx_lock);
-
return 0;
}
-/* flush endpoint TX queue */
-static void ath10k_htc_flush_endpoint_tx(struct ath10k_htc *htc,
- struct ath10k_htc_ep *ep)
-{
- struct sk_buff *skb;
- struct ath10k_skb_cb *skb_cb;
-
- spin_lock_bh(&htc->tx_lock);
- for (;;) {
- skb = __skb_dequeue(&ep->tx_queue);
- if (!skb)
- break;
-
- skb_cb = ATH10K_SKB_CB(skb);
- skb_cb->is_aborted = true;
- ath10k_htc_notify_tx_completion(ep, skb);
- }
- spin_unlock_bh(&htc->tx_lock);
-
- cancel_work_sync(&ep->send_work);
-}
-
/***********/
/* Receive */
/***********/
@@ -311,9 +231,6 @@ ath10k_htc_process_credit_report(struct ath10k_htc *htc,
ep->ep_ops.ep_tx_credits(htc->ar);
spin_lock_bh(&htc->tx_lock);
}
-
- if (ep->tx_credits && !skb_queue_empty(&ep->tx_queue))
- queue_work(htc->ar->workqueue, &ep->send_work);
}
spin_unlock_bh(&htc->tx_lock);
}
@@ -571,10 +488,8 @@ static void ath10k_htc_reset_endpoint_states(struct ath10k_htc *htc)
ep->max_ep_message_len = 0;
ep->max_tx_queue_depth = 0;
ep->eid = i;
- skb_queue_head_init(&ep->tx_queue);
ep->htc = htc;
ep->tx_credit_flow_enabled = true;
- INIT_WORK(&ep->send_work, ath10k_htc_send_work);
}
}
@@ -917,18 +832,10 @@ int ath10k_htc_start(struct ath10k_htc *htc)
*/
void ath10k_htc_stop(struct ath10k_htc *htc)
{
- int i;
- struct ath10k_htc_ep *ep;
-
spin_lock_bh(&htc->tx_lock);
htc->stopped = true;
spin_unlock_bh(&htc->tx_lock);
- for (i = ATH10K_HTC_EP_0; i < ATH10K_HTC_EP_COUNT; i++) {
- ep = &htc->endpoint[i];
- ath10k_htc_flush_endpoint_tx(htc, ep);
- }
-
ath10k_hif_stop(htc->ar);
}
diff --git a/drivers/net/wireless/ath/ath10k/htc.h b/drivers/net/wireless/ath/ath10k/htc.h
index 92ca29b..4716d33 100644
--- a/drivers/net/wireless/ath/ath10k/htc.h
+++ b/drivers/net/wireless/ath/ath10k/htc.h
@@ -316,15 +316,11 @@ struct ath10k_htc_ep {
int ul_is_polled; /* call HIF to get tx completions */
int dl_is_polled; /* call HIF to fetch rx (not implemented) */
- struct sk_buff_head tx_queue;
-
u8 seq_no; /* for debugging */
int tx_credits;
int tx_credit_size;
int tx_credits_per_max_message;
bool tx_credit_flow_enabled;
-
- struct work_struct send_work;
};
struct ath10k_htc_svc_tx_credits {
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index 66cd892..ff407c2 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -120,8 +120,7 @@ err_pull:
return ret;
}
-static void ath10k_wmi_op_ep_tx_credits(struct ath10k *ar,
- enum ath10k_htc_ep_id eid)
+static void ath10k_wmi_op_ep_tx_credits(struct ath10k *ar)
{
wake_up(&ar->wmi.tx_credits_wq);
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 5/7] ath10k: improve beacon submission latency
2013-09-10 13:49 [PATCH 0/7] ath10k: improve TX path Michal Kazior
` (3 preceding siblings ...)
2013-09-10 13:50 ` [PATCH 4/7] ath10k: simplify HTC command submitting Michal Kazior
@ 2013-09-10 13:50 ` Michal Kazior
2013-09-12 16:47 ` Kalle Valo
2013-09-10 13:50 ` [PATCH 6/7] ath10k: remove wmi pending count limit Michal Kazior
` (3 subsequent siblings)
8 siblings, 1 reply; 21+ messages in thread
From: Michal Kazior @ 2013-09-10 13:50 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless, Michal Kazior
The patch prevents beacon misses in some case of
heavy load on a system.
If a beacon can't be transmitted directly from an
SWBA event it will be left in arvif->beacon and
transmission will be retried once TX credits
become available.
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
drivers/net/wireless/ath/ath10k/core.h | 1 +
drivers/net/wireless/ath/ath10k/mac.c | 7 ++++
drivers/net/wireless/ath/ath10k/wmi.c | 70 ++++++++++++++++++++++++++------
drivers/net/wireless/ath/ath10k/wmi.h | 3 +-
4 files changed, 67 insertions(+), 14 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index c953a33..14b7d3d 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -204,6 +204,7 @@ struct ath10k_vif {
enum wmi_vdev_subtype vdev_subtype;
u32 beacon_interval;
u32 dtim_period;
+ struct sk_buff *beacon;
struct ath10k *ar;
struct ieee80211_vif *vif;
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 248248b..439d0c3 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -2056,6 +2056,13 @@ static void ath10k_remove_interface(struct ieee80211_hw *hw,
ath10k_dbg(ATH10K_DBG_MAC, "Remove interface: id %d\n", arvif->vdev_id);
+ spin_lock_bh(&ar->data_lock);
+ if (arvif->beacon) {
+ dev_kfree_skb_any(arvif->beacon);
+ arvif->beacon = NULL;
+ }
+ spin_unlock_bh(&ar->data_lock);
+
ar->free_vdev_map |= 1 << (arvif->vdev_id);
if (arvif->vdev_type == WMI_VDEV_TYPE_AP) {
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index ff407c2..c13e434 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -120,8 +120,51 @@ err_pull:
return ret;
}
+static void ath10k_wmi_tx_beacon_nowait(struct ath10k_vif *arvif)
+{
+ struct wmi_bcn_tx_arg arg = {0};
+ int ret;
+
+ lockdep_assert_held(&arvif->ar->data_lock);
+
+ if (arvif->beacon == NULL)
+ return;
+
+ arg.vdev_id = arvif->vdev_id;
+ arg.tx_rate = 0;
+ arg.tx_power = 0;
+ arg.bcn = arvif->beacon->data;
+ arg.bcn_len = arvif->beacon->len;
+
+ ret = ath10k_wmi_beacon_send_nowait(arvif->ar, &arg);
+ if (ret)
+ return;
+
+ dev_kfree_skb_any(arvif->beacon);
+ arvif->beacon = NULL;
+}
+
+static void ath10k_wmi_tx_beacons_iter(void *data, u8 *mac,
+ struct ieee80211_vif *vif)
+{
+ struct ath10k_vif *arvif = ath10k_vif_to_arvif(vif);
+
+ ath10k_wmi_tx_beacon_nowait(arvif);
+}
+
+static void ath10k_wmi_tx_beacons_nowait(struct ath10k *ar)
+{
+ spin_lock_bh(&ar->data_lock);
+ ieee80211_iterate_active_interfaces_atomic(ar->hw,
+ IEEE80211_IFACE_ITER_NORMAL,
+ ath10k_wmi_tx_beacons_iter,
+ NULL);
+ spin_unlock_bh(&ar->data_lock);
+}
+
static void ath10k_wmi_op_ep_tx_credits(struct ath10k *ar)
{
+ ath10k_wmi_tx_beacons_nowait(ar);
wake_up(&ar->wmi.tx_credits_wq);
}
@@ -131,6 +174,7 @@ static int ath10k_wmi_cmd_send(struct ath10k *ar, struct sk_buff *skb,
int ret = -EINVAL;
wait_event_timeout(ar->wmi.tx_credits_wq, ({
+ ath10k_wmi_tx_beacons_nowait(ar);
ret = ath10k_wmi_cmd_send_nowait(ar, skb, cmd_id);
(ret != -EAGAIN);
}), 3*HZ);
@@ -760,10 +804,8 @@ static void ath10k_wmi_event_host_swba(struct ath10k *ar, struct sk_buff *skb)
int i = -1;
struct wmi_bcn_info *bcn_info;
struct ath10k_vif *arvif;
- struct wmi_bcn_tx_arg arg;
struct sk_buff *bcn;
int vdev_id = 0;
- int ret;
ath10k_dbg(ATH10K_DBG_MGMT, "WMI_HOST_SWBA_EVENTID\n");
@@ -820,17 +862,17 @@ static void ath10k_wmi_event_host_swba(struct ath10k *ar, struct sk_buff *skb)
ath10k_wmi_update_tim(ar, arvif, bcn, bcn_info);
ath10k_wmi_update_noa(ar, arvif, bcn, bcn_info);
- arg.vdev_id = arvif->vdev_id;
- arg.tx_rate = 0;
- arg.tx_power = 0;
- arg.bcn = bcn->data;
- arg.bcn_len = bcn->len;
+ spin_lock_bh(&ar->data_lock);
+ if (arvif->beacon) {
+ ath10k_warn("SWBA overrun on vdev %d\n",
+ arvif->vdev_id);
+ dev_kfree_skb_any(arvif->beacon);
+ }
- ret = ath10k_wmi_beacon_send(ar, &arg);
- if (ret)
- ath10k_warn("could not send beacon (%d)\n", ret);
+ arvif->beacon = bcn;
- dev_kfree_skb_any(bcn);
+ ath10k_wmi_tx_beacon_nowait(arvif);
+ spin_unlock_bh(&ar->data_lock);
}
}
@@ -1181,6 +1223,7 @@ static void ath10k_wmi_process_rx(struct ath10k *ar, struct sk_buff *skb)
* thus can't be defered to a worker thread */
switch (event_id) {
case WMI_MGMT_RX_EVENTID:
+ case WMI_HOST_SWBA_EVENTID:
ath10k_wmi_event_process(ar, skb);
return;
default:
@@ -2138,7 +2181,8 @@ int ath10k_wmi_peer_assoc(struct ath10k *ar,
return ath10k_wmi_cmd_send(ar, skb, WMI_PEER_ASSOC_CMDID);
}
-int ath10k_wmi_beacon_send(struct ath10k *ar, const struct wmi_bcn_tx_arg *arg)
+int ath10k_wmi_beacon_send_nowait(struct ath10k *ar,
+ const struct wmi_bcn_tx_arg *arg)
{
struct wmi_bcn_tx_cmd *cmd;
struct sk_buff *skb;
@@ -2154,7 +2198,7 @@ int ath10k_wmi_beacon_send(struct ath10k *ar, const struct wmi_bcn_tx_arg *arg)
cmd->hdr.bcn_len = __cpu_to_le32(arg->bcn_len);
memcpy(cmd->bcn, arg->bcn, arg->bcn_len);
- return ath10k_wmi_cmd_send(ar, skb, WMI_BCN_TX_CMDID);
+ return ath10k_wmi_cmd_send_nowait(ar, skb, WMI_BCN_TX_CMDID);
}
static void ath10k_wmi_pdev_set_wmm_param(struct wmi_wmm_params *params,
diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
index 40fcf05..482461a 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.h
+++ b/drivers/net/wireless/ath/ath10k/wmi.h
@@ -3068,7 +3068,8 @@ int ath10k_wmi_set_ap_ps_param(struct ath10k *ar, u32 vdev_id, const u8 *mac,
enum wmi_ap_ps_peer_param param_id, u32 value);
int ath10k_wmi_scan_chan_list(struct ath10k *ar,
const struct wmi_scan_chan_list_arg *arg);
-int ath10k_wmi_beacon_send(struct ath10k *ar, const struct wmi_bcn_tx_arg *arg);
+int ath10k_wmi_beacon_send_nowait(struct ath10k *ar,
+ const struct wmi_bcn_tx_arg *arg);
int ath10k_wmi_pdev_set_wmm_params(struct ath10k *ar,
const struct wmi_pdev_set_wmm_params_arg *arg);
int ath10k_wmi_request_stats(struct ath10k *ar, enum wmi_stats_id stats_id);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 5/7] ath10k: improve beacon submission latency
2013-09-10 13:50 ` [PATCH 5/7] ath10k: improve beacon submission latency Michal Kazior
@ 2013-09-12 16:47 ` Kalle Valo
0 siblings, 0 replies; 21+ messages in thread
From: Kalle Valo @ 2013-09-12 16:47 UTC (permalink / raw)
To: Michal Kazior; +Cc: ath10k, linux-wireless
Michal Kazior <michal.kazior@tieto.com> writes:
> The patch prevents beacon misses in some case of
> heavy load on a system.
>
> If a beacon can't be transmitted directly from an
> SWBA event it will be left in arvif->beacon and
> transmission will be retried once TX credits
> become available.
>
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
[...]
> static void ath10k_wmi_op_ep_tx_credits(struct ath10k *ar)
> {
> + ath10k_wmi_tx_beacons_nowait(ar);
> wake_up(&ar->wmi.tx_credits_wq);
> }
>
> @@ -131,6 +174,7 @@ static int ath10k_wmi_cmd_send(struct ath10k *ar, struct sk_buff *skb,
> int ret = -EINVAL;
>
> wait_event_timeout(ar->wmi.tx_credits_wq, ({
> + ath10k_wmi_tx_beacons_nowait(ar);
> ret = ath10k_wmi_cmd_send_nowait(ar, skb, cmd_id);
> (ret != -EAGAIN);
Please add a short comment to both calls of
ath10k_wmi_tx_beacons_nowait(). Something like "first transmit all
pending beacons" or similar.
--
Kalle Valo
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 6/7] ath10k: remove wmi pending count limit
2013-09-10 13:49 [PATCH 0/7] ath10k: improve TX path Michal Kazior
` (4 preceding siblings ...)
2013-09-10 13:50 ` [PATCH 5/7] ath10k: improve beacon submission latency Michal Kazior
@ 2013-09-10 13:50 ` Michal Kazior
2013-09-10 13:50 ` [PATCH 7/7] ath10k: remove wmi event worker thread Michal Kazior
` (2 subsequent siblings)
8 siblings, 0 replies; 21+ messages in thread
From: Michal Kazior @ 2013-09-10 13:50 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless, Michal Kazior
It is no longer used nor necessary since WMI
commands can block.
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
drivers/net/wireless/ath/ath10k/core.h | 2 --
drivers/net/wireless/ath/ath10k/mac.c | 9 ---------
drivers/net/wireless/ath/ath10k/wmi.c | 32 --------------------------------
drivers/net/wireless/ath/ath10k/wmi.h | 2 --
4 files changed, 45 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 14b7d3d..c2b6a76 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -112,8 +112,6 @@ struct ath10k_wmi {
enum ath10k_htc_ep_id eid;
struct completion service_ready;
struct completion unified_ready;
- atomic_t pending_tx_count;
- wait_queue_head_t wq;
wait_queue_head_t tx_credits_wq;
struct sk_buff_head wmi_event_list;
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 439d0c3..698088f 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -1205,8 +1205,6 @@ static void ath10k_bss_disassoc(struct ieee80211_hw *hw,
ath10k_dbg(ATH10K_DBG_MAC, "VDEV: %d ath10k_wmi_vdev_down failed (%d)\n",
arvif->vdev_id, ret);
- ath10k_wmi_flush_tx(ar);
-
arvif->def_wep_key_index = 0;
}
@@ -1643,8 +1641,6 @@ static int ath10k_abort_scan(struct ath10k *ar)
return -EIO;
}
- ath10k_wmi_flush_tx(ar);
-
ret = wait_for_completion_timeout(&ar->scan.completed, 3*HZ);
if (ret == 0)
ath10k_warn("timed out while waiting for scan to stop\n");
@@ -1678,10 +1674,6 @@ static int ath10k_start_scan(struct ath10k *ar,
if (ret)
return ret;
- /* make sure we submit the command so the completion
- * timeout makes sense */
- ath10k_wmi_flush_tx(ar);
-
ret = wait_for_completion_timeout(&ar->scan.started, 1*HZ);
if (ret == 0) {
ath10k_abort_scan(ar);
@@ -1903,7 +1895,6 @@ static int ath10k_config(struct ieee80211_hw *hw, u32 changed)
ret = ath10k_monitor_destroy(ar);
}
- ath10k_wmi_flush_tx(ar);
mutex_unlock(&ar->conf_mutex);
return ret;
}
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index c13e434..aa7093a 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -23,30 +23,6 @@
#include "wmi.h"
#include "mac.h"
-void ath10k_wmi_flush_tx(struct ath10k *ar)
-{
- int ret;
-
- lockdep_assert_held(&ar->conf_mutex);
-
- if (ar->state == ATH10K_STATE_WEDGED) {
- ath10k_warn("wmi flush skipped - device is wedged anyway\n");
- return;
- }
-
- ret = wait_event_timeout(ar->wmi.wq,
- atomic_read(&ar->wmi.pending_tx_count) == 0,
- 5*HZ);
- if (atomic_read(&ar->wmi.pending_tx_count) == 0)
- return;
-
- if (ret == 0)
- ret = -ETIMEDOUT;
-
- if (ret < 0)
- ath10k_warn("wmi flush failed (%d)\n", ret);
-}
-
int ath10k_wmi_wait_for_service_ready(struct ath10k *ar)
{
int ret;
@@ -85,9 +61,6 @@ static struct sk_buff *ath10k_wmi_alloc_skb(u32 len)
static void ath10k_wmi_htc_tx_complete(struct ath10k *ar, struct sk_buff *skb)
{
dev_kfree_skb(skb);
-
- if (atomic_sub_return(1, &ar->wmi.pending_tx_count) == 0)
- wake_up(&ar->wmi.wq);
}
static int ath10k_wmi_cmd_send_nowait(struct ath10k *ar, struct sk_buff *skb,
@@ -1239,7 +1212,6 @@ int ath10k_wmi_attach(struct ath10k *ar)
{
init_completion(&ar->wmi.service_ready);
init_completion(&ar->wmi.unified_ready);
- init_waitqueue_head(&ar->wmi.wq);
init_waitqueue_head(&ar->wmi.tx_credits_wq);
skb_queue_head_init(&ar->wmi.wmi_event_list);
@@ -1250,10 +1222,6 @@ int ath10k_wmi_attach(struct ath10k *ar)
void ath10k_wmi_detach(struct ath10k *ar)
{
- /* HTC should've drained the packets already */
- if (WARN_ON(atomic_read(&ar->wmi.pending_tx_count) > 0))
- ath10k_warn("there are still pending packets\n");
-
cancel_work_sync(&ar->wmi.wmi_event_work);
skb_queue_purge(&ar->wmi.wmi_event_list);
}
diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
index 482461a..82d6103 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.h
+++ b/drivers/net/wireless/ath/ath10k/wmi.h
@@ -3002,7 +3002,6 @@ struct wmi_force_fw_hang_cmd {
#define WMI_MAX_EVENT 0x1000
/* Maximum number of pending TXed WMI packets */
-#define WMI_MAX_PENDING_TX_COUNT 128
#define WMI_SKB_HEADROOM sizeof(struct wmi_cmd_hdr)
/* By default disable power save for IBSS */
@@ -3015,7 +3014,6 @@ int ath10k_wmi_attach(struct ath10k *ar);
void ath10k_wmi_detach(struct ath10k *ar);
int ath10k_wmi_wait_for_service_ready(struct ath10k *ar);
int ath10k_wmi_wait_for_unified_ready(struct ath10k *ar);
-void ath10k_wmi_flush_tx(struct ath10k *ar);
int ath10k_wmi_connect_htc_service(struct ath10k *ar);
int ath10k_wmi_pdev_set_channel(struct ath10k *ar,
--
1.7.9.5
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 7/7] ath10k: remove wmi event worker thread
2013-09-10 13:49 [PATCH 0/7] ath10k: improve TX path Michal Kazior
` (5 preceding siblings ...)
2013-09-10 13:50 ` [PATCH 6/7] ath10k: remove wmi pending count limit Michal Kazior
@ 2013-09-10 13:50 ` Michal Kazior
2013-09-12 16:48 ` [PATCH 0/7] ath10k: improve TX path Kalle Valo
2013-09-13 12:16 ` [PATCHv2 " Michal Kazior
8 siblings, 0 replies; 21+ messages in thread
From: Michal Kazior @ 2013-09-10 13:50 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless, Michal Kazior
It's not really necessary to have this processed
in a worker. There are no sleepable calls (and
actually shouldn't be).
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
drivers/net/wireless/ath/ath10k/core.h | 3 ---
drivers/net/wireless/ath/ath10k/wmi.c | 44 +-------------------------------
2 files changed, 1 insertion(+), 46 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index c2b6a76..fcf94ee 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -113,9 +113,6 @@ struct ath10k_wmi {
struct completion service_ready;
struct completion unified_ready;
wait_queue_head_t tx_credits_wq;
-
- struct sk_buff_head wmi_event_list;
- struct work_struct wmi_event_work;
};
struct ath10k_peer_stat {
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index aa7093a..3831694 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -1051,7 +1051,7 @@ static int ath10k_wmi_ready_event_rx(struct ath10k *ar, struct sk_buff *skb)
return 0;
}
-static void ath10k_wmi_event_process(struct ath10k *ar, struct sk_buff *skb)
+static void ath10k_wmi_process_rx(struct ath10k *ar, struct sk_buff *skb)
{
struct wmi_cmd_hdr *cmd_hdr;
enum wmi_event_id id;
@@ -1170,43 +1170,6 @@ static void ath10k_wmi_event_process(struct ath10k *ar, struct sk_buff *skb)
dev_kfree_skb(skb);
}
-static void ath10k_wmi_event_work(struct work_struct *work)
-{
- struct ath10k *ar = container_of(work, struct ath10k,
- wmi.wmi_event_work);
- struct sk_buff *skb;
-
- for (;;) {
- skb = skb_dequeue(&ar->wmi.wmi_event_list);
- if (!skb)
- break;
-
- ath10k_wmi_event_process(ar, skb);
- }
-}
-
-static void ath10k_wmi_process_rx(struct ath10k *ar, struct sk_buff *skb)
-{
- struct wmi_cmd_hdr *cmd_hdr = (struct wmi_cmd_hdr *)skb->data;
- enum wmi_event_id event_id;
-
- event_id = MS(__le32_to_cpu(cmd_hdr->cmd_id), WMI_CMD_HDR_CMD_ID);
-
- /* some events require to be handled ASAP
- * thus can't be defered to a worker thread */
- switch (event_id) {
- case WMI_MGMT_RX_EVENTID:
- case WMI_HOST_SWBA_EVENTID:
- ath10k_wmi_event_process(ar, skb);
- return;
- default:
- break;
- }
-
- skb_queue_tail(&ar->wmi.wmi_event_list, skb);
- queue_work(ar->workqueue, &ar->wmi.wmi_event_work);
-}
-
/* WMI Initialization functions */
int ath10k_wmi_attach(struct ath10k *ar)
{
@@ -1214,16 +1177,11 @@ int ath10k_wmi_attach(struct ath10k *ar)
init_completion(&ar->wmi.unified_ready);
init_waitqueue_head(&ar->wmi.tx_credits_wq);
- skb_queue_head_init(&ar->wmi.wmi_event_list);
- INIT_WORK(&ar->wmi.wmi_event_work, ath10k_wmi_event_work);
-
return 0;
}
void ath10k_wmi_detach(struct ath10k *ar)
{
- cancel_work_sync(&ar->wmi.wmi_event_work);
- skb_queue_purge(&ar->wmi.wmi_event_list);
}
int ath10k_wmi_connect_htc_service(struct ath10k *ar)
--
1.7.9.5
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 0/7] ath10k: improve TX path
2013-09-10 13:49 [PATCH 0/7] ath10k: improve TX path Michal Kazior
` (6 preceding siblings ...)
2013-09-10 13:50 ` [PATCH 7/7] ath10k: remove wmi event worker thread Michal Kazior
@ 2013-09-12 16:48 ` Kalle Valo
2013-09-13 12:16 ` [PATCHv2 " Michal Kazior
8 siblings, 0 replies; 21+ messages in thread
From: Kalle Valo @ 2013-09-12 16:48 UTC (permalink / raw)
To: Michal Kazior; +Cc: ath10k, linux-wireless
Michal Kazior <michal.kazior@tieto.com> writes:
> Hi,
>
> This patchset addresses two issues:
>
> * system/userspace starvation on heavy briding
> UDP TX
> * unstable/inconsistent UDP TX throughput
>
> In short the patchset simplifies TX path by
> removing HTC TX workers, makes WMI commands block
> and makes ath10k more responsive to queues
> becoming full. This contributes to both improved
> throughput and makes the system more responsive
> under heavy UDP TX load.
This looks very good, it's great that we can get rid of the TX workers.
Please submit v2 and I'll apply these.
--
Kalle Valo
^ permalink raw reply [flat|nested] 21+ messages in thread* [PATCHv2 0/7] ath10k: improve TX path
2013-09-10 13:49 [PATCH 0/7] ath10k: improve TX path Michal Kazior
` (7 preceding siblings ...)
2013-09-12 16:48 ` [PATCH 0/7] ath10k: improve TX path Kalle Valo
@ 2013-09-13 12:16 ` Michal Kazior
2013-09-13 12:16 ` [PATCHv2 1/7] ath10k: simplify HTC credits calculation Michal Kazior
` (7 more replies)
8 siblings, 8 replies; 21+ messages in thread
From: Michal Kazior @ 2013-09-13 12:16 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless, Michal Kazior
Hi,
This patchset addresses two issues:
* system/userspace starvation on heavy briding
UDP TX
* unstable/inconsistent UDP TX throughput
In short the patchset simplifies TX path by
removing HTC TX workers, makes WMI commands block
and makes ath10k more responsive to queues
becoming full. This contributes to both improved
throughput and makes the system more responsive
under heavy UDP TX load.
Max stable briding TX (ath10k as TX):
UDP w/o patchset: 520mbps
UDP w/ patchset: 570mbps
TCP w/o patchset: 350mbps
TCP w/ patchset: 400mbps
Measured with two 2x2 cards, one acting as an AP
on AP135 board, the other as a STA on a core i5
laptop. The AP135 was passing traffic from a
station on LAN/eth1 to the STA/wlan0.
v2:
* use DIV_ROUND_UP
* comment usage of ath10k_wmi_tx_beacons_nowait
* additional check for tx_credits_flow_enabled
Michal Kazior (7):
ath10k: simplify HTC credits calculation
ath10k: add HTC TX credits replenishing notification
ath10k: make WMI commands block by design
ath10k: simplify HTC command submitting
ath10k: improve beacon submission latency
ath10k: remove wmi pending count limit
ath10k: remove wmi event worker thread
drivers/net/wireless/ath/ath10k/core.h | 7 +-
drivers/net/wireless/ath/ath10k/htc.c | 215 +++++++------------------------
drivers/net/wireless/ath/ath10k/htc.h | 5 +-
drivers/net/wireless/ath/ath10k/mac.c | 16 +--
drivers/net/wireless/ath/ath10k/trace.h | 11 +-
drivers/net/wireless/ath/ath10k/wmi.c | 201 ++++++++++++++---------------
drivers/net/wireless/ath/ath10k/wmi.h | 5 +-
7 files changed, 160 insertions(+), 300 deletions(-)
--
1.7.9.5
^ permalink raw reply [flat|nested] 21+ messages in thread* [PATCHv2 1/7] ath10k: simplify HTC credits calculation
2013-09-13 12:16 ` [PATCHv2 " Michal Kazior
@ 2013-09-13 12:16 ` Michal Kazior
2013-09-13 12:16 ` [PATCHv2 2/7] ath10k: add HTC TX credits replenishing notification Michal Kazior
` (6 subsequent siblings)
7 siblings, 0 replies; 21+ messages in thread
From: Michal Kazior @ 2013-09-13 12:16 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless, Michal Kazior
Credit calculation was overly complex
unnecessarily. Now skb dequeing is more unified.
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
v2:
* use DIV_ROUND_UP
drivers/net/wireless/ath/ath10k/htc.c | 58 ++++++---------------------------
1 file changed, 10 insertions(+), 48 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/htc.c b/drivers/net/wireless/ath/ath10k/htc.c
index f03fd8d..1da0f82 100644
--- a/drivers/net/wireless/ath/ath10k/htc.c
+++ b/drivers/net/wireless/ath/ath10k/htc.c
@@ -167,49 +167,6 @@ err:
return ret;
}
-static struct sk_buff *ath10k_htc_get_skb_credit_based(struct ath10k_htc *htc,
- struct ath10k_htc_ep *ep,
- u8 *credits)
-{
- struct sk_buff *skb;
- struct ath10k_skb_cb *skb_cb;
- int credits_required;
- int remainder;
- unsigned int transfer_len;
-
- lockdep_assert_held(&htc->tx_lock);
-
- skb = __skb_dequeue(&ep->tx_queue);
- if (!skb)
- return NULL;
-
- skb_cb = ATH10K_SKB_CB(skb);
- transfer_len = skb->len;
-
- if (likely(transfer_len <= htc->target_credit_size)) {
- credits_required = 1;
- } else {
- /* figure out how many credits this message requires */
- credits_required = transfer_len / htc->target_credit_size;
- remainder = transfer_len % htc->target_credit_size;
-
- if (remainder)
- credits_required++;
- }
-
- ath10k_dbg(ATH10K_DBG_HTC, "Credits required %d got %d\n",
- credits_required, ep->tx_credits);
-
- if (ep->tx_credits < credits_required) {
- __skb_queue_head(&ep->tx_queue, skb);
- return NULL;
- }
-
- ep->tx_credits -= credits_required;
- *credits = credits_required;
- return skb;
-}
-
static void ath10k_htc_send_work(struct work_struct *work)
{
struct ath10k_htc_ep *ep = container_of(work,
@@ -224,11 +181,16 @@ static void ath10k_htc_send_work(struct work_struct *work)
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 = __skb_dequeue(&ep->tx_queue);
+ skb = __skb_dequeue(&ep->tx_queue);
+
+ if (ep->tx_credit_flow_enabled) {
+ credits = DIV_ROUND_UP(skb->len,
+ htc->target_credit_size);
+ if (ep->tx_credits < credits) {
+ __skb_queue_head(&ep->tx_queue, skb);
+ skb = NULL;
+ }
+ }
spin_unlock_bh(&htc->tx_lock);
if (!skb)
--
1.7.9.5
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCHv2 2/7] ath10k: add HTC TX credits replenishing notification
2013-09-13 12:16 ` [PATCHv2 " Michal Kazior
2013-09-13 12:16 ` [PATCHv2 1/7] ath10k: simplify HTC credits calculation Michal Kazior
@ 2013-09-13 12:16 ` Michal Kazior
2013-09-13 12:16 ` [PATCHv2 3/7] ath10k: make WMI commands block by design Michal Kazior
` (5 subsequent siblings)
7 siblings, 0 replies; 21+ messages in thread
From: Michal Kazior @ 2013-09-13 12:16 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless, Michal Kazior
This will allow higher layers to anticipate and
act upon TX credits renewal. This will be
important for some future rework of WMI command
submission.
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
drivers/net/wireless/ath/ath10k/htc.c | 9 +++++++++
drivers/net/wireless/ath/ath10k/htc.h | 1 +
2 files changed, 10 insertions(+)
diff --git a/drivers/net/wireless/ath/ath10k/htc.c b/drivers/net/wireless/ath/ath10k/htc.c
index 1da0f82..49da4e5 100644
--- a/drivers/net/wireless/ath/ath10k/htc.c
+++ b/drivers/net/wireless/ath/ath10k/htc.c
@@ -150,6 +150,9 @@ err:
ep->tx_credits += credits;
spin_unlock_bh(&htc->tx_lock);
+ if (ep->ep_ops.ep_tx_credits)
+ ep->ep_ops.ep_tx_credits(htc->ar);
+
/* 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 */
@@ -302,6 +305,12 @@ ath10k_htc_process_credit_report(struct ath10k_htc *htc,
ep = &htc->endpoint[report->eid];
ep->tx_credits += report->credits;
+ if (ep->ep_ops.ep_tx_credits) {
+ spin_unlock_bh(&htc->tx_lock);
+ ep->ep_ops.ep_tx_credits(htc->ar);
+ spin_lock_bh(&htc->tx_lock);
+ }
+
if (ep->tx_credits && !skb_queue_empty(&ep->tx_queue))
queue_work(htc->ar->workqueue, &ep->send_work);
}
diff --git a/drivers/net/wireless/ath/ath10k/htc.h b/drivers/net/wireless/ath/ath10k/htc.h
index e1dd8c7..92ca29b 100644
--- a/drivers/net/wireless/ath/ath10k/htc.h
+++ b/drivers/net/wireless/ath/ath10k/htc.h
@@ -276,6 +276,7 @@ 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 (*ep_tx_credits)(struct ath10k *);
};
/* service connection information */
--
1.7.9.5
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCHv2 3/7] ath10k: make WMI commands block by design
2013-09-13 12:16 ` [PATCHv2 " Michal Kazior
2013-09-13 12:16 ` [PATCHv2 1/7] ath10k: simplify HTC credits calculation Michal Kazior
2013-09-13 12:16 ` [PATCHv2 2/7] ath10k: add HTC TX credits replenishing notification Michal Kazior
@ 2013-09-13 12:16 ` Michal Kazior
2013-09-13 12:16 ` [PATCHv2 4/7] ath10k: simplify HTC command submitting Michal Kazior
` (4 subsequent siblings)
7 siblings, 0 replies; 21+ messages in thread
From: Michal Kazior @ 2013-09-13 12:16 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless, Michal Kazior
This will be necessary for further changes in
command submission scheme.
Once HTC is cleaned up WMI commands will finally
block.
This requires for SWBA to be processed in a
non-atomic context for now. Once other necessary
changes are in this will be reverted.
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
drivers/net/wireless/ath/ath10k/core.h | 1 +
drivers/net/wireless/ath/ath10k/trace.h | 11 ++++---
drivers/net/wireless/ath/ath10k/wmi.c | 54 +++++++++++++++++++------------
3 files changed, 42 insertions(+), 24 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 22b17d6..c953a33 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -114,6 +114,7 @@ struct ath10k_wmi {
struct completion unified_ready;
atomic_t pending_tx_count;
wait_queue_head_t wq;
+ wait_queue_head_t tx_credits_wq;
struct sk_buff_head wmi_event_list;
struct work_struct wmi_event_work;
diff --git a/drivers/net/wireless/ath/ath10k/trace.h b/drivers/net/wireless/ath/ath10k/trace.h
index bf1ceb8..fd53130 100644
--- a/drivers/net/wireless/ath/ath10k/trace.h
+++ b/drivers/net/wireless/ath/ath10k/trace.h
@@ -111,26 +111,29 @@ TRACE_EVENT(ath10k_log_dbg_dump,
);
TRACE_EVENT(ath10k_wmi_cmd,
- TP_PROTO(int id, void *buf, size_t buf_len),
+ TP_PROTO(int id, void *buf, size_t buf_len, int ret),
- TP_ARGS(id, buf, buf_len),
+ TP_ARGS(id, buf, buf_len, ret),
TP_STRUCT__entry(
__field(unsigned int, id)
__field(size_t, buf_len)
__dynamic_array(u8, buf, buf_len)
+ __field(int ret)
),
TP_fast_assign(
__entry->id = id;
__entry->buf_len = buf_len;
+ __entry->ret = ret;
memcpy(__get_dynamic_array(buf), buf, buf_len);
),
TP_printk(
- "id %d len %zu",
+ "id %d len %zu ret %d",
__entry->id,
- __entry->buf_len
+ __entry->buf_len,
+ __entry->ret
)
);
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index 32fd5e7..66cd892 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -90,13 +90,12 @@ static void ath10k_wmi_htc_tx_complete(struct ath10k *ar, struct sk_buff *skb)
wake_up(&ar->wmi.wq);
}
-/* WMI command API */
-static int ath10k_wmi_cmd_send(struct ath10k *ar, struct sk_buff *skb,
- enum wmi_cmd_id cmd_id)
+static int ath10k_wmi_cmd_send_nowait(struct ath10k *ar, struct sk_buff *skb,
+ enum wmi_cmd_id cmd_id)
{
struct ath10k_skb_cb *skb_cb = ATH10K_SKB_CB(skb);
struct wmi_cmd_hdr *cmd_hdr;
- int status;
+ int ret;
u32 cmd = 0;
if (skb_push(skb, sizeof(struct wmi_cmd_hdr)) == NULL)
@@ -107,26 +106,40 @@ static int ath10k_wmi_cmd_send(struct ath10k *ar, struct sk_buff *skb,
cmd_hdr = (struct wmi_cmd_hdr *)skb->data;
cmd_hdr->cmd_id = __cpu_to_le32(cmd);
- if (atomic_add_return(1, &ar->wmi.pending_tx_count) >
- WMI_MAX_PENDING_TX_COUNT) {
- /* avoid using up memory when FW hangs */
- dev_kfree_skb(skb);
- atomic_dec(&ar->wmi.pending_tx_count);
- return -EBUSY;
- }
-
memset(skb_cb, 0, sizeof(*skb_cb));
+ ret = ath10k_htc_send(&ar->htc, ar->wmi.eid, skb);
+ trace_ath10k_wmi_cmd(cmd_id, skb->data, skb->len, ret);
- trace_ath10k_wmi_cmd(cmd_id, skb->data, skb->len);
+ if (ret)
+ goto err_pull;
- status = ath10k_htc_send(&ar->htc, ar->wmi.eid, skb);
- if (status) {
+ return 0;
+
+err_pull:
+ skb_pull(skb, sizeof(struct wmi_cmd_hdr));
+ return ret;
+}
+
+static void ath10k_wmi_op_ep_tx_credits(struct ath10k *ar,
+ enum ath10k_htc_ep_id eid)
+{
+ wake_up(&ar->wmi.tx_credits_wq);
+}
+
+static int ath10k_wmi_cmd_send(struct ath10k *ar, struct sk_buff *skb,
+ enum wmi_cmd_id cmd_id)
+{
+ int ret = -EINVAL;
+
+ wait_event_timeout(ar->wmi.tx_credits_wq, ({
+ ret = ath10k_wmi_cmd_send_nowait(ar, skb, cmd_id);
+ (ret != -EAGAIN);
+ }), 3*HZ);
+
+ if (ret)
dev_kfree_skb_any(skb);
- atomic_dec(&ar->wmi.pending_tx_count);
- return status;
- }
- return 0;
+ return ret;
}
static int ath10k_wmi_event_scan(struct ath10k *ar, struct sk_buff *skb)
@@ -1168,7 +1181,6 @@ static void ath10k_wmi_process_rx(struct ath10k *ar, struct sk_buff *skb)
/* some events require to be handled ASAP
* thus can't be defered to a worker thread */
switch (event_id) {
- case WMI_HOST_SWBA_EVENTID:
case WMI_MGMT_RX_EVENTID:
ath10k_wmi_event_process(ar, skb);
return;
@@ -1186,6 +1198,7 @@ int ath10k_wmi_attach(struct ath10k *ar)
init_completion(&ar->wmi.service_ready);
init_completion(&ar->wmi.unified_ready);
init_waitqueue_head(&ar->wmi.wq);
+ init_waitqueue_head(&ar->wmi.tx_credits_wq);
skb_queue_head_init(&ar->wmi.wmi_event_list);
INIT_WORK(&ar->wmi.wmi_event_work, ath10k_wmi_event_work);
@@ -1215,6 +1228,7 @@ int ath10k_wmi_connect_htc_service(struct ath10k *ar)
/* these fields are the same for all service endpoints */
conn_req.ep_ops.ep_tx_complete = ath10k_wmi_htc_tx_complete;
conn_req.ep_ops.ep_rx_complete = ath10k_wmi_process_rx;
+ conn_req.ep_ops.ep_tx_credits = ath10k_wmi_op_ep_tx_credits;
/* connect to control service */
conn_req.service_id = ATH10K_HTC_SVC_ID_WMI_CONTROL;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCHv2 4/7] ath10k: simplify HTC command submitting
2013-09-13 12:16 ` [PATCHv2 " Michal Kazior
` (2 preceding siblings ...)
2013-09-13 12:16 ` [PATCHv2 3/7] ath10k: make WMI commands block by design Michal Kazior
@ 2013-09-13 12:16 ` Michal Kazior
2013-09-13 12:16 ` [PATCHv2 5/7] ath10k: improve beacon submission latency Michal Kazior
` (3 subsequent siblings)
7 siblings, 0 replies; 21+ messages in thread
From: Michal Kazior @ 2013-09-13 12:16 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless, Michal Kazior
The patch removes HTC endpoint tx workers in
favour of direct command submission. This makes a
lot more sense for data path.
mac80211 queues are effectively stopped/woken up
in a more timely fashion preventing build up of
frames. It's possible to push more traffic than
the device/system is able to handle and have no
hiccups or performance degradation with UDP
traffic.
WMI commands will now report errors properly and
possibly block as they actively can wait for tx
credits to become available.
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
v2:
* use DIV_ROUND_UP
drivers/net/wireless/ath/ath10k/htc.c | 176 ++++++++-------------------------
drivers/net/wireless/ath/ath10k/htc.h | 4 -
drivers/net/wireless/ath/ath10k/wmi.c | 3 +-
3 files changed, 43 insertions(+), 140 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/htc.c b/drivers/net/wireless/ath/ath10k/htc.c
index 49da4e5..d0d7212 100644
--- a/drivers/net/wireless/ath/ath10k/htc.c
+++ b/drivers/net/wireless/ath/ath10k/htc.c
@@ -117,99 +117,13 @@ static void ath10k_htc_prepare_tx_skb(struct ath10k_htc_ep *ep,
spin_unlock_bh(&ep->htc->tx_lock);
}
-static int ath10k_htc_issue_skb(struct ath10k_htc *htc,
- struct ath10k_htc_ep *ep,
- struct sk_buff *skb,
- u8 credits)
-{
- struct ath10k_skb_cb *skb_cb = ATH10K_SKB_CB(skb);
- int ret;
-
- ath10k_dbg(ATH10K_DBG_HTC, "%s: ep %d skb %p\n", __func__,
- ep->eid, skb);
-
- ath10k_htc_prepare_tx_skb(ep, skb);
-
- ret = ath10k_skb_map(htc->ar->dev, skb);
- if (ret)
- goto err;
-
- ret = ath10k_hif_send_head(htc->ar,
- ep->ul_pipe_id,
- ep->eid,
- skb->len,
- skb);
- if (unlikely(ret))
- goto err;
-
- return 0;
-err:
- ath10k_warn("HTC issue failed: %d\n", ret);
-
- spin_lock_bh(&htc->tx_lock);
- ep->tx_credits += credits;
- spin_unlock_bh(&htc->tx_lock);
-
- if (ep->ep_ops.ep_tx_credits)
- ep->ep_ops.ep_tx_credits(htc->ar);
-
- /* 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);
-
- return ret;
-}
-
-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;
- u8 credits = 0;
- int ret;
-
- while (true) {
- if (ep->ul_is_polled)
- ath10k_htc_send_complete_check(ep, 0);
-
- spin_lock_bh(&htc->tx_lock);
- skb = __skb_dequeue(&ep->tx_queue);
-
- if (ep->tx_credit_flow_enabled) {
- credits = DIV_ROUND_UP(skb->len,
- htc->target_credit_size);
- if (ep->tx_credits < credits) {
- __skb_queue_head(&ep->tx_queue, skb);
- skb = NULL;
- }
- }
- spin_unlock_bh(&htc->tx_lock);
-
- if (!skb)
- break;
-
- ret = ath10k_htc_issue_skb(htc, ep, skb, credits);
- if (ret == -ENOSR)
- break;
- }
-}
-
int ath10k_htc_send(struct ath10k_htc *htc,
enum ath10k_htc_ep_id eid,
struct sk_buff *skb)
{
struct ath10k_htc_ep *ep = &htc->endpoint[eid];
+ int credits = 0;
+ int ret;
if (htc->ar->state == ATH10K_STATE_WEDGED)
return -ECOMM;
@@ -219,18 +133,55 @@ int ath10k_htc_send(struct ath10k_htc *htc,
return -ENOENT;
}
+ /* FIXME: This looks ugly, can we fix it? */
spin_lock_bh(&htc->tx_lock);
if (htc->stopped) {
spin_unlock_bh(&htc->tx_lock);
return -ESHUTDOWN;
}
+ spin_unlock_bh(&htc->tx_lock);
- __skb_queue_tail(&ep->tx_queue, skb);
skb_push(skb, sizeof(struct ath10k_htc_hdr));
- spin_unlock_bh(&htc->tx_lock);
- queue_work(htc->ar->workqueue, &ep->send_work);
+ if (ep->tx_credit_flow_enabled) {
+ credits = DIV_ROUND_UP(skb->len, htc->target_credit_size);
+ spin_lock_bh(&htc->tx_lock);
+ if (ep->tx_credits < credits) {
+ spin_unlock_bh(&htc->tx_lock);
+ ret = -EAGAIN;
+ goto err_pull;
+ }
+ ep->tx_credits -= credits;
+ spin_unlock_bh(&htc->tx_lock);
+ }
+
+ ath10k_htc_prepare_tx_skb(ep, skb);
+
+ ret = ath10k_skb_map(htc->ar->dev, skb);
+ if (ret)
+ goto err_credits;
+
+ ret = ath10k_hif_send_head(htc->ar, ep->ul_pipe_id, ep->eid,
+ skb->len, skb);
+ if (ret)
+ goto err_unmap;
+
return 0;
+
+err_unmap:
+ ath10k_skb_unmap(htc->ar->dev, skb);
+err_credits:
+ if (ep->tx_credit_flow_enabled) {
+ spin_lock_bh(&htc->tx_lock);
+ ep->tx_credits += credits;
+ spin_unlock_bh(&htc->tx_lock);
+
+ if (ep->ep_ops.ep_tx_credits)
+ ep->ep_ops.ep_tx_credits(htc->ar);
+ }
+err_pull:
+ skb_pull(skb, sizeof(struct ath10k_htc_hdr));
+ return ret;
}
static int ath10k_htc_tx_completion_handler(struct ath10k *ar,
@@ -243,39 +194,9 @@ static int ath10k_htc_tx_completion_handler(struct ath10k *ar,
ath10k_htc_notify_tx_completion(ep, skb);
/* the skb now belongs to the completion handler */
- /* note: when using TX credit flow, the re-checking of queues happens
- * when credits flow back from the target. in the non-TX credit case,
- * we recheck after the packet completes */
- spin_lock_bh(&htc->tx_lock);
- if (!ep->tx_credit_flow_enabled && !htc->stopped)
- queue_work(ar->workqueue, &ep->send_work);
- spin_unlock_bh(&htc->tx_lock);
-
return 0;
}
-/* flush endpoint TX queue */
-static void ath10k_htc_flush_endpoint_tx(struct ath10k_htc *htc,
- struct ath10k_htc_ep *ep)
-{
- struct sk_buff *skb;
- struct ath10k_skb_cb *skb_cb;
-
- spin_lock_bh(&htc->tx_lock);
- for (;;) {
- skb = __skb_dequeue(&ep->tx_queue);
- if (!skb)
- break;
-
- skb_cb = ATH10K_SKB_CB(skb);
- skb_cb->is_aborted = true;
- ath10k_htc_notify_tx_completion(ep, skb);
- }
- spin_unlock_bh(&htc->tx_lock);
-
- cancel_work_sync(&ep->send_work);
-}
-
/***********/
/* Receive */
/***********/
@@ -310,9 +231,6 @@ ath10k_htc_process_credit_report(struct ath10k_htc *htc,
ep->ep_ops.ep_tx_credits(htc->ar);
spin_lock_bh(&htc->tx_lock);
}
-
- if (ep->tx_credits && !skb_queue_empty(&ep->tx_queue))
- queue_work(htc->ar->workqueue, &ep->send_work);
}
spin_unlock_bh(&htc->tx_lock);
}
@@ -570,10 +488,8 @@ static void ath10k_htc_reset_endpoint_states(struct ath10k_htc *htc)
ep->max_ep_message_len = 0;
ep->max_tx_queue_depth = 0;
ep->eid = i;
- skb_queue_head_init(&ep->tx_queue);
ep->htc = htc;
ep->tx_credit_flow_enabled = true;
- INIT_WORK(&ep->send_work, ath10k_htc_send_work);
}
}
@@ -916,18 +832,10 @@ int ath10k_htc_start(struct ath10k_htc *htc)
*/
void ath10k_htc_stop(struct ath10k_htc *htc)
{
- int i;
- struct ath10k_htc_ep *ep;
-
spin_lock_bh(&htc->tx_lock);
htc->stopped = true;
spin_unlock_bh(&htc->tx_lock);
- for (i = ATH10K_HTC_EP_0; i < ATH10K_HTC_EP_COUNT; i++) {
- ep = &htc->endpoint[i];
- ath10k_htc_flush_endpoint_tx(htc, ep);
- }
-
ath10k_hif_stop(htc->ar);
}
diff --git a/drivers/net/wireless/ath/ath10k/htc.h b/drivers/net/wireless/ath/ath10k/htc.h
index 92ca29b..4716d33 100644
--- a/drivers/net/wireless/ath/ath10k/htc.h
+++ b/drivers/net/wireless/ath/ath10k/htc.h
@@ -316,15 +316,11 @@ struct ath10k_htc_ep {
int ul_is_polled; /* call HIF to get tx completions */
int dl_is_polled; /* call HIF to fetch rx (not implemented) */
- struct sk_buff_head tx_queue;
-
u8 seq_no; /* for debugging */
int tx_credits;
int tx_credit_size;
int tx_credits_per_max_message;
bool tx_credit_flow_enabled;
-
- struct work_struct send_work;
};
struct ath10k_htc_svc_tx_credits {
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index 66cd892..ff407c2 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -120,8 +120,7 @@ err_pull:
return ret;
}
-static void ath10k_wmi_op_ep_tx_credits(struct ath10k *ar,
- enum ath10k_htc_ep_id eid)
+static void ath10k_wmi_op_ep_tx_credits(struct ath10k *ar)
{
wake_up(&ar->wmi.tx_credits_wq);
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCHv2 5/7] ath10k: improve beacon submission latency
2013-09-13 12:16 ` [PATCHv2 " Michal Kazior
` (3 preceding siblings ...)
2013-09-13 12:16 ` [PATCHv2 4/7] ath10k: simplify HTC command submitting Michal Kazior
@ 2013-09-13 12:16 ` Michal Kazior
2013-09-13 12:16 ` [PATCHv2 6/7] ath10k: remove wmi pending count limit Michal Kazior
` (2 subsequent siblings)
7 siblings, 0 replies; 21+ messages in thread
From: Michal Kazior @ 2013-09-13 12:16 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless, Michal Kazior
The patch prevents beacon misses in some case of
heavy load on a system.
If a beacon can't be transmitted directly from an
SWBA event it will be left in arvif->beacon and
transmission will be retried once TX credits
become available.
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
v2:
* add comments on ath10k_wmi_tx_beacons_nowait() usage
drivers/net/wireless/ath/ath10k/core.h | 1 +
drivers/net/wireless/ath/ath10k/mac.c | 7 +++
drivers/net/wireless/ath/ath10k/wmi.c | 74 ++++++++++++++++++++++++++------
drivers/net/wireless/ath/ath10k/wmi.h | 3 +-
4 files changed, 71 insertions(+), 14 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index c953a33..14b7d3d 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -204,6 +204,7 @@ struct ath10k_vif {
enum wmi_vdev_subtype vdev_subtype;
u32 beacon_interval;
u32 dtim_period;
+ struct sk_buff *beacon;
struct ath10k *ar;
struct ieee80211_vif *vif;
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 8b9fb66..6c3e9d1 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -2075,6 +2075,13 @@ static void ath10k_remove_interface(struct ieee80211_hw *hw,
mutex_lock(&ar->conf_mutex);
+ spin_lock_bh(&ar->data_lock);
+ if (arvif->beacon) {
+ dev_kfree_skb_any(arvif->beacon);
+ arvif->beacon = NULL;
+ }
+ spin_unlock_bh(&ar->data_lock);
+
ar->free_vdev_map |= 1 << (arvif->vdev_id);
if (arvif->vdev_type == WMI_VDEV_TYPE_AP) {
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index ff407c2..9152dae 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -120,8 +120,53 @@ err_pull:
return ret;
}
+static void ath10k_wmi_tx_beacon_nowait(struct ath10k_vif *arvif)
+{
+ struct wmi_bcn_tx_arg arg = {0};
+ int ret;
+
+ lockdep_assert_held(&arvif->ar->data_lock);
+
+ if (arvif->beacon == NULL)
+ return;
+
+ arg.vdev_id = arvif->vdev_id;
+ arg.tx_rate = 0;
+ arg.tx_power = 0;
+ arg.bcn = arvif->beacon->data;
+ arg.bcn_len = arvif->beacon->len;
+
+ ret = ath10k_wmi_beacon_send_nowait(arvif->ar, &arg);
+ if (ret)
+ return;
+
+ dev_kfree_skb_any(arvif->beacon);
+ arvif->beacon = NULL;
+}
+
+static void ath10k_wmi_tx_beacons_iter(void *data, u8 *mac,
+ struct ieee80211_vif *vif)
+{
+ struct ath10k_vif *arvif = ath10k_vif_to_arvif(vif);
+
+ ath10k_wmi_tx_beacon_nowait(arvif);
+}
+
+static void ath10k_wmi_tx_beacons_nowait(struct ath10k *ar)
+{
+ spin_lock_bh(&ar->data_lock);
+ ieee80211_iterate_active_interfaces_atomic(ar->hw,
+ IEEE80211_IFACE_ITER_NORMAL,
+ ath10k_wmi_tx_beacons_iter,
+ NULL);
+ spin_unlock_bh(&ar->data_lock);
+}
+
static void ath10k_wmi_op_ep_tx_credits(struct ath10k *ar)
{
+ /* try to send pending beacons first. they take priority */
+ ath10k_wmi_tx_beacons_nowait(ar);
+
wake_up(&ar->wmi.tx_credits_wq);
}
@@ -131,6 +176,9 @@ static int ath10k_wmi_cmd_send(struct ath10k *ar, struct sk_buff *skb,
int ret = -EINVAL;
wait_event_timeout(ar->wmi.tx_credits_wq, ({
+ /* try to send pending beacons first. they take priority */
+ ath10k_wmi_tx_beacons_nowait(ar);
+
ret = ath10k_wmi_cmd_send_nowait(ar, skb, cmd_id);
(ret != -EAGAIN);
}), 3*HZ);
@@ -760,10 +808,8 @@ static void ath10k_wmi_event_host_swba(struct ath10k *ar, struct sk_buff *skb)
int i = -1;
struct wmi_bcn_info *bcn_info;
struct ath10k_vif *arvif;
- struct wmi_bcn_tx_arg arg;
struct sk_buff *bcn;
int vdev_id = 0;
- int ret;
ath10k_dbg(ATH10K_DBG_MGMT, "WMI_HOST_SWBA_EVENTID\n");
@@ -820,17 +866,17 @@ static void ath10k_wmi_event_host_swba(struct ath10k *ar, struct sk_buff *skb)
ath10k_wmi_update_tim(ar, arvif, bcn, bcn_info);
ath10k_wmi_update_noa(ar, arvif, bcn, bcn_info);
- arg.vdev_id = arvif->vdev_id;
- arg.tx_rate = 0;
- arg.tx_power = 0;
- arg.bcn = bcn->data;
- arg.bcn_len = bcn->len;
+ spin_lock_bh(&ar->data_lock);
+ if (arvif->beacon) {
+ ath10k_warn("SWBA overrun on vdev %d\n",
+ arvif->vdev_id);
+ dev_kfree_skb_any(arvif->beacon);
+ }
- ret = ath10k_wmi_beacon_send(ar, &arg);
- if (ret)
- ath10k_warn("could not send beacon (%d)\n", ret);
+ arvif->beacon = bcn;
- dev_kfree_skb_any(bcn);
+ ath10k_wmi_tx_beacon_nowait(arvif);
+ spin_unlock_bh(&ar->data_lock);
}
}
@@ -1181,6 +1227,7 @@ static void ath10k_wmi_process_rx(struct ath10k *ar, struct sk_buff *skb)
* thus can't be defered to a worker thread */
switch (event_id) {
case WMI_MGMT_RX_EVENTID:
+ case WMI_HOST_SWBA_EVENTID:
ath10k_wmi_event_process(ar, skb);
return;
default:
@@ -2138,7 +2185,8 @@ int ath10k_wmi_peer_assoc(struct ath10k *ar,
return ath10k_wmi_cmd_send(ar, skb, WMI_PEER_ASSOC_CMDID);
}
-int ath10k_wmi_beacon_send(struct ath10k *ar, const struct wmi_bcn_tx_arg *arg)
+int ath10k_wmi_beacon_send_nowait(struct ath10k *ar,
+ const struct wmi_bcn_tx_arg *arg)
{
struct wmi_bcn_tx_cmd *cmd;
struct sk_buff *skb;
@@ -2154,7 +2202,7 @@ int ath10k_wmi_beacon_send(struct ath10k *ar, const struct wmi_bcn_tx_arg *arg)
cmd->hdr.bcn_len = __cpu_to_le32(arg->bcn_len);
memcpy(cmd->bcn, arg->bcn, arg->bcn_len);
- return ath10k_wmi_cmd_send(ar, skb, WMI_BCN_TX_CMDID);
+ return ath10k_wmi_cmd_send_nowait(ar, skb, WMI_BCN_TX_CMDID);
}
static void ath10k_wmi_pdev_set_wmm_param(struct wmi_wmm_params *params,
diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
index ab46582..b100431 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.h
+++ b/drivers/net/wireless/ath/ath10k/wmi.h
@@ -3110,7 +3110,8 @@ int ath10k_wmi_set_ap_ps_param(struct ath10k *ar, u32 vdev_id, const u8 *mac,
enum wmi_ap_ps_peer_param param_id, u32 value);
int ath10k_wmi_scan_chan_list(struct ath10k *ar,
const struct wmi_scan_chan_list_arg *arg);
-int ath10k_wmi_beacon_send(struct ath10k *ar, const struct wmi_bcn_tx_arg *arg);
+int ath10k_wmi_beacon_send_nowait(struct ath10k *ar,
+ const struct wmi_bcn_tx_arg *arg);
int ath10k_wmi_pdev_set_wmm_params(struct ath10k *ar,
const struct wmi_pdev_set_wmm_params_arg *arg);
int ath10k_wmi_request_stats(struct ath10k *ar, enum wmi_stats_id stats_id);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCHv2 6/7] ath10k: remove wmi pending count limit
2013-09-13 12:16 ` [PATCHv2 " Michal Kazior
` (4 preceding siblings ...)
2013-09-13 12:16 ` [PATCHv2 5/7] ath10k: improve beacon submission latency Michal Kazior
@ 2013-09-13 12:16 ` Michal Kazior
2013-09-13 12:16 ` [PATCHv2 7/7] ath10k: remove wmi event worker thread Michal Kazior
2013-09-16 17:05 ` [PATCHv2 0/7] ath10k: improve TX path Kalle Valo
7 siblings, 0 replies; 21+ messages in thread
From: Michal Kazior @ 2013-09-13 12:16 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless, Michal Kazior
It is no longer used nor necessary since WMI
commands can block.
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
drivers/net/wireless/ath/ath10k/core.h | 2 --
drivers/net/wireless/ath/ath10k/mac.c | 9 ---------
drivers/net/wireless/ath/ath10k/wmi.c | 32 --------------------------------
drivers/net/wireless/ath/ath10k/wmi.h | 2 --
4 files changed, 45 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 14b7d3d..c2b6a76 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -112,8 +112,6 @@ struct ath10k_wmi {
enum ath10k_htc_ep_id eid;
struct completion service_ready;
struct completion unified_ready;
- atomic_t pending_tx_count;
- wait_queue_head_t wq;
wait_queue_head_t tx_credits_wq;
struct sk_buff_head wmi_event_list;
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 6c3e9d1..11aa13e 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -1224,8 +1224,6 @@ static void ath10k_bss_disassoc(struct ieee80211_hw *hw,
/* FIXME: why don't we print error if wmi call fails? */
ret = ath10k_wmi_vdev_down(ar, arvif->vdev_id);
- ath10k_wmi_flush_tx(ar);
-
arvif->def_wep_key_index = 0;
}
@@ -1664,8 +1662,6 @@ static int ath10k_abort_scan(struct ath10k *ar)
return -EIO;
}
- ath10k_wmi_flush_tx(ar);
-
ret = wait_for_completion_timeout(&ar->scan.completed, 3*HZ);
if (ret == 0)
ath10k_warn("timed out while waiting for scan to stop\n");
@@ -1699,10 +1695,6 @@ static int ath10k_start_scan(struct ath10k *ar,
if (ret)
return ret;
- /* make sure we submit the command so the completion
- * timeout makes sense */
- ath10k_wmi_flush_tx(ar);
-
ret = wait_for_completion_timeout(&ar->scan.started, 1*HZ);
if (ret == 0) {
ath10k_abort_scan(ar);
@@ -1924,7 +1916,6 @@ static int ath10k_config(struct ieee80211_hw *hw, u32 changed)
ret = ath10k_monitor_destroy(ar);
}
- ath10k_wmi_flush_tx(ar);
mutex_unlock(&ar->conf_mutex);
return ret;
}
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index 9152dae..b29d2b9 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -23,30 +23,6 @@
#include "wmi.h"
#include "mac.h"
-void ath10k_wmi_flush_tx(struct ath10k *ar)
-{
- int ret;
-
- lockdep_assert_held(&ar->conf_mutex);
-
- if (ar->state == ATH10K_STATE_WEDGED) {
- ath10k_warn("wmi flush skipped - device is wedged anyway\n");
- return;
- }
-
- ret = wait_event_timeout(ar->wmi.wq,
- atomic_read(&ar->wmi.pending_tx_count) == 0,
- 5*HZ);
- if (atomic_read(&ar->wmi.pending_tx_count) == 0)
- return;
-
- if (ret == 0)
- ret = -ETIMEDOUT;
-
- if (ret < 0)
- ath10k_warn("wmi flush failed (%d)\n", ret);
-}
-
int ath10k_wmi_wait_for_service_ready(struct ath10k *ar)
{
int ret;
@@ -85,9 +61,6 @@ static struct sk_buff *ath10k_wmi_alloc_skb(u32 len)
static void ath10k_wmi_htc_tx_complete(struct ath10k *ar, struct sk_buff *skb)
{
dev_kfree_skb(skb);
-
- if (atomic_sub_return(1, &ar->wmi.pending_tx_count) == 0)
- wake_up(&ar->wmi.wq);
}
static int ath10k_wmi_cmd_send_nowait(struct ath10k *ar, struct sk_buff *skb,
@@ -1243,7 +1216,6 @@ int ath10k_wmi_attach(struct ath10k *ar)
{
init_completion(&ar->wmi.service_ready);
init_completion(&ar->wmi.unified_ready);
- init_waitqueue_head(&ar->wmi.wq);
init_waitqueue_head(&ar->wmi.tx_credits_wq);
skb_queue_head_init(&ar->wmi.wmi_event_list);
@@ -1254,10 +1226,6 @@ int ath10k_wmi_attach(struct ath10k *ar)
void ath10k_wmi_detach(struct ath10k *ar)
{
- /* HTC should've drained the packets already */
- if (WARN_ON(atomic_read(&ar->wmi.pending_tx_count) > 0))
- ath10k_warn("there are still pending packets\n");
-
cancel_work_sync(&ar->wmi.wmi_event_work);
skb_queue_purge(&ar->wmi.wmi_event_list);
}
diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
index b100431..2c52c23 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.h
+++ b/drivers/net/wireless/ath/ath10k/wmi.h
@@ -3044,7 +3044,6 @@ struct wmi_force_fw_hang_cmd {
#define WMI_MAX_EVENT 0x1000
/* Maximum number of pending TXed WMI packets */
-#define WMI_MAX_PENDING_TX_COUNT 128
#define WMI_SKB_HEADROOM sizeof(struct wmi_cmd_hdr)
/* By default disable power save for IBSS */
@@ -3057,7 +3056,6 @@ int ath10k_wmi_attach(struct ath10k *ar);
void ath10k_wmi_detach(struct ath10k *ar);
int ath10k_wmi_wait_for_service_ready(struct ath10k *ar);
int ath10k_wmi_wait_for_unified_ready(struct ath10k *ar);
-void ath10k_wmi_flush_tx(struct ath10k *ar);
int ath10k_wmi_connect_htc_service(struct ath10k *ar);
int ath10k_wmi_pdev_set_channel(struct ath10k *ar,
--
1.7.9.5
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCHv2 7/7] ath10k: remove wmi event worker thread
2013-09-13 12:16 ` [PATCHv2 " Michal Kazior
` (5 preceding siblings ...)
2013-09-13 12:16 ` [PATCHv2 6/7] ath10k: remove wmi pending count limit Michal Kazior
@ 2013-09-13 12:16 ` Michal Kazior
2013-09-16 17:05 ` [PATCHv2 0/7] ath10k: improve TX path Kalle Valo
7 siblings, 0 replies; 21+ messages in thread
From: Michal Kazior @ 2013-09-13 12:16 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless, Michal Kazior
It's not really necessary to have this processed
in a worker. There are no sleepable calls (and
actually shouldn't be).
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
drivers/net/wireless/ath/ath10k/core.h | 3 ---
drivers/net/wireless/ath/ath10k/wmi.c | 44 +-------------------------------
2 files changed, 1 insertion(+), 46 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index c2b6a76..fcf94ee 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -113,9 +113,6 @@ struct ath10k_wmi {
struct completion service_ready;
struct completion unified_ready;
wait_queue_head_t tx_credits_wq;
-
- struct sk_buff_head wmi_event_list;
- struct work_struct wmi_event_work;
};
struct ath10k_peer_stat {
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index b29d2b9..6803ead 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -1055,7 +1055,7 @@ static int ath10k_wmi_ready_event_rx(struct ath10k *ar, struct sk_buff *skb)
return 0;
}
-static void ath10k_wmi_event_process(struct ath10k *ar, struct sk_buff *skb)
+static void ath10k_wmi_process_rx(struct ath10k *ar, struct sk_buff *skb)
{
struct wmi_cmd_hdr *cmd_hdr;
enum wmi_event_id id;
@@ -1174,43 +1174,6 @@ static void ath10k_wmi_event_process(struct ath10k *ar, struct sk_buff *skb)
dev_kfree_skb(skb);
}
-static void ath10k_wmi_event_work(struct work_struct *work)
-{
- struct ath10k *ar = container_of(work, struct ath10k,
- wmi.wmi_event_work);
- struct sk_buff *skb;
-
- for (;;) {
- skb = skb_dequeue(&ar->wmi.wmi_event_list);
- if (!skb)
- break;
-
- ath10k_wmi_event_process(ar, skb);
- }
-}
-
-static void ath10k_wmi_process_rx(struct ath10k *ar, struct sk_buff *skb)
-{
- struct wmi_cmd_hdr *cmd_hdr = (struct wmi_cmd_hdr *)skb->data;
- enum wmi_event_id event_id;
-
- event_id = MS(__le32_to_cpu(cmd_hdr->cmd_id), WMI_CMD_HDR_CMD_ID);
-
- /* some events require to be handled ASAP
- * thus can't be defered to a worker thread */
- switch (event_id) {
- case WMI_MGMT_RX_EVENTID:
- case WMI_HOST_SWBA_EVENTID:
- ath10k_wmi_event_process(ar, skb);
- return;
- default:
- break;
- }
-
- skb_queue_tail(&ar->wmi.wmi_event_list, skb);
- queue_work(ar->workqueue, &ar->wmi.wmi_event_work);
-}
-
/* WMI Initialization functions */
int ath10k_wmi_attach(struct ath10k *ar)
{
@@ -1218,16 +1181,11 @@ int ath10k_wmi_attach(struct ath10k *ar)
init_completion(&ar->wmi.unified_ready);
init_waitqueue_head(&ar->wmi.tx_credits_wq);
- skb_queue_head_init(&ar->wmi.wmi_event_list);
- INIT_WORK(&ar->wmi.wmi_event_work, ath10k_wmi_event_work);
-
return 0;
}
void ath10k_wmi_detach(struct ath10k *ar)
{
- cancel_work_sync(&ar->wmi.wmi_event_work);
- skb_queue_purge(&ar->wmi.wmi_event_list);
}
int ath10k_wmi_connect_htc_service(struct ath10k *ar)
--
1.7.9.5
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCHv2 0/7] ath10k: improve TX path
2013-09-13 12:16 ` [PATCHv2 " Michal Kazior
` (6 preceding siblings ...)
2013-09-13 12:16 ` [PATCHv2 7/7] ath10k: remove wmi event worker thread Michal Kazior
@ 2013-09-16 17:05 ` Kalle Valo
7 siblings, 0 replies; 21+ messages in thread
From: Kalle Valo @ 2013-09-16 17:05 UTC (permalink / raw)
To: Michal Kazior; +Cc: ath10k, linux-wireless
Michal Kazior <michal.kazior@tieto.com> writes:
> Hi,
>
> This patchset addresses two issues:
>
> * system/userspace starvation on heavy briding
> UDP TX
> * unstable/inconsistent UDP TX throughput
>
> In short the patchset simplifies TX path by
> removing HTC TX workers, makes WMI commands block
> and makes ath10k more responsive to queues
> becoming full. This contributes to both improved
> throughput and makes the system more responsive
> under heavy UDP TX load.
>
> Max stable briding TX (ath10k as TX):
> UDP w/o patchset: 520mbps
> UDP w/ patchset: 570mbps
>
> TCP w/o patchset: 350mbps
> TCP w/ patchset: 400mbps
>
> Measured with two 2x2 cards, one acting as an AP
> on AP135 board, the other as a STA on a core i5
> laptop. The AP135 was passing traffic from a
> station on LAN/eth1 to the STA/wlan0.
>
> v2:
> * use DIV_ROUND_UP
> * comment usage of ath10k_wmi_tx_beacons_nowait
> * additional check for tx_credits_flow_enabled
>
>
> Michal Kazior (7):
> ath10k: simplify HTC credits calculation
> ath10k: add HTC TX credits replenishing notification
> ath10k: make WMI commands block by design
> ath10k: simplify HTC command submitting
> ath10k: improve beacon submission latency
> ath10k: remove wmi pending count limit
> ath10k: remove wmi event worker thread
Thanks, applied.
--
Kalle Valo
^ permalink raw reply [flat|nested] 21+ messages in thread