* [Make-wifi-fast] [PATCH 0/2] ath9k: Add airtime fairness scheduler
@ 2016-06-17 9:17 Toke Høiland-Jørgensen
2016-06-17 9:17 ` [Make-wifi-fast] [PATCH 1/2] ath9k: use mac80211 intermediate software queues Toke Høiland-Jørgensen
2016-06-17 9:17 ` [Make-wifi-fast] [PATCH 2/2] ath9k: Add a per-station airtime deficit scheduler Toke Høiland-Jørgensen
0 siblings, 2 replies; 9+ messages in thread
From: Toke Høiland-Jørgensen @ 2016-06-17 9:17 UTC (permalink / raw)
To: make-wifi-fast
(Re-send to make-wifi-fast list due to a busted MX record).
This is the second version of my airtime fairness patch. This version
has a somewhat reworked scheduler (now closer to the structure of
fq_codel) and a different way to measure RX airtime; and there's a
debugfs entry to control which airtime measurements to include in the
scheduling decisions. For a simple one-way UDP test, the scheduler
achieves pretty much perfect airtime share (by its own measure). There's
not much throughput difference in the UDP case, but TCP tests see a
moderate improvement. I'll write up something more detailed on the
performance measures over the weekend and post it in a separate mail.
This patch set is rebased to mac80211-next - which means it no longer
includes Michal's patch to disable qdiscs. I have retained my version of
Tim's patch to make ath9k use wake_tx_queue in this patch set. That
probably needs some work still, but I believe he is working on that. I
have not tested extensively with the mac80211 FQ-CoDel patches enabled,
but I expect them to be complementary to this.
Changes since the RFC version:
- The scheduler will now enforce fairness harder. The previous version
would refill the deficit of slow stations too fast in some cases.
- Change the way RX airtime is measured. For aggregates, the airtime is
now calculated as the difference between the rs->rs_tstamp of the
first and last frame in the aggregate. For non-aggregates, the
previous calculation from the packet size is retained.
- There is now an 'airtime_flags' debugfs entry which can be used to
control which airtime measures are accounted to the deficit. If bit 0
is set, TX airtime will be accounted, and if bit 1 is set, RX airtime
will. If no bits are set, the scheduler will revert to simple
round-robin scheduling. The default is enabling both TX and RX.
- Squashed the whole thing into one patch and rebased to mac80211-next.
Toke Høiland-Jørgensen (2):
ath9k: use mac80211 intermediate software queues
ath9k: Add a per-station airtime deficit scheduler
drivers/net/wireless/ath/ath9k/ath9k.h | 34 +++-
drivers/net/wireless/ath/ath9k/channel.c | 12 +-
drivers/net/wireless/ath/ath9k/debug.c | 3 +
drivers/net/wireless/ath/ath9k/debug.h | 29 ++++
drivers/net/wireless/ath/ath9k/debug_sta.c | 53 +++++-
drivers/net/wireless/ath/ath9k/init.c | 2 +
drivers/net/wireless/ath/ath9k/main.c | 7 +-
drivers/net/wireless/ath/ath9k/recv.c | 60 +++++++
drivers/net/wireless/ath/ath9k/xmit.c | 255 ++++++++++++++++++++++-------
9 files changed, 386 insertions(+), 69 deletions(-)
--
2.8.3
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Make-wifi-fast] [PATCH 1/2] ath9k: use mac80211 intermediate software queues
2016-06-17 9:17 [Make-wifi-fast] [PATCH 0/2] ath9k: Add airtime fairness scheduler Toke Høiland-Jørgensen
@ 2016-06-17 9:17 ` Toke Høiland-Jørgensen
2016-06-17 9:17 ` [Make-wifi-fast] [PATCH 2/2] ath9k: Add a per-station airtime deficit scheduler Toke Høiland-Jørgensen
1 sibling, 0 replies; 9+ messages in thread
From: Toke Høiland-Jørgensen @ 2016-06-17 9:17 UTC (permalink / raw)
To: make-wifi-fast; +Cc: Toke Høiland-Jørgensen, Tim Shepard
This patch leaves the code for ath9k's internal per-node per-tid
queues in place and just modifies the driver to also pull from
the new mac80211 intermediate software queues, and implements
the .wake_tx_queue method, which will cause mac80211 to deliver
packets to be sent via the new intermediate queue.
Signed-off-by: Tim Shepard <shep@alum.mit.edu>
Reworked to not require the global variable renaming in ath9k.
Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
---
drivers/net/wireless/ath/ath9k/ath9k.h | 16 +++-
drivers/net/wireless/ath/ath9k/debug_sta.c | 7 +-
drivers/net/wireless/ath/ath9k/init.c | 1 +
drivers/net/wireless/ath/ath9k/main.c | 1 +
drivers/net/wireless/ath/ath9k/xmit.c | 119 +++++++++++++++++++++++++----
5 files changed, 125 insertions(+), 19 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index 93b3793..caeae10 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -145,8 +145,6 @@ 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 IS_HT_RATE(rate) (rate & 0x80)
#define IS_CCK_RATE(rate) ((rate >= 0x18) && (rate <= 0x1e))
#define IS_OFDM_RATE(rate) ((rate >= 0x8) && (rate <= 0xf))
@@ -232,8 +230,10 @@ struct ath_buf {
struct ath_atx_tid {
struct list_head list;
+ struct sk_buff_head i_q;
struct sk_buff_head buf_q;
struct sk_buff_head retry_q;
+ struct ieee80211_txq *swq;
struct ath_node *an;
struct ath_txq *txq;
unsigned long tx_buf[BITS_TO_LONGS(ATH_TID_MAX_BUFS)];
@@ -247,13 +247,13 @@ struct ath_atx_tid {
s8 bar_index;
bool active;
bool clear_ps_filter;
+ bool swq_nonempty;
};
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;
@@ -271,6 +271,15 @@ struct ath_node {
struct list_head list;
};
+static inline
+struct ath_atx_tid *ath_an_2_tid(struct ath_node *an, u8 tidno)
+{
+ struct ieee80211_sta *sta = an->sta;
+ struct ieee80211_vif *vif = an->vif;
+ struct ieee80211_txq *swq = sta ? sta->txq[tidno] : vif->txq;
+ return (struct ath_atx_tid *) swq->drv_priv;
+}
+
struct ath_tx_control {
struct ath_txq *txq;
struct ath_node *an;
@@ -585,6 +594,7 @@ void ath9k_release_buffered_frames(struct ieee80211_hw *hw,
u16 tids, int nframes,
enum ieee80211_frame_release_type reason,
bool more_data);
+void ath9k_wake_tx_queue(struct ieee80211_hw *hw, struct ieee80211_txq *swq);
/********/
/* VIFs */
diff --git a/drivers/net/wireless/ath/ath9k/debug_sta.c b/drivers/net/wireless/ath/ath9k/debug_sta.c
index b66cfa9..0e7f6b5 100644
--- a/drivers/net/wireless/ath/ath9k/debug_sta.c
+++ b/drivers/net/wireless/ath/ath9k/debug_sta.c
@@ -25,6 +25,7 @@ static ssize_t read_file_node_aggr(struct file *file, char __user *user_buf,
{
struct ath_node *an = file->private_data;
struct ath_softc *sc = an->sc;
+ struct ieee80211_txq *swq;
struct ath_atx_tid *tid;
struct ath_txq *txq;
u32 len = 0, size = 4096;
@@ -52,8 +53,10 @@ static ssize_t read_file_node_aggr(struct file *file, char __user *user_buf,
"TID", "SEQ_START", "SEQ_NEXT", "BAW_SIZE",
"BAW_HEAD", "BAW_TAIL", "BAR_IDX", "SCHED", "PAUSED");
- for (tidno = 0, tid = &an->tid[tidno];
- tidno < IEEE80211_NUM_TIDS; tidno++, tid++) {
+ for (tidno = 0;
+ tidno < IEEE80211_NUM_TIDS; tidno++) {
+ swq = an->sta->txq[tidno];
+ tid = (struct ath_atx_tid *) swq->drv_priv;
txq = tid->txq;
ath_txq_lock(sc, txq);
if (tid->active) {
diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index 2ee8624..211736c 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -873,6 +873,7 @@ static void ath9k_set_hw_capab(struct ath_softc *sc, struct ieee80211_hw *hw)
hw->max_rate_tries = 10;
hw->sta_data_size = sizeof(struct ath_node);
hw->vif_data_size = sizeof(struct ath_vif);
+ hw->txq_data_size = sizeof(struct ath_atx_tid);
hw->extra_tx_headroom = 4;
hw->wiphy->available_antennas_rx = BIT(ah->caps.max_rxchains) - 1;
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 8b63988..6ab56e5 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -2668,4 +2668,5 @@ struct ieee80211_ops ath9k_ops = {
.sw_scan_start = ath9k_sw_scan_start,
.sw_scan_complete = ath9k_sw_scan_complete,
.get_txpower = ath9k_get_txpower,
+ .wake_tx_queue = ath9k_wake_tx_queue,
};
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index 8ddd604..cdc8684 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -65,6 +65,8 @@ static struct ath_buf *ath_tx_setup_buffer(struct ath_softc *sc,
struct ath_txq *txq,
struct ath_atx_tid *tid,
struct sk_buff *skb);
+static int ath_tx_prepare(struct ieee80211_hw *hw, struct sk_buff *skb,
+ struct ath_tx_control *txctl);
enum {
MCS_HT20,
@@ -118,6 +120,21 @@ static void ath_tx_queue_tid(struct ath_softc *sc, struct ath_txq *txq,
list_add_tail(&tid->list, list);
}
+void ath9k_wake_tx_queue(struct ieee80211_hw *hw, struct ieee80211_txq *swq)
+{
+ struct ath_softc *sc = hw->priv;
+ struct ath_atx_tid *tid = (struct ath_atx_tid *) swq->drv_priv;
+ struct ath_txq *txq = tid->txq;
+
+ spin_lock_bh(&txq->axq_lock);
+
+ tid->swq_nonempty = true;
+ ath_tx_queue_tid(sc, txq, tid);
+ ath_txq_schedule(sc, txq);
+
+ spin_unlock_bh(&txq->axq_lock);
+}
+
static struct ath_frame_info *get_frame_info(struct sk_buff *skb)
{
struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(skb);
@@ -170,12 +187,51 @@ static struct ath_atx_tid *
ath_get_skb_tid(struct ath_softc *sc, struct ath_node *an, struct sk_buff *skb)
{
u8 tidno = skb->priority & IEEE80211_QOS_CTL_TID_MASK;
- return ATH_AN_2_TID(an, tidno);
+ return ath_an_2_tid(an, tidno);
}
+static void ath_swq_pull(struct ath_atx_tid *tid)
+{
+ struct sk_buff *skb;
+ struct ath_tx_control txctl;
+ struct ath_frame_info *fi;
+ int r;
+
+ if (!skb_queue_empty(&tid->i_q))
+ return;
+
+ if (!tid->swq_nonempty)
+ return;
+
+ skb = ieee80211_tx_dequeue(tid->an->sc->hw, tid->swq);
+ if (!skb) {
+ tid->swq_nonempty = false;
+ } else {
+ /* sad to do all this with axq_lock held */
+ memset(&txctl, 0, sizeof txctl);
+ txctl.txq = tid->txq;
+ txctl.sta = tid->an->sta;
+ r = ath_tx_prepare(tid->an->sc->hw, skb, &txctl);
+ if (WARN_ON(r != 0)) {
+ /** should not happen ??? */
+ } else {
+ /* perhaps not needed here ??? */
+ fi = get_frame_info(skb);
+ fi->txq = skb_get_queue_mapping(skb);
+
+ __skb_queue_tail(&tid->i_q, skb);
+ ++tid->txq->pending_frames;
+ }
+ }
+ }
+
+
static bool ath_tid_has_buffered(struct ath_atx_tid *tid)
{
- return !skb_queue_empty(&tid->buf_q) || !skb_queue_empty(&tid->retry_q);
+ if (!skb_queue_empty(&tid->buf_q) || !skb_queue_empty(&tid->retry_q) || !skb_queue_empty(&tid->i_q))
+ return true;
+ ath_swq_pull(tid);
+ return !skb_queue_empty(&tid->i_q);
}
static struct sk_buff *ath_tid_dequeue(struct ath_atx_tid *tid)
@@ -185,6 +241,12 @@ static struct sk_buff *ath_tid_dequeue(struct ath_atx_tid *tid)
skb = __skb_dequeue(&tid->retry_q);
if (!skb)
skb = __skb_dequeue(&tid->buf_q);
+ if (!skb)
+ skb = __skb_dequeue(&tid->i_q);
+ if (!skb) {
+ ath_swq_pull(tid);
+ skb = __skb_dequeue(&tid->i_q);
+ }
return skb;
}
@@ -870,6 +932,10 @@ ath_tx_get_tid_subframe(struct ath_softc *sc, struct ath_txq *txq,
*q = &tid->retry_q;
if (skb_queue_empty(*q))
*q = &tid->buf_q;
+ if (skb_queue_empty(*q))
+ *q = &tid->i_q;
+ if (skb_queue_empty(*q))
+ ath_swq_pull(tid);
skb = skb_peek(*q);
if (!skb)
@@ -1482,7 +1548,7 @@ int ath_tx_aggr_start(struct ath_softc *sc, struct ieee80211_sta *sta,
ath_dbg(common, XMIT, "%s called\n", __func__);
an = (struct ath_node *)sta->drv_priv;
- txtid = ATH_AN_2_TID(an, tid);
+ txtid = ath_an_2_tid(an, tid);
txq = txtid->txq;
ath_txq_lock(sc, txq);
@@ -1517,7 +1583,7 @@ void ath_tx_aggr_stop(struct ath_softc *sc, struct ieee80211_sta *sta, u16 tid)
{
struct ath_common *common = ath9k_hw_common(sc->sc_ah);
struct ath_node *an = (struct ath_node *)sta->drv_priv;
- struct ath_atx_tid *txtid = ATH_AN_2_TID(an, tid);
+ struct ath_atx_tid *txtid = ath_an_2_tid(an, tid);
struct ath_txq *txq = txtid->txq;
ath_dbg(common, XMIT, "%s called\n", __func__);
@@ -1533,6 +1599,7 @@ void ath_tx_aggr_sleep(struct ieee80211_sta *sta, struct ath_softc *sc,
struct ath_node *an)
{
struct ath_common *common = ath9k_hw_common(sc->sc_ah);
+ struct ieee80211_txq *swq;
struct ath_atx_tid *tid;
struct ath_txq *txq;
bool buffered;
@@ -1540,9 +1607,11 @@ void ath_tx_aggr_sleep(struct ieee80211_sta *sta, struct ath_softc *sc,
ath_dbg(common, XMIT, "%s called\n", __func__);
- for (tidno = 0, tid = &an->tid[tidno];
- tidno < IEEE80211_NUM_TIDS; tidno++, tid++) {
+ for (tidno = 0;
+ tidno < IEEE80211_NUM_TIDS; tidno++) {
+ swq = an->sta->txq[tidno];
+ tid = (struct ath_atx_tid *) swq->drv_priv;
txq = tid->txq;
ath_txq_lock(sc, txq);
@@ -1565,15 +1634,18 @@ void ath_tx_aggr_sleep(struct ieee80211_sta *sta, struct ath_softc *sc,
void ath_tx_aggr_wakeup(struct ath_softc *sc, struct ath_node *an)
{
struct ath_common *common = ath9k_hw_common(sc->sc_ah);
+ struct ieee80211_txq *swq;
struct ath_atx_tid *tid;
struct ath_txq *txq;
int tidno;
ath_dbg(common, XMIT, "%s called\n", __func__);
- for (tidno = 0, tid = &an->tid[tidno];
- tidno < IEEE80211_NUM_TIDS; tidno++, tid++) {
+ for (tidno = 0;
+ tidno < IEEE80211_NUM_TIDS; tidno++) {
+ swq = an->sta->txq[tidno];
+ tid = (struct ath_atx_tid *) swq->drv_priv;
txq = tid->txq;
ath_txq_lock(sc, txq);
@@ -1599,7 +1671,7 @@ void ath_tx_aggr_resume(struct ath_softc *sc, struct ieee80211_sta *sta,
ath_dbg(common, XMIT, "%s called\n", __func__);
an = (struct ath_node *)sta->drv_priv;
- tid = ATH_AN_2_TID(an, tidno);
+ tid = ath_an_2_tid(an, tidno);
txq = tid->txq;
ath_txq_lock(sc, txq);
@@ -1637,7 +1709,7 @@ void ath9k_release_buffered_frames(struct ieee80211_hw *hw,
if (!(tids & 1))
continue;
- tid = ATH_AN_2_TID(an, i);
+ tid = ath_an_2_tid(an, i);
ath_txq_lock(sc, tid->txq);
while (nframes > 0) {
@@ -2853,12 +2925,18 @@ int ath_tx_init(struct ath_softc *sc, int nbufs)
void ath_tx_node_init(struct ath_softc *sc, struct ath_node *an)
{
+ struct ieee80211_txq *swq;
+ struct ieee80211_sta *sta = an->sta;
+ struct ieee80211_vif *vif = an->vif;
struct ath_atx_tid *tid;
int tidno, acno;
- for (tidno = 0, tid = &an->tid[tidno];
+ for (tidno = 0;
tidno < IEEE80211_NUM_TIDS;
- tidno++, tid++) {
+ tidno++) {
+ swq = sta ? sta->txq[tidno] : vif->txq;
+ tid = (struct ath_atx_tid *) swq->drv_priv;
+ tid->swq = swq;
tid->an = an;
tid->tidno = tidno;
tid->seq_start = tid->seq_next = 0;
@@ -2866,23 +2944,33 @@ void ath_tx_node_init(struct ath_softc *sc, struct ath_node *an)
tid->baw_head = tid->baw_tail = 0;
tid->active = false;
tid->clear_ps_filter = true;
+ tid->swq_nonempty = false;
+ __skb_queue_head_init(&tid->i_q);
__skb_queue_head_init(&tid->buf_q);
__skb_queue_head_init(&tid->retry_q);
INIT_LIST_HEAD(&tid->list);
acno = TID_TO_WME_AC(tidno);
tid->txq = sc->tx.txq_map[acno];
+
+ if (!sta)
+ break; /* just one multicast ath_atx_tid */
}
}
void ath_tx_node_cleanup(struct ath_softc *sc, struct ath_node *an)
{
+ struct ieee80211_txq *swq;
+ struct ieee80211_sta *sta = an->sta;
+ struct ieee80211_vif *vif = an->vif;
struct ath_atx_tid *tid;
struct ath_txq *txq;
int tidno;
- for (tidno = 0, tid = &an->tid[tidno];
- tidno < IEEE80211_NUM_TIDS; tidno++, tid++) {
+ for (tidno = 0;
+ tidno < IEEE80211_NUM_TIDS; tidno++) {
+ swq = sta ? sta->txq[tidno] : vif->txq;
+ tid = (struct ath_atx_tid *) swq->drv_priv;
txq = tid->txq;
ath_txq_lock(sc, txq);
@@ -2894,6 +2982,9 @@ void ath_tx_node_cleanup(struct ath_softc *sc, struct ath_node *an)
tid->active = false;
ath_txq_unlock(sc, txq);
+
+ if (!sta)
+ break; /* just one multicast ath_atx_tid */
}
}
--
2.8.3
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Make-wifi-fast] [PATCH 2/2] ath9k: Add a per-station airtime deficit scheduler
2016-06-17 9:17 [Make-wifi-fast] [PATCH 0/2] ath9k: Add airtime fairness scheduler Toke Høiland-Jørgensen
2016-06-17 9:17 ` [Make-wifi-fast] [PATCH 1/2] ath9k: use mac80211 intermediate software queues Toke Høiland-Jørgensen
@ 2016-06-17 9:17 ` Toke Høiland-Jørgensen
1 sibling, 0 replies; 9+ messages in thread
From: Toke Høiland-Jørgensen @ 2016-06-17 9:17 UTC (permalink / raw)
To: make-wifi-fast
This modifies the logic in ath_txq_schedule to account airtime consumed
by each station and uses a deficit-based scheduler derived from FQ-CoDel
to try to enforce airtime fairness. A debugfs entry controls whether TX
airtime, RX airtime or both is accounted to the deficit on which the
scheduler makes decisions.
Uses the ts->duration + retry-chain information to account for time
spent transmitting to a station. The RX airtime is measured as the
duration from first to last frame in an aggregate, using the rs_tstamp
fields.
Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
---
drivers/net/wireless/ath/ath9k/ath9k.h | 18 +++-
drivers/net/wireless/ath/ath9k/channel.c | 12 ++-
drivers/net/wireless/ath/ath9k/debug.c | 3 +
drivers/net/wireless/ath/ath9k/debug.h | 29 ++++++
drivers/net/wireless/ath/ath9k/debug_sta.c | 46 ++++++++++
drivers/net/wireless/ath/ath9k/init.c | 1 +
drivers/net/wireless/ath/ath9k/main.c | 6 +-
drivers/net/wireless/ath/ath9k/recv.c | 60 +++++++++++++
drivers/net/wireless/ath/ath9k/xmit.c | 136 ++++++++++++++++++++---------
9 files changed, 261 insertions(+), 50 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index caeae10..e5a930c 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -261,9 +261,12 @@ struct ath_node {
bool sleeping;
bool no_ps_filter;
+ s64 airtime_deficit;
+ u32 airtime_rx_start;
#ifdef CONFIG_ATH9K_STATION_STATISTICS
struct ath_rx_rate_stats rx_rate_stats;
+ struct ath_airtime_stats airtime_stats;
#endif
u8 key_idx[4];
@@ -331,10 +334,15 @@ struct ath_rx {
/* Channel Context */
/*******************/
+struct ath_acq {
+ struct list_head acq_new;
+ struct list_head acq_old;
+};
+
struct ath_chanctx {
struct cfg80211_chan_def chandef;
struct list_head vifs;
- struct list_head acq[IEEE80211_NUM_ACS];
+ struct ath_acq acq[IEEE80211_NUM_ACS];
int hw_queue_base;
/* do not dereference, use for comparison only */
@@ -573,6 +581,8 @@ void ath_txq_schedule_all(struct ath_softc *sc);
int ath_tx_init(struct ath_softc *sc, int nbufs);
int ath_txq_update(struct ath_softc *sc, int qnum,
struct ath9k_tx_queue_info *q);
+u32 ath_pkt_duration(struct ath_softc *sc, u8 rix, int pktlen,
+ int width, int half_gi, bool shortPreamble);
void ath_update_max_aggr_framelen(struct ath_softc *sc, int queue, int txop);
void ath_assign_seq(struct ath_common *common, struct sk_buff *skb);
int ath_tx_start(struct ieee80211_hw *hw, struct sk_buff *skb,
@@ -959,6 +969,10 @@ void ath_ant_comb_scan(struct ath_softc *sc, struct ath_rx_status *rs);
#define ATH9K_NUM_CHANCTX 2 /* supports 2 operating channels */
+#define AIRTIME_USE_TX BIT(0)
+#define AIRTIME_USE_RX BIT(1)
+#define AIRTIME_ACTIVE(flags) (!!(flags & (AIRTIME_USE_TX|AIRTIME_USE_RX)))
+
struct ath_softc {
struct ieee80211_hw *hw;
struct device *dev;
@@ -1001,6 +1015,8 @@ struct ath_softc {
short nbcnvifs;
unsigned long ps_usecount;
+ u16 airtime_flags; /* AIRTIME_* */
+
struct ath_rx rx;
struct ath_tx tx;
struct ath_beacon beacon;
diff --git a/drivers/net/wireless/ath/ath9k/channel.c b/drivers/net/wireless/ath/ath9k/channel.c
index e56bafc..2594029 100644
--- a/drivers/net/wireless/ath/ath9k/channel.c
+++ b/drivers/net/wireless/ath/ath9k/channel.c
@@ -118,8 +118,10 @@ void ath_chanctx_init(struct ath_softc *sc)
INIT_LIST_HEAD(&ctx->vifs);
ctx->txpower = ATH_TXPOWER_MAX;
ctx->flush_timeout = HZ / 5; /* 200ms */
- for (j = 0; j < ARRAY_SIZE(ctx->acq); j++)
- INIT_LIST_HEAD(&ctx->acq[j]);
+ for (j = 0; j < ARRAY_SIZE(ctx->acq); j++) {
+ INIT_LIST_HEAD(&ctx->acq[j].acq_new);
+ INIT_LIST_HEAD(&ctx->acq[j].acq_old);
+ }
}
}
@@ -1344,8 +1346,10 @@ void ath9k_offchannel_init(struct ath_softc *sc)
ctx->txpower = ATH_TXPOWER_MAX;
cfg80211_chandef_create(&ctx->chandef, chan, NL80211_CHAN_HT20);
- for (i = 0; i < ARRAY_SIZE(ctx->acq); i++)
- INIT_LIST_HEAD(&ctx->acq[i]);
+ for (i = 0; i < ARRAY_SIZE(ctx->acq); i++) {
+ INIT_LIST_HEAD(&ctx->acq[i].acq_new);
+ INIT_LIST_HEAD(&ctx->acq[i].acq_old);
+ }
sc->offchannel.chan.offchannel = true;
}
diff --git a/drivers/net/wireless/ath/ath9k/debug.c b/drivers/net/wireless/ath/ath9k/debug.c
index c56e40f..413de3c 100644
--- a/drivers/net/wireless/ath/ath9k/debug.c
+++ b/drivers/net/wireless/ath/ath9k/debug.c
@@ -1411,5 +1411,8 @@ int ath9k_init_debug(struct ath_hw *ah)
debugfs_create_file("tpc", S_IRUSR | S_IWUSR,
sc->debug.debugfs_phy, sc, &fops_tpc);
+ debugfs_create_u16("airtime_flags", S_IRUSR | S_IWUSR,
+ sc->debug.debugfs_phy, &sc->airtime_flags);
+
return 0;
}
diff --git a/drivers/net/wireless/ath/ath9k/debug.h b/drivers/net/wireless/ath/ath9k/debug.h
index cd68c5f..bf1a540 100644
--- a/drivers/net/wireless/ath/ath9k/debug.h
+++ b/drivers/net/wireless/ath/ath9k/debug.h
@@ -223,6 +223,11 @@ struct ath_rx_rate_stats {
} cck_stats[4];
};
+struct ath_airtime_stats {
+ u32 rx_airtime;
+ u32 tx_airtime;
+};
+
#define ANT_MAIN 0
#define ANT_ALT 1
@@ -316,12 +321,36 @@ ath9k_debug_sync_cause(struct ath_softc *sc, u32 sync_cause)
void ath_debug_rate_stats(struct ath_softc *sc,
struct ath_rx_status *rs,
struct sk_buff *skb);
+void ath_debug_tx_airtime(struct ath_softc *sc,
+ struct ath_buf *bf,
+ struct ath_tx_status *ts);
+void ath_debug_rx_airtime(struct ath_softc *sc,
+ struct ath_rx_status *rs,
+ struct sk_buff *skb);
+void ath_debug_airtime(struct ath_softc *sc,
+ struct ath_node *an,
+ u32 rx, u32 tx);
#else
static inline void ath_debug_rate_stats(struct ath_softc *sc,
struct ath_rx_status *rs,
struct sk_buff *skb)
{
}
+static inline void ath_debug_tx_airtime(struct ath_softc *sc,
+ struct ath_buf *bf,
+ struct ath_tx_status *ts)
+{
+}
+static inline void ath_debug_rx_airtime(struct ath_softc *sc,
+ struct ath_rx_status *rs,
+ struct sk_buff *skb)
+{
+}
+static void ath_debug_airtime(struct ath_softc *sc,
+ struct ath_node *an,
+ u32 rx, u32 tx)
+{
+}
#endif /* CONFIG_ATH9K_STATION_STATISTICS */
#endif /* DEBUG_H */
diff --git a/drivers/net/wireless/ath/ath9k/debug_sta.c b/drivers/net/wireless/ath/ath9k/debug_sta.c
index 0e7f6b5..e7f2ef2 100644
--- a/drivers/net/wireless/ath/ath9k/debug_sta.c
+++ b/drivers/net/wireless/ath/ath9k/debug_sta.c
@@ -245,6 +245,51 @@ static const struct file_operations fops_node_recv = {
.llseek = default_llseek,
};
+void ath_debug_airtime(struct ath_softc *sc,
+ struct ath_node *an,
+ u32 rx,
+ u32 tx)
+{
+ struct ath_airtime_stats *astats = &an->airtime_stats;
+
+ astats->rx_airtime += rx;
+ astats->tx_airtime += tx;
+}
+
+static ssize_t read_airtime(struct file *file, char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ struct ath_node *an = file->private_data;
+ struct ath_airtime_stats *astats;
+ u32 len = 0, size = 128;
+ char *buf;
+ size_t retval;
+
+ buf = kzalloc(size, GFP_KERNEL);
+ if (buf == NULL)
+ return -ENOMEM;
+
+ astats = &an->airtime_stats;
+
+ len += scnprintf(buf + len, size - len, "RX: %u us\n", astats->rx_airtime);
+ len += scnprintf(buf + len, size - len, "TX: %u us\n", astats->tx_airtime);
+ len += scnprintf(buf + len, size - len, "Deficit: %lld us\n", an->airtime_deficit);
+
+ retval = simple_read_from_buffer(user_buf, count, ppos, buf, len);
+ kfree(buf);
+
+ return retval;
+}
+
+
+static const struct file_operations fops_airtime = {
+ .read = read_airtime,
+ .open = simple_open,
+ .owner = THIS_MODULE,
+ .llseek = default_llseek,
+};
+
+
void ath9k_sta_add_debugfs(struct ieee80211_hw *hw,
struct ieee80211_vif *vif,
struct ieee80211_sta *sta,
@@ -254,4 +299,5 @@ void ath9k_sta_add_debugfs(struct ieee80211_hw *hw,
debugfs_create_file("node_aggr", S_IRUGO, dir, an, &fops_node_aggr);
debugfs_create_file("node_recv", S_IRUGO, dir, an, &fops_node_recv);
+ debugfs_create_file("airtime", S_IRUGO, dir, an, &fops_airtime);
}
diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index 211736c..f4e9dd3 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -560,6 +560,7 @@ static int ath9k_init_softc(u16 devid, struct ath_softc *sc,
/* Will be cleared in ath9k_start() */
set_bit(ATH_OP_INVALID, &common->op_flags);
+ sc->airtime_flags = AIRTIME_USE_TX | AIRTIME_USE_RX;
sc->sc_ah = ah;
sc->dfs_detector = dfs_pattern_detector_init(common, NL80211_DFS_UNSET);
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 6ab56e5..e13068b 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -70,10 +70,10 @@ static bool ath9k_has_pending_frames(struct ath_softc *sc, struct ath_txq *txq,
goto out;
if (txq->mac80211_qnum >= 0) {
- struct list_head *list;
+ struct ath_acq *acq;
- list = &sc->cur_chan->acq[txq->mac80211_qnum];
- if (!list_empty(list))
+ acq = &sc->cur_chan->acq[txq->mac80211_qnum];
+ if (!list_empty(&acq->acq_new) || !list_empty(&acq->acq_old))
pending = true;
}
out:
diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
index 32160fc..a48667f 100644
--- a/drivers/net/wireless/ath/ath9k/recv.c
+++ b/drivers/net/wireless/ath/ath9k/recv.c
@@ -991,6 +991,65 @@ static void ath9k_apply_ampdu_details(struct ath_softc *sc,
}
}
+static void ath_rx_count_airtime(struct ath_softc *sc,
+ struct ath_rx_status *rs,
+ struct sk_buff *skb)
+{
+ struct ath_node *an;
+ struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
+ struct ath_hw *ah = sc->sc_ah;
+ struct ath_common *common = ath9k_hw_common(ah);
+ struct ieee80211_sta *sta;
+ struct ieee80211_rx_status *rxs;
+ const struct ieee80211_rate *rate;
+ bool is_sgi, is_40, is_sp;
+ int phy;
+ u32 airtime = 0;
+
+ if (!ieee80211_is_data(hdr->frame_control))
+ return;
+
+ rcu_read_lock();
+
+ sta = ieee80211_find_sta_by_ifaddr(sc->hw, hdr->addr2, NULL);
+ if (!sta)
+ goto exit;
+ an = (struct ath_node *) sta->drv_priv;
+
+ if (rs->rs_isaggr && rs->rs_firstaggr) {
+ an->airtime_rx_start = rs->rs_tstamp;
+ } else if (rs->rs_isaggr && !rs->rs_moreaggr && an->airtime_rx_start) {
+ airtime = rs->rs_tstamp - an->airtime_rx_start;
+ } else if (!rs->rs_isaggr) {
+ an->airtime_rx_start = 0;
+
+ rxs = IEEE80211_SKB_RXCB(skb);
+
+ is_sgi = !!(rxs->flag & RX_FLAG_SHORT_GI);
+ is_40 = !!(rxs->flag & RX_FLAG_40MHZ);
+ is_sp = !!(rxs->flag & RX_FLAG_SHORTPRE);
+
+ if (!!(rxs->flag & RX_FLAG_HT)) {
+ /* MCS rates */
+
+ airtime += ath_pkt_duration(sc, rxs->rate_idx, rs->rs_datalen,
+ is_40, is_sgi, is_sp);
+ } else {
+
+ phy = IS_CCK_RATE(rs->rs_rate) ? WLAN_RC_PHY_CCK : WLAN_RC_PHY_OFDM;
+ rate = &common->sbands[rxs->band].bitrates[rxs->rate_idx];
+ airtime += ath9k_hw_computetxtime(ah, phy, rate->bitrate * 100,
+ rs->rs_datalen, rxs->rate_idx, is_sp);
+ }
+ }
+
+ if (!!(sc->airtime_flags & AIRTIME_USE_RX))
+ an->airtime_deficit -= airtime;
+ ath_debug_airtime(sc, an, airtime, 0);
+exit:
+ rcu_read_unlock();
+}
+
int ath_rx_tasklet(struct ath_softc *sc, int flush, bool hp)
{
struct ath_rxbuf *bf;
@@ -1137,6 +1196,7 @@ int ath_rx_tasklet(struct ath_softc *sc, int flush, bool hp)
ath9k_antenna_check(sc, &rs);
ath9k_apply_ampdu_details(sc, &rs, rxs);
ath_debug_rate_stats(sc, &rs, skb);
+ ath_rx_count_airtime(sc, &rs, skb);
hdr = (struct ieee80211_hdr *)skb->data;
if (ieee80211_is_ack(hdr->frame_control))
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index cdc8684..ef0a4a1 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -108,16 +108,19 @@ void ath_txq_unlock_complete(struct ath_softc *sc, struct ath_txq *txq)
static void ath_tx_queue_tid(struct ath_softc *sc, struct ath_txq *txq,
struct ath_atx_tid *tid)
{
- struct list_head *list;
struct ath_vif *avp = (struct ath_vif *) tid->an->vif->drv_priv;
struct ath_chanctx *ctx = avp->chanctx;
+ struct ath_acq *acq;
+ struct list_head *tid_list;
if (!ctx)
return;
- list = &ctx->acq[TID_TO_WME_AC(tid->tidno)];
+
+ acq = &ctx->acq[TID_TO_WME_AC(tid->tidno)];
+ tid_list = AIRTIME_ACTIVE(sc->airtime_flags) ? &acq->acq_new : &acq->acq_old;
if (list_empty(&tid->list))
- list_add_tail(&tid->list, list);
+ list_add_tail(&tid->list, tid_list);
}
void ath9k_wake_tx_queue(struct ieee80211_hw *hw, struct ieee80211_txq *swq)
@@ -722,6 +725,48 @@ static bool bf_is_ampdu_not_probing(struct ath_buf *bf)
return bf_isampdu(bf) && !(info->flags & IEEE80211_TX_CTL_RATE_CTRL_PROBE);
}
+static void ath_tx_count_airtime(struct ath_softc *sc,
+ struct ath_buf *bf,
+ struct ath_tx_status *ts)
+{
+ struct ath_node *an;
+ struct sk_buff *skb;
+ struct ieee80211_hdr *hdr;
+ struct ieee80211_hw *hw = sc->hw;
+ struct ieee80211_tx_rate rates[4];
+ struct ieee80211_sta *sta;
+ int i;
+ u32 airtime = 0;
+
+ skb = bf->bf_mpdu;
+ if(!skb)
+ return;
+
+ hdr = (struct ieee80211_hdr *)skb->data;
+ memcpy(rates, bf->rates, sizeof(rates));
+
+ rcu_read_lock();
+
+ sta = ieee80211_find_sta_by_ifaddr(hw, hdr->addr1, hdr->addr2);
+ if(!sta)
+ goto exit;
+
+
+ an = (struct ath_node *) sta->drv_priv;
+
+ airtime += ts->duration * (ts->ts_longretry + 1);
+
+ for(i=0; i < ts->ts_rateindex; i++)
+ airtime += ath9k_hw_get_duration(sc->sc_ah, bf->bf_desc, i) * rates[i].count;
+
+ if (!!(sc->airtime_flags & AIRTIME_USE_TX))
+ an->airtime_deficit -= airtime;
+ ath_debug_airtime(sc, an, 0, airtime);
+
+exit:
+ rcu_read_unlock();
+}
+
static void ath_tx_process_buffer(struct ath_softc *sc, struct ath_txq *txq,
struct ath_tx_status *ts, struct ath_buf *bf,
struct list_head *bf_head)
@@ -739,6 +784,8 @@ static void ath_tx_process_buffer(struct ath_softc *sc, struct ath_txq *txq,
ts->duration = ath9k_hw_get_duration(sc->sc_ah, bf->bf_desc,
ts->ts_rateindex);
+ ath_tx_count_airtime(sc, bf, ts);
+
if (!bf_isampdu(bf)) {
if (!flush) {
info = IEEE80211_SKB_CB(bf->bf_mpdu);
@@ -751,6 +798,7 @@ static void ath_tx_process_buffer(struct ath_softc *sc, struct ath_txq *txq,
} else
ath_tx_complete_aggr(sc, txq, bf, bf_head, ts, txok);
+
if (!flush)
ath_txq_schedule(sc, txq);
}
@@ -1090,8 +1138,8 @@ ath_tx_form_aggr(struct ath_softc *sc, struct ath_txq *txq,
* width - 0 for 20 MHz, 1 for 40 MHz
* half_gi - to use 4us v/s 3.6 us for symbol time
*/
-static u32 ath_pkt_duration(struct ath_softc *sc, u8 rix, int pktlen,
- int width, int half_gi, bool shortPreamble)
+u32 ath_pkt_duration(struct ath_softc *sc, u8 rix, int pktlen,
+ int width, int half_gi, bool shortPreamble)
{
u32 nbits, nsymbits, duration, nsymbols;
int streams;
@@ -1490,7 +1538,7 @@ ath_tx_form_burst(struct ath_softc *sc, struct ath_txq *txq,
}
static bool ath_tx_sched_aggr(struct ath_softc *sc, struct ath_txq *txq,
- struct ath_atx_tid *tid, bool *stop)
+ struct ath_atx_tid *tid)
{
struct ath_buf *bf;
struct ieee80211_tx_info *tx_info;
@@ -1512,7 +1560,6 @@ static bool ath_tx_sched_aggr(struct ath_softc *sc, struct ath_txq *txq,
aggr = !!(tx_info->flags & IEEE80211_TX_CTL_AMPDU);
if ((aggr && txq->axq_ampdu_depth >= ATH_AGGR_MIN_QDEPTH) ||
(!aggr && txq->axq_depth >= ATH_NON_AGGR_MIN_QDEPTH)) {
- *stop = true;
return false;
}
@@ -1984,9 +2031,10 @@ void ath_tx_cleanupq(struct ath_softc *sc, struct ath_txq *txq)
void ath_txq_schedule(struct ath_softc *sc, struct ath_txq *txq)
{
struct ath_common *common = ath9k_hw_common(sc->sc_ah);
- struct ath_atx_tid *tid, *last_tid;
+ struct ath_atx_tid *tid;
struct list_head *tid_list;
- bool sent = false;
+ struct ath_acq *acq;
+ bool active = AIRTIME_ACTIVE(sc->airtime_flags);
if (txq->mac80211_qnum < 0)
return;
@@ -1995,48 +2043,50 @@ void ath_txq_schedule(struct ath_softc *sc, struct ath_txq *txq)
return;
spin_lock_bh(&sc->chan_lock);
- tid_list = &sc->cur_chan->acq[txq->mac80211_qnum];
+ acq = &sc->cur_chan->acq[txq->mac80211_qnum];
- if (list_empty(tid_list)) {
- spin_unlock_bh(&sc->chan_lock);
- return;
- }
+ if (sc->cur_chan->stopped)
+ goto out;
rcu_read_lock();
+begin:
+ tid_list = &acq->acq_new;
+ if (list_empty(tid_list)) {
+ tid_list = &acq->acq_old;
+ if (list_empty(tid_list))
+ goto out;
+ }
+ tid = list_first_entry(tid_list, struct ath_atx_tid, list);
- last_tid = list_entry(tid_list->prev, struct ath_atx_tid, list);
- while (!list_empty(tid_list)) {
- bool stop = false;
-
- if (sc->cur_chan->stopped)
- break;
-
- tid = list_first_entry(tid_list, struct ath_atx_tid, list);
- list_del_init(&tid->list);
-
- if (ath_tx_sched_aggr(sc, txq, tid, &stop))
- sent = true;
-
- /*
- * add tid to round-robin queue if more frames
- * are pending for the tid
- */
- if (ath_tid_has_buffered(tid))
- ath_tx_queue_tid(sc, txq, tid);
+ if (active && tid->an->airtime_deficit <= 0) {
+ tid->an->airtime_deficit += 300;
+ list_move_tail(&tid->list, &acq->acq_old);
+ goto begin;
+ }
- if (stop)
- break;
+ if (!ath_tid_has_buffered(tid)) {
+ if ((tid_list == &acq->acq_new) && !list_empty(&acq->acq_old))
+ list_move_tail(&tid->list, &acq->acq_old);
+ else
+ list_del_init(&tid->list);
+ goto begin;
+ }
- if (tid == last_tid) {
- if (!sent)
- break;
- sent = false;
- last_tid = list_entry(tid_list->prev,
- struct ath_atx_tid, list);
- }
+ /* If a station succeeds in queueing something, immediately restart the
+ * loop. This makes sure the queues are shuffled if the station now has
+ * no more packets queued, and also ensures we keep the hardware queues
+ * full.
+ *
+ * If we dequeued from a new queue, shuffle the queues, to prevent it
+ * from hogging too much airtime. */
+ if(ath_tx_sched_aggr(sc, txq, tid)) {
+ if (!active || ((tid_list == &acq->acq_new) && !list_empty(&acq->acq_old)))
+ list_move_tail(&tid->list, &acq->acq_old);
+ goto begin;
}
+out:
rcu_read_unlock();
spin_unlock_bh(&sc->chan_lock);
}
@@ -2931,6 +2981,8 @@ void ath_tx_node_init(struct ath_softc *sc, struct ath_node *an)
struct ath_atx_tid *tid;
int tidno, acno;
+ an->airtime_deficit = 300;
+
for (tidno = 0;
tidno < IEEE80211_NUM_TIDS;
tidno++) {
--
2.8.3
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Make-wifi-fast] [PATCH 1/2] ath9k: use mac80211 intermediate software queues
[not found] <E1bDxpG-0004Ic-00@www.xplot.org>
@ 2016-06-17 19:15 ` Toke Høiland-Jørgensen
0 siblings, 0 replies; 9+ messages in thread
From: Toke Høiland-Jørgensen @ 2016-06-17 19:15 UTC (permalink / raw)
To: Tim Shepard; +Cc: Felix Fietkau, linux-wireless, make-wifi-fast, ath9k-devel
Tim Shepard <shep@alum.mit.edu> writes:
> Hmm... if the renaming is going to go in mainline, I feel pretty
> strongly it should go in *before* a patch to switch over to use the
> intermediate queues. The whole point of the renaming was to make the
> code that uses the intermediate queues much more understandable
> (avoiding the unfortuante collision of "txq" meaning two different
> things throughout the code).
>
> Once it is all done and everyone's done reading and trying to
> understand this code, there's much less reason to do the renaming.
>
> Toke, how do you feel about this at this point?
I'm fine with not renaming things for now. Been looking at the current
code enough that it doesn't bother me.
Oh, and you can hide most of the ieee80211_txq stuff behind macros, so
it doesn't have to be all over the code. Makes the patch set smaller
too...
> I'm asking because I hope to have a new version of my patch soon
> (fixing a bug in how it handles tid->hwq->pending_frames and
> hq_max_pending[*] ),
Cool. I started looking into what it will take to do a full conversion
(getting rid of the old TX path). Not quite there yet (to say the
least), so if you have a less buggy base I can work from that would be
cool ;)
-Toke
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Make-wifi-fast] [PATCH 1/2] ath9k: use mac80211 intermediate software queues
2016-06-17 13:48 ` Felix Fietkau
@ 2016-06-17 16:33 ` Felix Fietkau
0 siblings, 0 replies; 9+ messages in thread
From: Felix Fietkau @ 2016-06-17 16:33 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: linux-wireless, make-wifi-fast, ath9k-devel, Tim Shepard
On 2016-06-17 15:48, Felix Fietkau wrote:
> On 2016-06-17 15:43, Toke Høiland-Jørgensen wrote:
>> Felix Fietkau <nbd@nbd.name> writes:
>>
>>> On 2016-06-17 11:09, Toke Høiland-Jørgensen wrote:
>>>> This patch leaves the code for ath9k's internal per-node per-tid
>>>> queues in place and just modifies the driver to also pull from
>>>> the new mac80211 intermediate software queues, and implements
>>>> the .wake_tx_queue method, which will cause mac80211 to deliver
>>>> packets to be sent via the new intermediate queue.
>>>>
>>>> Signed-off-by: Tim Shepard <shep@alum.mit.edu>
>>>>
>>>> Reworked to not require the global variable renaming in ath9k.
>>>>
>>>> Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
>>>> ---
>>>> drivers/net/wireless/ath/ath9k/ath9k.h | 16 +++-
>>>> drivers/net/wireless/ath/ath9k/debug_sta.c | 7 +-
>>>> drivers/net/wireless/ath/ath9k/init.c | 1 +
>>>> drivers/net/wireless/ath/ath9k/main.c | 1 +
>>>> drivers/net/wireless/ath/ath9k/xmit.c | 119 +++++++++++++++++++++++++----
>>>> 5 files changed, 125 insertions(+), 19 deletions(-)
>>>>
>>>> diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
>>>> index 93b3793..caeae10 100644
>>>> --- a/drivers/net/wireless/ath/ath9k/ath9k.h
>>>> +++ b/drivers/net/wireless/ath/ath9k/ath9k.h
>>>> @@ -145,8 +145,6 @@ 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 IS_HT_RATE(rate) (rate & 0x80)
>>>> #define IS_CCK_RATE(rate) ((rate >= 0x18) && (rate <= 0x1e))
>>>> #define IS_OFDM_RATE(rate) ((rate >= 0x8) && (rate <= 0xf))
>>>> @@ -232,8 +230,10 @@ struct ath_buf {
>>>>
>>>> struct ath_atx_tid {
>>>> struct list_head list;
>>>> + struct sk_buff_head i_q;
>>> Do we really need a third queue here? Instead of adding yet another
>>> layer of queueing here, I think we should even get rid of buf_q.
>>
>> This is definitely something that needs to be improved. One other
>> sticking point related to this: in the current version of this patch
>> ath_tid_has_buffered() gains a side effect of pulling from the mac80211
>> txq, which is obviously not so nice.
>>
>> The obvious way to get rid of this is to export a txq_has_buffered()
>> function at the mac80211 layer. But avoiding that may be possible; the
>> sticking point is what to do with the code paths that do not dequeue
>> packets, but check ath_tid_has_buffered() to decide whether to schedule
>> the queue and/or to tell ieee80211_sta_set_buffered() about it (these
>> are for instance ath_tx_aggr_sleep/wakeup(). Can those just be removed
>> (i.e. don't call into ieee80211, and always schedule the txq on wakeup?
>> I'm not familiar enough with the intermediate queues to make that
>> call...
> For tx scheduling, we can use swq_nonempty and deal with false positives.
> For power save we should only use ieee80211_sta_set_buffered if the
> driver itself has buffered some frames. Indication of packets in the
> mac80211 intermediate queue is already taken care of inside mac80211.
One more thing that I forgot in my previous reply: on PS wakeup, the
driver does not need to schedule the intermediate queues itself -
mac80211 will call drv_wake_tx_queue if frames are pending.
- Felix
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Make-wifi-fast] [PATCH 1/2] ath9k: use mac80211 intermediate software queues
[not found] <E1bDu1d-0007mR-00@www.xplot.org>
@ 2016-06-17 14:35 ` Felix Fietkau
0 siblings, 0 replies; 9+ messages in thread
From: Felix Fietkau @ 2016-06-17 14:35 UTC (permalink / raw)
To: Tim Shepard
Cc: Toke Høiland-Jørgensen, linux-wireless, make-wifi-fast,
ath9k-devel
On 2016-06-17 15:41, Tim Shepard wrote:
>> > diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
>> > index 93b3793..caeae10 100644
>> > --- a/drivers/net/wireless/ath/ath9k/ath9k.h
>> > +++ b/drivers/net/wireless/ath/ath9k/ath9k.h
>> > @@ -145,8 +145,6 @@ 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 IS_HT_RATE(rate) (rate & 0x80)
>> > #define IS_CCK_RATE(rate) ((rate >= 0x18) && (rate <= 0x1e))
>> > #define IS_OFDM_RATE(rate) ((rate >= 0x8) && (rate <= 0xf))
>> > @@ -232,8 +230,10 @@ struct ath_buf {
>> >
>> > struct ath_atx_tid {
>> > struct list_head list;
>> > + struct sk_buff_head i_q;
>> Do we really need a third queue here? Instead of adding yet another
>> layer of queueing here, I think we should even get rid of buf_q.
>>
>> Channel context based queue handling can be dealt with by
>> stopping/starting relevant queues on channel context changes.
>>
>> buf_q becomes unnecessary when you remove all code in the drv_tx
>> codepath that moves frames to the intermediate queue.
>>
>> Any frame that was pulled from the intermediate queue and prepared for
>> tx, but which can't be sent right now can simply be queued to retry_q.
>>
>> This will also help with getting the diffstat insertion/deletion ratio
>> under control ;)
>>
>> > struct sk_buff_head buf_q;
>> > struct sk_buff_head retry_q;
>> > + struct ieee80211_txq *swq;
>> No need for this pointer, you can use container_of.
>
>
> Felix, great to hear from you and thanks for your feedback. I will
> try to work on this.
>
> I was struggling to understand the channel context stuff, and I have
> no idea how to test it. (Is there anyone else listening who might be
> able to help with testing the channel context stuff as we improve this
> patch and simplify the ath9k driver's use of the new mac80211
> intermediate queues?)
>
>
> Felix, do you have any thoughts on the renaming of txq to hwx that I
> had done in my original version of this patch? I had a good e-mail
> discussion with Toke a week or two ago (cc these same various lists)
> and I believe he came to understand that perhaps the renaming I had
> done in the original version of this patch was worth doing.
>
> Now in Toke's version of my patch he calls the ieee80211 txq a "swq"
> and the ath9k hardware queue is called a "txq". (I had called the
> ieee80211 txq a "txq" and I renamed the ath9k hardware queue "hwq"
> throught all the ath9k driver code. This also made ath9k's names of
> things more similar to mt76 which I was looking at as an example of a
> driver that uses your new ieee80211 txq mechanism.
>
> I think the renaming is worth doing, but I also understand the
> renaming can be disruptive to others actively working on ath9k.
> It would be nice to have another opinion on this.
I think we should finish intermediate queues support first and then look
into the rename later.
- Felix
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Make-wifi-fast] [PATCH 1/2] ath9k: use mac80211 intermediate software queues
2016-06-17 13:43 ` Toke Høiland-Jørgensen
@ 2016-06-17 13:48 ` Felix Fietkau
2016-06-17 16:33 ` Felix Fietkau
0 siblings, 1 reply; 9+ messages in thread
From: Felix Fietkau @ 2016-06-17 13:48 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: linux-wireless, make-wifi-fast, ath9k-devel, Tim Shepard
On 2016-06-17 15:43, Toke Høiland-Jørgensen wrote:
> Felix Fietkau <nbd@nbd.name> writes:
>
>> On 2016-06-17 11:09, Toke Høiland-Jørgensen wrote:
>>> This patch leaves the code for ath9k's internal per-node per-tid
>>> queues in place and just modifies the driver to also pull from
>>> the new mac80211 intermediate software queues, and implements
>>> the .wake_tx_queue method, which will cause mac80211 to deliver
>>> packets to be sent via the new intermediate queue.
>>>
>>> Signed-off-by: Tim Shepard <shep@alum.mit.edu>
>>>
>>> Reworked to not require the global variable renaming in ath9k.
>>>
>>> Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
>>> ---
>>> drivers/net/wireless/ath/ath9k/ath9k.h | 16 +++-
>>> drivers/net/wireless/ath/ath9k/debug_sta.c | 7 +-
>>> drivers/net/wireless/ath/ath9k/init.c | 1 +
>>> drivers/net/wireless/ath/ath9k/main.c | 1 +
>>> drivers/net/wireless/ath/ath9k/xmit.c | 119 +++++++++++++++++++++++++----
>>> 5 files changed, 125 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
>>> index 93b3793..caeae10 100644
>>> --- a/drivers/net/wireless/ath/ath9k/ath9k.h
>>> +++ b/drivers/net/wireless/ath/ath9k/ath9k.h
>>> @@ -145,8 +145,6 @@ 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 IS_HT_RATE(rate) (rate & 0x80)
>>> #define IS_CCK_RATE(rate) ((rate >= 0x18) && (rate <= 0x1e))
>>> #define IS_OFDM_RATE(rate) ((rate >= 0x8) && (rate <= 0xf))
>>> @@ -232,8 +230,10 @@ struct ath_buf {
>>>
>>> struct ath_atx_tid {
>>> struct list_head list;
>>> + struct sk_buff_head i_q;
>> Do we really need a third queue here? Instead of adding yet another
>> layer of queueing here, I think we should even get rid of buf_q.
>
> This is definitely something that needs to be improved. One other
> sticking point related to this: in the current version of this patch
> ath_tid_has_buffered() gains a side effect of pulling from the mac80211
> txq, which is obviously not so nice.
>
> The obvious way to get rid of this is to export a txq_has_buffered()
> function at the mac80211 layer. But avoiding that may be possible; the
> sticking point is what to do with the code paths that do not dequeue
> packets, but check ath_tid_has_buffered() to decide whether to schedule
> the queue and/or to tell ieee80211_sta_set_buffered() about it (these
> are for instance ath_tx_aggr_sleep/wakeup(). Can those just be removed
> (i.e. don't call into ieee80211, and always schedule the txq on wakeup?
> I'm not familiar enough with the intermediate queues to make that
> call...
For tx scheduling, we can use swq_nonempty and deal with false positives.
For power save we should only use ieee80211_sta_set_buffered if the
driver itself has buffered some frames. Indication of packets in the
mac80211 intermediate queue is already taken care of inside mac80211.
- Felix
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Make-wifi-fast] [PATCH 1/2] ath9k: use mac80211 intermediate software queues
2016-06-17 13:28 ` [Make-wifi-fast] [PATCH 1/2] ath9k: use mac80211 intermediate software queues Felix Fietkau
@ 2016-06-17 13:43 ` Toke Høiland-Jørgensen
2016-06-17 13:48 ` Felix Fietkau
0 siblings, 1 reply; 9+ messages in thread
From: Toke Høiland-Jørgensen @ 2016-06-17 13:43 UTC (permalink / raw)
To: Felix Fietkau; +Cc: linux-wireless, make-wifi-fast, ath9k-devel, Tim Shepard
Felix Fietkau <nbd@nbd.name> writes:
> On 2016-06-17 11:09, Toke Høiland-Jørgensen wrote:
>> This patch leaves the code for ath9k's internal per-node per-tid
>> queues in place and just modifies the driver to also pull from
>> the new mac80211 intermediate software queues, and implements
>> the .wake_tx_queue method, which will cause mac80211 to deliver
>> packets to be sent via the new intermediate queue.
>>
>> Signed-off-by: Tim Shepard <shep@alum.mit.edu>
>>
>> Reworked to not require the global variable renaming in ath9k.
>>
>> Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
>> ---
>> drivers/net/wireless/ath/ath9k/ath9k.h | 16 +++-
>> drivers/net/wireless/ath/ath9k/debug_sta.c | 7 +-
>> drivers/net/wireless/ath/ath9k/init.c | 1 +
>> drivers/net/wireless/ath/ath9k/main.c | 1 +
>> drivers/net/wireless/ath/ath9k/xmit.c | 119 +++++++++++++++++++++++++----
>> 5 files changed, 125 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
>> index 93b3793..caeae10 100644
>> --- a/drivers/net/wireless/ath/ath9k/ath9k.h
>> +++ b/drivers/net/wireless/ath/ath9k/ath9k.h
>> @@ -145,8 +145,6 @@ 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 IS_HT_RATE(rate) (rate & 0x80)
>> #define IS_CCK_RATE(rate) ((rate >= 0x18) && (rate <= 0x1e))
>> #define IS_OFDM_RATE(rate) ((rate >= 0x8) && (rate <= 0xf))
>> @@ -232,8 +230,10 @@ struct ath_buf {
>>
>> struct ath_atx_tid {
>> struct list_head list;
>> + struct sk_buff_head i_q;
> Do we really need a third queue here? Instead of adding yet another
> layer of queueing here, I think we should even get rid of buf_q.
This is definitely something that needs to be improved. One other
sticking point related to this: in the current version of this patch
ath_tid_has_buffered() gains a side effect of pulling from the mac80211
txq, which is obviously not so nice.
The obvious way to get rid of this is to export a txq_has_buffered()
function at the mac80211 layer. But avoiding that may be possible; the
sticking point is what to do with the code paths that do not dequeue
packets, but check ath_tid_has_buffered() to decide whether to schedule
the queue and/or to tell ieee80211_sta_set_buffered() about it (these
are for instance ath_tx_aggr_sleep/wakeup(). Can those just be removed
(i.e. don't call into ieee80211, and always schedule the txq on wakeup?
I'm not familiar enough with the intermediate queues to make that
call...
> Channel context based queue handling can be dealt with by
> stopping/starting relevant queues on channel context changes.
Noted.
> buf_q becomes unnecessary when you remove all code in the drv_tx
> codepath that moves frames to the intermediate queue.
>
> Any frame that was pulled from the intermediate queue and prepared for
> tx, but which can't be sent right now can simply be queued to retry_q.
Right.
> This will also help with getting the diffstat insertion/deletion ratio
> under control ;)
Yes, that would be good ;)
>> struct sk_buff_head buf_q;
>> struct sk_buff_head retry_q;
>> + struct ieee80211_txq *swq;
> No need for this pointer, you can use container_of.
Ah, cool, thanks!
-Toke
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Make-wifi-fast] [PATCH 1/2] ath9k: use mac80211 intermediate software queues
[not found] ` <20160617090929.31606-2-toke@toke.dk>
@ 2016-06-17 13:28 ` Felix Fietkau
2016-06-17 13:43 ` Toke Høiland-Jørgensen
0 siblings, 1 reply; 9+ messages in thread
From: Felix Fietkau @ 2016-06-17 13:28 UTC (permalink / raw)
To: Toke Høiland-Jørgensen, linux-wireless, make-wifi-fast,
ath9k-devel
Cc: Tim Shepard
On 2016-06-17 11:09, Toke Høiland-Jørgensen wrote:
> This patch leaves the code for ath9k's internal per-node per-tid
> queues in place and just modifies the driver to also pull from
> the new mac80211 intermediate software queues, and implements
> the .wake_tx_queue method, which will cause mac80211 to deliver
> packets to be sent via the new intermediate queue.
>
> Signed-off-by: Tim Shepard <shep@alum.mit.edu>
>
> Reworked to not require the global variable renaming in ath9k.
>
> Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
> ---
> drivers/net/wireless/ath/ath9k/ath9k.h | 16 +++-
> drivers/net/wireless/ath/ath9k/debug_sta.c | 7 +-
> drivers/net/wireless/ath/ath9k/init.c | 1 +
> drivers/net/wireless/ath/ath9k/main.c | 1 +
> drivers/net/wireless/ath/ath9k/xmit.c | 119 +++++++++++++++++++++++++----
> 5 files changed, 125 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
> index 93b3793..caeae10 100644
> --- a/drivers/net/wireless/ath/ath9k/ath9k.h
> +++ b/drivers/net/wireless/ath/ath9k/ath9k.h
> @@ -145,8 +145,6 @@ 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 IS_HT_RATE(rate) (rate & 0x80)
> #define IS_CCK_RATE(rate) ((rate >= 0x18) && (rate <= 0x1e))
> #define IS_OFDM_RATE(rate) ((rate >= 0x8) && (rate <= 0xf))
> @@ -232,8 +230,10 @@ struct ath_buf {
>
> struct ath_atx_tid {
> struct list_head list;
> + struct sk_buff_head i_q;
Do we really need a third queue here? Instead of adding yet another
layer of queueing here, I think we should even get rid of buf_q.
Channel context based queue handling can be dealt with by
stopping/starting relevant queues on channel context changes.
buf_q becomes unnecessary when you remove all code in the drv_tx
codepath that moves frames to the intermediate queue.
Any frame that was pulled from the intermediate queue and prepared for
tx, but which can't be sent right now can simply be queued to retry_q.
This will also help with getting the diffstat insertion/deletion ratio
under control ;)
> struct sk_buff_head buf_q;
> struct sk_buff_head retry_q;
> + struct ieee80211_txq *swq;
No need for this pointer, you can use container_of.
- Felix
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-06-17 19:16 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-17 9:17 [Make-wifi-fast] [PATCH 0/2] ath9k: Add airtime fairness scheduler Toke Høiland-Jørgensen
2016-06-17 9:17 ` [Make-wifi-fast] [PATCH 1/2] ath9k: use mac80211 intermediate software queues Toke Høiland-Jørgensen
2016-06-17 9:17 ` [Make-wifi-fast] [PATCH 2/2] ath9k: Add a per-station airtime deficit scheduler Toke Høiland-Jørgensen
[not found] <20160617090929.31606-1-toke@toke.dk>
[not found] ` <20160617090929.31606-2-toke@toke.dk>
2016-06-17 13:28 ` [Make-wifi-fast] [PATCH 1/2] ath9k: use mac80211 intermediate software queues Felix Fietkau
2016-06-17 13:43 ` Toke Høiland-Jørgensen
2016-06-17 13:48 ` Felix Fietkau
2016-06-17 16:33 ` Felix Fietkau
[not found] <E1bDu1d-0007mR-00@www.xplot.org>
2016-06-17 14:35 ` Felix Fietkau
[not found] <E1bDxpG-0004Ic-00@www.xplot.org>
2016-06-17 19:15 ` Toke Høiland-Jørgensen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox