[PATCH v5] ath9k: Switch to using mac80211 intermediate software queues.
This switches ath9k over to using the mac80211 intermediate software
queueing mechanism for data packets. It removes the queueing inside the
driver, except for the retry queue, and instead pulls from mac80211 when
a packet is needed. The retry queue is used to store a packet that was
pulled but can't be sent immediately.
The old code path in ath_tx_start that would queue packets has been
removed completely, as has the qlen limit tunables (since there's no
longer a queue in the driver to limit).
Based on Tim's original patch set, but reworked quite thoroughly.
Cc: Tim Shepard
Cc: Felix Fietkau
Signed-off-by: Toke Høiland-Jørgensen
---
Changes since v4:
- Fix regression where PS response frames (which go through the old TX
path) would not get assigned a seqno because the tid variable was not
assigned correctly. Many thanks to Felix for debugging this and coming
up with a fix.
Hopefully that is the last regression (apart from the ongoing mac80211
restructure). :)
drivers/net/wireless/ath/ath9k/ath9k.h | 27 ++-
drivers/net/wireless/ath/ath9k/channel.c | 2 -
drivers/net/wireless/ath/ath9k/debug.c | 14 +-
drivers/net/wireless/ath/ath9k/debug.h | 2 -
drivers/net/wireless/ath/ath9k/debug_sta.c | 4 +-
drivers/net/wireless/ath/ath9k/init.c | 2 +-
drivers/net/wireless/ath/ath9k/main.c | 9 +-
drivers/net/wireless/ath/ath9k/xmit.c | 338 -
8 files changed, 163 insertions(+), 235 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h
b/drivers/net/wireless/ath/ath9k/ath9k.h
index 26fc8ec..378d345 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -91,7 +91,6 @@ int ath_descdma_setup(struct ath_softc *sc, struct
ath_descdma *dd,
#define ATH_RXBUF 512
#define ATH_TXBUF 512
#define ATH_TXBUF_RESERVE 5
-#define ATH_MAX_QDEPTH (ATH_TXBUF / 4 - ATH_TXBUF_RESERVE)
#define ATH_TXMAXTRY13
#define ATH_MAX_SW_RETRIES 30
@@ -145,7 +144,7 @@ int ath_descdma_setup(struct ath_softc *sc, struct
ath_descdma *dd,
#define BAW_WITHIN(_start, _bawsz, _seqno) \
_seqno) - (_start)) & 4095) < (_bawsz))
-#define ATH_AN_2_TID(_an, _tidno) (&(_an)->tid[(_tidno)])
+#define ATH_AN_2_TID(_an, _tidno) ath_node_to_tid(_an, _tidno)
#define IS_HT_RATE(rate) (rate & 0x80)
#define IS_CCK_RATE(rate) ((rate >= 0x18) && (rate <= 0x1e))
@@ -164,7 +163,6 @@ struct ath_txq {
spinlock_t axq_lock;
u32 axq_depth;
u32 axq_ampdu_depth;
- bool stopped;
bool axq_tx_inprogress;
struct list_head txq_fifo[ATH_TXFIFO_DEPTH];
u8 txq_headidx;
@@ -232,7 +230,6 @@ struct ath_buf {
struct ath_atx_tid {
struct list_head list;
- struct sk_buff_head buf_q;
struct sk_buff_head retry_q;
struct ath_node *an;
struct ath_txq *txq;
@@ -247,13 +244,13 @@ struct ath_atx_tid {
s8 bar_index;
bool active;
bool clear_ps_filter;
+ bool has_queued;
};
struct ath_node {
struct ath_softc *sc;
struct ieee80211_sta *sta; /* station struct we're part of */
struct ieee80211_vif *vif; /* interface with which we're associated */
- struct ath_atx_tid tid[IEEE80211_NUM_TIDS];
u16 maxampdu;
u8 mpdudensity;
@@ -276,7 +273,6 @@ struct ath_tx_control {
struct ath_node *an;
struct ieee80211_sta *sta;
u8 paprd;
- bool force_channel;
};
@@ -293,7 +289,6 @@ struct ath_tx {
struct ath_descdma txdma;
struct ath_txq *txq_map[IEEE80211_NUM_ACS];
struct ath_txq *uapsdq;
- u32 txq_max_pending[IEEE80211_NUM_ACS];
u16 max_aggr_framelen[IEEE80211_NUM_ACS][4][32];
};
@@ -421,6 +416,22 @@ struct ath_offchannel {
int duration;
};
+static inline struct ath_atx_tid *
+ath_node_to_tid(struct ath_node *an, u8 tidno)
+{
+ struct ieee80211_sta *sta = an->sta;
+ struct ieee80211_vif *vif = an->vif;
+ struct ieee80211_txq *txq;
+
+ BUG_ON(!vif);
+ if (sta)
+ txq = sta->txq[tidno % ARRAY_SIZE(sta->txq)];
+ else
+ txq = vif->txq;
+
+ return (struct ath_atx_tid *) txq->drv_priv;
+}
+
#define case_rtn_string(val) case val: return #val
#define ath_for_each_chanctx(_sc, _ctx) \
@@ -575,7 +586,6 @@ void ath_tx_edma_tasklet(struct ath_softc *sc);
int ath_tx_aggr_start(struct ath_softc *sc, struct ieee80211_sta *sta,
u16 tid, u16 *ssn);
void ath_tx_aggr_stop(struct ath_softc *sc, struct ieee80211_sta *sta, u16
tid);
-void ath_tx_aggr_resume(struct ath_softc *sc, struct ieee80211_sta *sta, u16
tid);
void ath_tx_aggr_wakeup(struct ath_softc *sc, struct ath_node *an);
void ath_tx_aggr_sleep(struct ieee80211_sta *sta, struct ath_softc *sc,
@@ -585,6 +595,7 @@ void ath9k_release_buffered_frames(struct ieee80211_hw *h
Re: [v5] ath9k: Switch to using mac80211 intermediate software queues.
Toke Høiland-Jørgensen wrote: > This switches ath9k over to using the mac80211 intermediate software > queueing mechanism for data packets. It removes the queueing inside the > driver, except for the retry queue, and instead pulls from mac80211 when > a packet is needed. The retry queue is used to store a packet that was > pulled but can't be sent immediately. > > The old code path in ath_tx_start that would queue packets has been > removed completely, as has the qlen limit tunables (since there's no > longer a queue in the driver to limit). > > Based on Tim's original patch set, but reworked quite thoroughly. > > Cc: Tim Shepard > Cc: Felix Fietkau > Signed-off-by: Toke Høiland-Jørgensen > Signed-off-by: Felix Fietkau Depends on: bb42f2d13ffc mac80211: Move reorder-sensitive TX handlers to after TXQ dequeue Patch set to Awaiting Upstream. -- https://patchwork.kernel.org/patch/9311037/ Documentation about submitting wireless patches and checking status from patchwork: https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [v5] ath9k: Switch to using mac80211 intermediate software queues.
Toke Høiland-Jørgensen wrote: > This switches ath9k over to using the mac80211 intermediate software > queueing mechanism for data packets. It removes the queueing inside the > driver, except for the retry queue, and instead pulls from mac80211 when > a packet is needed. The retry queue is used to store a packet that was > pulled but can't be sent immediately. > > The old code path in ath_tx_start that would queue packets has been > removed completely, as has the qlen limit tunables (since there's no > longer a queue in the driver to limit). > > Based on Tim's original patch set, but reworked quite thoroughly. > > Cc: Tim Shepard > Cc: Felix Fietkau > Signed-off-by: Toke Høiland-Jørgensen > Signed-off-by: Felix Fietkau All dependencies have trickled down to ath.git but unfortunately doesn't apply anymore, so please rebase. Hopefully the last time :) While at it, could you also add to the commit log some info how awesome this patch is from user's point of view and how it helps. For example, before and and after numbers and other results. error: patch failed: drivers/net/wireless/ath/ath9k/xmit.c:921 error: drivers/net/wireless/ath/ath9k/xmit.c: patch does not apply stg import: Diff does not apply cleanly Patch set to Changes Requested. -- https://patchwork.kernel.org/patch/9311037/ Documentation about submitting wireless patches and checking status from patchwork: https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [v5] ath9k: Switch to using mac80211 intermediate software queues.
> While at it, could you also add to the commit log some info how awesome this > patch is from user's point of view and how it helps. For example, before and > and after numbers and other results. That varies wildly, depending on many details of the scenario (including the wireless capabilities of the clients connected to the AP using this patch, and how far away those clients are). There's really not enough room in a commit message to explain enough to make any such claimed numbers reproducible. And, BTW, this patch alone is not really where the big improvement comes from. This just cuts ath9k over to use the new transmit-a-packet interface between the mac80211 layer and the wireless device driver. All the good work to improve latency is being done on the new mac80211 interface to the driver to transmit a packet. If you want numbers in a commit message, those numbers belong over on those commits, not on this ath9k commit, IMHO. And there's different patches that taken together achieve the best improvement, and in various subsets differing amounts of improvement, which again all depends on the scenarios. And all this is complicated by how often real users are involved in scenarios where this matters (hard to say). This mainly improves things when you have an ath9k interface with packets queued simultaneously to different wireless destinations, which is typically when the ath9k interface is serving as an AP. And I would expect similar improvements for any wireless driver cut over to use the new mac80211 interface. (This patch might be a helpful guide to other wireless driver maintainers wishing to see the same improvements other wireless drivers.) I hope that the above paragraphs were helpful. -Tim Shepard s...@alum.mit.edu
Re: [v5] ath9k: Switch to using mac80211 intermediate software queues.
Tim Shepard writes: >> While at it, could you also add to the commit log some info how awesome this >> patch is from user's point of view and how it helps. For example, before and >> and after numbers and other results. > > That varies wildly, depending on many details of the scenario > (including the wireless capabilities of the clients connected to the > AP using this patch, and how far away those clients are). There's > really not enough room in a commit message to explain enough to make > any such claimed numbers reproducible. I disagree; it's quite straightforward to demonstrate a gain from this on an ath9k access point. And while of course the mac80211 queues is the reason for this, the commit that uses it (i.e. this one) can explain that and include some numbers. I'll add that and resend :) -Toke
Re: [v5] ath9k: Switch to using mac80211 intermediate software queues.
Toke Høiland-Jørgensen writes: > Tim Shepard writes: > >>> While at it, could you also add to the commit log some info how awesome this >>> patch is from user's point of view and how it helps. For example, before and >>> and after numbers and other results. >> >> That varies wildly, depending on many details of the scenario >> (including the wireless capabilities of the clients connected to the >> AP using this patch, and how far away those clients are). There's >> really not enough room in a commit message to explain enough to make >> any such claimed numbers reproducible. > > I disagree; it's quite straightforward to demonstrate a gain from this > on an ath9k access point. And while of course the mac80211 queues is the > reason for this, the commit that uses it (i.e. this one) can explain > that and include some numbers. Yeah, so with that you are basically answering "Why?" part in the commit log so that others can also understand the motivation behind all this. > I'll add that and resend :) Thanks. And sorry for taking so long with this, I have been basically travelling most of the last month. I should get back home on Friday and things should get normal soon. -- Kalle Valo
Re: [PATCH v5] ath9k: Switch to using mac80211 intermediate software queues.
On 2016-09-02 16:00, Toke Høiland-Jørgensen wrote: > This switches ath9k over to using the mac80211 intermediate software > queueing mechanism for data packets. It removes the queueing inside the > driver, except for the retry queue, and instead pulls from mac80211 when > a packet is needed. The retry queue is used to store a packet that was > pulled but can't be sent immediately. > > The old code path in ath_tx_start that would queue packets has been > removed completely, as has the qlen limit tunables (since there's no > longer a queue in the driver to limit). > > Based on Tim's original patch set, but reworked quite thoroughly. > > Cc: Tim Shepard > Cc: Felix Fietkau > Signed-off-by: Toke Høiland-Jørgensen You can add: Signed-off-by: Felix Fietkau - Felix
