* [PATCH 1/2] mac80211: Add TXQ scheduling API
@ 2017-10-16 16:09 Toke Høiland-Jørgensen
2017-10-16 16:09 ` [PATCH 2/2] mac80211: Add airtime accounting and scheduling to TXQs Toke Høiland-Jørgensen
` (4 more replies)
0 siblings, 5 replies; 20+ messages in thread
From: Toke Høiland-Jørgensen @ 2017-10-16 16:09 UTC (permalink / raw)
To: make-wifi-fast, linux-wireless; +Cc: Toke Høiland-Jørgensen
This adds an API to mac80211 to handle scheduling of TXQs and changes the
interface between driver and mac80211 for TXQ handling as follows:
- The wake_tx_queue callback interface no longer includes the TXQ. Instead, the
driver is expected to retrieve that from ieee80211_next_txq()
- Two new mac80211 functions are added: ieee80211_next_txq() and
ieee80211_schedule_txq(). The former returns the next TXQ that should be
scheduled, and is how the driver gets a queue to pull packets from. The latter
is called internally by mac80211 to start scheduling a queue, and the driver
is supposed to call it to re-schedule the TXQ after it is finished pulling
packets from it (unless the queue emptied).
The ath9k and ath10k drivers are changed to use the new API.
Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
---
Changes since the RFC:
- Don't call wake_tx_queue() if the TXQ was already scheduled.
- Fold ath9k and ath9k patches into this one.
- Don't mess with the powersave buffering behaviour, but keep the old
driver behaviour for that (haven't tested with powersave enabled,
though...).
- A couple of adjustments to ath9k behaviour after testing.
I don't have ath10k hardware handy, so this is untested on ath10k.
However, ath10k required way less surgery to support the new API than
ath9k did....
drivers/net/wireless/ath/ath10k/core.c | 2 -
drivers/net/wireless/ath/ath10k/core.h | 3 -
drivers/net/wireless/ath/ath10k/mac.c | 54 +++------
drivers/net/wireless/ath/ath9k/ath9k.h | 6 +-
drivers/net/wireless/ath/ath9k/main.c | 2 +-
drivers/net/wireless/ath/ath9k/xmit.c | 209 ++++++++-------------------------
include/net/mac80211.h | 35 +++++-
net/mac80211/agg-tx.c | 6 +-
net/mac80211/driver-ops.h | 12 +-
net/mac80211/ieee80211_i.h | 4 +
net/mac80211/main.c | 3 +
net/mac80211/sta_info.c | 5 +-
net/mac80211/trace.h | 21 +---
net/mac80211/tx.c | 48 +++++++-
14 files changed, 167 insertions(+), 243 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index a4f635820f35..759df3297d48 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -2561,9 +2561,7 @@ struct ath10k *ath10k_core_create(size_t priv_size, struct device *dev,
mutex_init(&ar->conf_mutex);
spin_lock_init(&ar->data_lock);
- spin_lock_init(&ar->txqs_lock);
- INIT_LIST_HEAD(&ar->txqs);
INIT_LIST_HEAD(&ar->peers);
init_waitqueue_head(&ar->peer_mapping_wq);
init_waitqueue_head(&ar->htt.empty_tx_wq);
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 949ebb3e967b..e7fc241addf6 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -892,10 +892,7 @@ struct ath10k {
/* protects shared structure data */
spinlock_t data_lock;
- /* protects: ar->txqs, artxq->list */
- spinlock_t txqs_lock;
- struct list_head txqs;
struct list_head arvifs;
struct list_head peers;
struct ath10k_peer *peer_map[ATH10K_MAX_NUM_PEER_IDS];
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 5683f1a5330e..43ea23af8255 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -3825,7 +3825,6 @@ static void ath10k_mac_txq_init(struct ieee80211_txq *txq)
static void ath10k_mac_txq_unref(struct ath10k *ar, struct ieee80211_txq *txq)
{
- struct ath10k_txq *artxq;
struct ath10k_skb_cb *cb;
struct sk_buff *msdu;
int msdu_id;
@@ -3833,12 +3832,6 @@ static void ath10k_mac_txq_unref(struct ath10k *ar, struct ieee80211_txq *txq)
if (!txq)
return;
- artxq = (void *)txq->drv_priv;
- spin_lock_bh(&ar->txqs_lock);
- if (!list_empty(&artxq->list))
- list_del_init(&artxq->list);
- spin_unlock_bh(&ar->txqs_lock);
-
spin_lock_bh(&ar->htt.tx_lock);
idr_for_each_entry(&ar->htt.pending_tx, msdu, msdu_id) {
cb = ATH10K_SKB_CB(msdu);
@@ -3968,23 +3961,17 @@ int ath10k_mac_tx_push_txq(struct ieee80211_hw *hw,
void ath10k_mac_tx_push_pending(struct ath10k *ar)
{
struct ieee80211_hw *hw = ar->hw;
- struct ieee80211_txq *txq;
- struct ath10k_txq *artxq;
- struct ath10k_txq *last;
+ struct ieee80211_txq *txq, *first = NULL;
int ret;
int max;
if (ar->htt.num_pending_tx >= (ar->htt.max_num_pending_tx / 2))
return;
- spin_lock_bh(&ar->txqs_lock);
rcu_read_lock();
- last = list_last_entry(&ar->txqs, struct ath10k_txq, list);
- while (!list_empty(&ar->txqs)) {
- artxq = list_first_entry(&ar->txqs, struct ath10k_txq, list);
- txq = container_of((void *)artxq, struct ieee80211_txq,
- drv_priv);
+ txq = ieee80211_next_txq(hw);
+ while (txq) {
/* Prevent aggressive sta/tid taking over tx queue */
max = 16;
@@ -3995,18 +3982,21 @@ void ath10k_mac_tx_push_pending(struct ath10k *ar)
break;
}
- list_del_init(&artxq->list);
if (ret != -ENOENT)
- list_add_tail(&artxq->list, &ar->txqs);
+ ieee80211_schedule_txq(hw, txq);
ath10k_htt_tx_txq_update(hw, txq);
- if (artxq == last || (ret < 0 && ret != -ENOENT))
+ if (first == txq || (ret < 0 && ret != -ENOENT))
break;
+
+ if (!first)
+ first = txq;
+
+ txq = ieee80211_next_txq(hw);
}
rcu_read_unlock();
- spin_unlock_bh(&ar->txqs_lock);
}
/************/
@@ -4240,34 +4230,22 @@ static void ath10k_mac_op_tx(struct ieee80211_hw *hw,
}
}
-static void ath10k_mac_op_wake_tx_queue(struct ieee80211_hw *hw,
- struct ieee80211_txq *txq)
+static void ath10k_mac_op_wake_tx_queue(struct ieee80211_hw *hw)
{
- struct ath10k *ar = hw->priv;
- struct ath10k_txq *artxq = (void *)txq->drv_priv;
- struct ieee80211_txq *f_txq;
- struct ath10k_txq *f_artxq;
+ struct ieee80211_txq *txq;
int ret = 0;
int max = 16;
- spin_lock_bh(&ar->txqs_lock);
- if (list_empty(&artxq->list))
- list_add_tail(&artxq->list, &ar->txqs);
-
- f_artxq = list_first_entry(&ar->txqs, struct ath10k_txq, list);
- f_txq = container_of((void *)f_artxq, struct ieee80211_txq, drv_priv);
- list_del_init(&f_artxq->list);
+ txq = ieee80211_next_txq(hw);
- while (ath10k_mac_tx_can_push(hw, f_txq) && max--) {
- ret = ath10k_mac_tx_push_txq(hw, f_txq);
+ while (ath10k_mac_tx_can_push(hw, txq) && max--) {
+ ret = ath10k_mac_tx_push_txq(hw, txq);
if (ret)
break;
}
if (ret != -ENOENT)
- list_add_tail(&f_artxq->list, &ar->txqs);
- spin_unlock_bh(&ar->txqs_lock);
+ ieee80211_schedule_txq(hw, txq);
- ath10k_htt_tx_txq_update(hw, f_txq);
ath10k_htt_tx_txq_update(hw, txq);
}
diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index cf076719c27e..f6c53df78856 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -246,7 +246,6 @@ struct ath_atx_tid {
s8 bar_index;
bool active;
bool clear_ps_filter;
- bool has_queued;
};
void __ath_tx_queue_tid(struct ath_softc *sc, struct ath_atx_tid *tid);
@@ -591,8 +590,7 @@ bool ath_drain_all_txq(struct ath_softc *sc);
void ath_draintxq(struct ath_softc *sc, struct ath_txq *txq);
void ath_tx_node_init(struct ath_softc *sc, struct ath_node *an);
void ath_tx_node_cleanup(struct ath_softc *sc, struct ath_node *an);
-void ath_txq_schedule(struct ath_softc *sc, struct ath_txq *txq);
-void ath_txq_schedule_all(struct ath_softc *sc);
+void ath_txq_schedule(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);
@@ -618,7 +616,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 *queue);
+void ath9k_wake_tx_queue(struct ieee80211_hw *hw);
/********/
/* VIFs */
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 8b4ac7f0a09b..7c2ba21fd972 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -265,7 +265,7 @@ static bool ath_complete_reset(struct ath_softc *sc, bool start)
}
work:
ath_restart_work(sc);
- ath_txq_schedule_all(sc);
+ ath_txq_schedule(sc);
}
sc->gtt_cnt = 0;
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index 396bf05c6bf6..4256701500ff 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -112,62 +112,11 @@ void ath_txq_unlock_complete(struct ath_softc *sc, struct ath_txq *txq)
ath_tx_status(hw, skb);
}
-void __ath_tx_queue_tid(struct ath_softc *sc, struct ath_atx_tid *tid)
-{
- 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;
- u8 acno = TID_TO_WME_AC(tid->tidno);
-
- if (!ctx || !list_empty(&tid->list))
- return;
-
-
- acq = &ctx->acq[acno];
- if ((sc->airtime_flags & AIRTIME_USE_NEW_QUEUES) &&
- tid->an->airtime_deficit[acno] > 0)
- tid_list = &acq->acq_new;
- else
- tid_list = &acq->acq_old;
-
- list_add_tail(&tid->list, tid_list);
-}
-
-void ath_tx_queue_tid(struct ath_softc *sc, struct ath_atx_tid *tid)
-{
- struct ath_vif *avp = (struct ath_vif *) tid->an->vif->drv_priv;
- struct ath_chanctx *ctx = avp->chanctx;
- struct ath_acq *acq;
-
- if (!ctx || !list_empty(&tid->list))
- return;
-
- acq = &ctx->acq[TID_TO_WME_AC(tid->tidno)];
- spin_lock_bh(&acq->lock);
- __ath_tx_queue_tid(sc, tid);
- spin_unlock_bh(&acq->lock);
-}
-
-
-void ath9k_wake_tx_queue(struct ieee80211_hw *hw, struct ieee80211_txq *queue)
+void ath9k_wake_tx_queue(struct ieee80211_hw *hw)
{
struct ath_softc *sc = hw->priv;
- struct ath_common *common = ath9k_hw_common(sc->sc_ah);
- struct ath_atx_tid *tid = (struct ath_atx_tid *) queue->drv_priv;
- struct ath_txq *txq = tid->txq;
-
- ath_dbg(common, QUEUE, "Waking TX queue: %pM (%d)\n",
- queue->sta ? queue->sta->addr : queue->vif->addr,
- tid->tidno);
-
- ath_txq_lock(sc, txq);
-
- tid->has_queued = true;
- ath_tx_queue_tid(sc, tid);
- ath_txq_schedule(sc, txq);
- ath_txq_unlock(sc, txq);
+ ath_txq_schedule(sc);
}
static struct ath_frame_info *get_frame_info(struct sk_buff *skb)
@@ -230,14 +179,9 @@ ath_tid_pull(struct ath_atx_tid *tid)
struct ath_frame_info *fi;
int q;
- if (!tid->has_queued)
- return NULL;
-
skb = ieee80211_tx_dequeue(hw, txq);
- if (!skb) {
- tid->has_queued = false;
+ if (!skb)
return NULL;
- }
if (ath_tx_prepare(hw, skb, &txctl)) {
ieee80211_free_txskb(hw, skb);
@@ -254,12 +198,6 @@ ath_tid_pull(struct ath_atx_tid *tid)
return skb;
}
-
-static bool ath_tid_has_buffered(struct ath_atx_tid *tid)
-{
- return !skb_queue_empty(&tid->retry_q) || tid->has_queued;
-}
-
static struct sk_buff *ath_tid_dequeue(struct ath_atx_tid *tid)
{
struct sk_buff *skb;
@@ -671,7 +609,8 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
skb_queue_splice_tail(&bf_pending, &tid->retry_q);
if (!an->sleeping) {
- ath_tx_queue_tid(sc, tid);
+ struct ieee80211_txq *queue = container_of((void*)tid, struct ieee80211_txq, drv_priv);
+ ieee80211_schedule_txq(sc->hw, queue);
if (ts->ts_status & (ATH9K_TXERR_FILT | ATH9K_TXERR_XRETRY))
tid->clear_ps_filter = true;
@@ -719,8 +658,6 @@ static void ath_tx_count_airtime(struct ath_softc *sc, struct ath_node *an,
spin_lock_bh(&acq->lock);
an->airtime_deficit[q] -= airtime;
- if (an->airtime_deficit[q] <= 0)
- __ath_tx_queue_tid(sc, tid);
spin_unlock_bh(&acq->lock);
}
ath_debug_airtime(sc, an, 0, airtime);
@@ -770,8 +707,6 @@ static void ath_tx_process_buffer(struct ath_softc *sc, struct ath_txq *txq,
} else
ath_tx_complete_aggr(sc, txq, bf, bf_head, sta, tid, ts, txok);
- if (!flush)
- ath_txq_schedule(sc, txq);
}
static bool ath_lookup_legacy(struct ath_buf *bf)
@@ -1506,7 +1441,7 @@ ath_tx_form_burst(struct ath_softc *sc, struct ath_txq *txq,
} while (1);
}
-static bool ath_tx_sched_aggr(struct ath_softc *sc, struct ath_txq *txq,
+static int ath_tx_sched_aggr(struct ath_softc *sc, struct ath_txq *txq,
struct ath_atx_tid *tid)
{
struct ath_buf *bf;
@@ -1515,21 +1450,18 @@ static bool ath_tx_sched_aggr(struct ath_softc *sc, struct ath_txq *txq,
int aggr_len = 0;
bool aggr;
- if (!ath_tid_has_buffered(tid))
- return false;
-
INIT_LIST_HEAD(&bf_q);
bf = ath_tx_get_tid_subframe(sc, txq, tid);
if (!bf)
- return false;
+ return -ENOENT;
tx_info = IEEE80211_SKB_CB(bf->bf_mpdu);
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)) {
__skb_queue_tail(&tid->retry_q, bf->bf_mpdu);
- return false;
+ return -ENOBUFS;
}
ath_set_rates(tid->an->vif, tid->an->sta, bf);
@@ -1539,7 +1471,7 @@ static bool ath_tx_sched_aggr(struct ath_softc *sc, struct ath_txq *txq,
ath_tx_form_burst(sc, txq, tid, &bf_q, bf);
if (list_empty(&bf_q))
- return false;
+ return -ENOENT;
if (tid->clear_ps_filter || tid->an->no_ps_filter) {
tid->clear_ps_filter = false;
@@ -1548,7 +1480,7 @@ static bool ath_tx_sched_aggr(struct ath_softc *sc, struct ath_txq *txq,
ath_tx_fill_desc(sc, bf, txq, aggr_len);
ath_tx_txqaddbuf(sc, txq, &bf_q, false);
- return true;
+ return 0;
}
int ath_tx_aggr_start(struct ath_softc *sc, struct ieee80211_sta *sta,
@@ -1611,53 +1543,49 @@ void ath_tx_aggr_sleep(struct ieee80211_sta *sta, struct ath_softc *sc,
{
struct ath_common *common = ath9k_hw_common(sc->sc_ah);
struct ath_atx_tid *tid;
- struct ath_txq *txq;
+ struct ieee80211_txq *queue;
int tidno;
ath_dbg(common, XMIT, "%s called\n", __func__);
for (tidno = 0; tidno < IEEE80211_NUM_TIDS; tidno++) {
tid = ath_node_to_tid(an, tidno);
- txq = tid->txq;
-
- ath_txq_lock(sc, txq);
-
- if (list_empty(&tid->list)) {
- ath_txq_unlock(sc, txq);
- continue;
- }
+ queue = container_of((void*)tid, struct ieee80211_txq, drv_priv);
if (!skb_queue_empty(&tid->retry_q))
ieee80211_sta_set_buffered(sta, tid->tidno, true);
- list_del_init(&tid->list);
-
- ath_txq_unlock(sc, txq);
}
}
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 *queue;
struct ath_atx_tid *tid;
struct ath_txq *txq;
int tidno;
+ bool sched, wake = false;
ath_dbg(common, XMIT, "%s called\n", __func__);
for (tidno = 0; tidno < IEEE80211_NUM_TIDS; tidno++) {
tid = ath_node_to_tid(an, tidno);
txq = tid->txq;
+ queue = container_of((void*)tid, struct ieee80211_txq, drv_priv);
ath_txq_lock(sc, txq);
tid->clear_ps_filter = true;
- if (ath_tid_has_buffered(tid)) {
- ath_tx_queue_tid(sc, tid);
- ath_txq_schedule(sc, txq);
- }
- ath_txq_unlock_complete(sc, txq);
- }
-}
+ sched = !skb_queue_empty(&tid->retry_q);
+ ath_txq_unlock(sc, txq);
+
+ if (sched)
+ wake = wake || ieee80211_schedule_txq(sc->hw, queue);
+ }
+ if (wake)
+ ath_txq_schedule(sc);
+ }
+
void ath9k_release_buffered_frames(struct ieee80211_hw *hw,
struct ieee80211_sta *sta,
@@ -1948,86 +1876,44 @@ void ath_tx_cleanupq(struct ath_softc *sc, struct ath_txq *txq)
/* For each acq entry, for each tid, try to schedule packets
* for transmit until ampdu_depth has reached min Q depth.
*/
-void ath_txq_schedule(struct ath_softc *sc, struct ath_txq *txq)
+void ath_txq_schedule(struct ath_softc *sc)
{
+ struct ieee80211_hw *hw = sc->hw;
struct ath_common *common = ath9k_hw_common(sc->sc_ah);
+ struct ieee80211_txq *queue;
struct ath_atx_tid *tid;
- struct list_head *tid_list;
- struct ath_acq *acq;
- bool active = AIRTIME_ACTIVE(sc->airtime_flags);
+ struct ath_txq *txq;
+ int ret = 0;
- if (txq->mac80211_qnum < 0)
+ if (test_bit(ATH_OP_HW_RESET, &common->op_flags))
return;
- if (test_bit(ATH_OP_HW_RESET, &common->op_flags))
+ queue = ieee80211_next_txq(hw);
+ if (!queue)
return;
- spin_lock_bh(&sc->chan_lock);
- rcu_read_lock();
- acq = &sc->cur_chan->acq[txq->mac80211_qnum];
+ tid = (struct ath_atx_tid *)queue->drv_priv;
+ txq = tid->txq;
- if (sc->cur_chan->stopped)
+ ath_txq_lock(sc, txq);
+ if (txq->mac80211_qnum < 0)
goto out;
-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);
-
- if (active && tid->an->airtime_deficit[txq->mac80211_qnum] <= 0) {
- spin_lock_bh(&acq->lock);
- tid->an->airtime_deficit[txq->mac80211_qnum] += ATH_AIRTIME_QUANTUM;
- list_move_tail(&tid->list, &acq->acq_old);
- spin_unlock_bh(&acq->lock);
- goto begin;
- }
-
- if (!ath_tid_has_buffered(tid)) {
- spin_lock_bh(&acq->lock);
- 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);
- }
- spin_unlock_bh(&acq->lock);
- goto begin;
- }
+ spin_lock_bh(&sc->chan_lock);
+ rcu_read_lock();
+ if (!sc->cur_chan->stopped)
+ ret = ath_tx_sched_aggr(sc, txq, tid);
- /*
- * If we succeed in scheduling something, immediately restart to make
- * sure we keep the HW busy.
- */
- if(ath_tx_sched_aggr(sc, txq, tid)) {
- if (!active) {
- spin_lock_bh(&acq->lock);
- list_move_tail(&tid->list, &acq->acq_old);
- spin_unlock_bh(&acq->lock);
- }
- goto begin;
- }
-
-out:
rcu_read_unlock();
spin_unlock_bh(&sc->chan_lock);
-}
-void ath_txq_schedule_all(struct ath_softc *sc)
-{
- struct ath_txq *txq;
- int i;
+out:
- for (i = 0; i < IEEE80211_NUM_ACS; i++) {
- txq = sc->tx.txq_map[i];
+ if (ret != -ENOENT)
+ ieee80211_schedule_txq(hw, queue);
- spin_lock_bh(&txq->axq_lock);
- ath_txq_schedule(sc, txq);
- spin_unlock_bh(&txq->axq_lock);
- }
+ ath_txq_unlock(sc, txq);
}
/***********/
@@ -2645,7 +2531,6 @@ static void ath_tx_processq(struct ath_softc *sc, struct ath_txq *txq)
if (list_empty(&txq->axq_q)) {
txq->axq_link = NULL;
- ath_txq_schedule(sc, txq);
break;
}
bf = list_first_entry(&txq->axq_q, struct ath_buf, list);
@@ -2697,6 +2582,7 @@ static void ath_tx_processq(struct ath_softc *sc, struct ath_txq *txq)
ath_tx_process_buffer(sc, txq, &ts, bf, &bf_head);
}
ath_txq_unlock_complete(sc, txq);
+ ath_txq_schedule(sc);
}
void ath_tx_tasklet(struct ath_softc *sc)
@@ -2711,6 +2597,7 @@ void ath_tx_tasklet(struct ath_softc *sc)
ath_tx_processq(sc, &sc->tx.txq[i]);
}
rcu_read_unlock();
+ ath_txq_schedule(sc);
}
void ath_tx_edma_tasklet(struct ath_softc *sc)
@@ -2796,6 +2683,7 @@ void ath_tx_edma_tasklet(struct ath_softc *sc)
ath_txq_unlock_complete(sc, txq);
}
rcu_read_unlock();
+ ath_txq_schedule(sc);
}
/*****************/
@@ -2875,7 +2763,6 @@ 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->has_queued = false;
__skb_queue_head_init(&tid->retry_q);
INIT_LIST_HEAD(&tid->list);
acno = TID_TO_WME_AC(tidno);
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index cc9073e45be9..f0373dd3d714 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -105,9 +105,12 @@
* The driver is expected to initialize its private per-queue data for stations
* and interfaces in the .add_interface and .sta_add ops.
*
- * The driver can't access the queue directly. To dequeue a frame, it calls
- * ieee80211_tx_dequeue(). Whenever mac80211 adds a new frame to a queue, it
- * calls the .wake_tx_queue driver op.
+ * The driver can't access the queue directly. To obtain the next queue to pull
+ * frames from, the driver calls ieee80211_next_txq(). To dequeue a frame from a
+ * txq, it calls ieee80211_tx_dequeue(). Whenever mac80211 adds a new frame to a
+ * queue, it calls the .wake_tx_queue driver op. The driver is expected to
+ * re-schedule the txq using ieee80211_schedule_txq() if it is still active
+ * after the driver has finished pulling packets from it.
*
* For AP powersave TIM handling, the driver only needs to indicate if it has
* buffered packets in the driver specific data structures by calling
@@ -3723,8 +3726,7 @@ struct ieee80211_ops {
struct ieee80211_vif *vif,
struct ieee80211_tdls_ch_sw_params *params);
- void (*wake_tx_queue)(struct ieee80211_hw *hw,
- struct ieee80211_txq *txq);
+ void (*wake_tx_queue)(struct ieee80211_hw *hw);
void (*sync_rx_queues)(struct ieee80211_hw *hw);
int (*start_nan)(struct ieee80211_hw *hw,
@@ -5876,6 +5878,29 @@ void ieee80211_unreserve_tid(struct ieee80211_sta *sta, u8 tid);
struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
struct ieee80211_txq *txq);
+/**
+ * ieee80211_schedule_txq - add txq to scheduling loop
+ *
+ * @hw: pointer as obtained from ieee80211_alloc_hw()
+ * @txq: pointer obtained from station or virtual interface
+ *
+ * Returns true if the txq was actually added to the scheduling,
+ * false otherwise.
+ */
+bool ieee80211_schedule_txq(struct ieee80211_hw *hw,
+ struct ieee80211_txq *txq);
+
+/**
+ * ieee80211_next_txq - get next tx queue to pull packets from
+ *
+ * @hw: pointer as obtained from ieee80211_alloc_hw()
+ *
+ * Returns the next txq if successful, %NULL if no queue is eligible. If a txq
+ * is returned, it will have been removed from the scheduler queue and needs to
+ * be re-scheduled with ieee80211_schedule_txq() to continue to be active.
+ */
+struct ieee80211_txq *ieee80211_next_txq(struct ieee80211_hw *hw);
+
/**
* ieee80211_txq_get_depth - get pending frame/byte count of given txq
*
diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c
index bef516ec47f9..569b5b5c6d70 100644
--- a/net/mac80211/agg-tx.c
+++ b/net/mac80211/agg-tx.c
@@ -226,9 +226,13 @@ ieee80211_agg_start_txq(struct sta_info *sta, int tid, bool enable)
clear_bit(IEEE80211_TXQ_AMPDU, &txqi->flags);
clear_bit(IEEE80211_TXQ_STOP, &txqi->flags);
+
+ if (!ieee80211_schedule_txq(&sta->sdata->local->hw, txq))
+ return;
+
local_bh_disable();
rcu_read_lock();
- drv_wake_tx_queue(sta->sdata->local, txqi);
+ drv_wake_tx_queue(sta->sdata->local);
rcu_read_unlock();
local_bh_enable();
}
diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
index 09f77e4a8a79..e20a9e2acd53 100644
--- a/net/mac80211/driver-ops.h
+++ b/net/mac80211/driver-ops.h
@@ -1157,16 +1157,10 @@ drv_tdls_recv_channel_switch(struct ieee80211_local *local,
trace_drv_return_void(local);
}
-static inline void drv_wake_tx_queue(struct ieee80211_local *local,
- struct txq_info *txq)
+static inline void drv_wake_tx_queue(struct ieee80211_local *local)
{
- struct ieee80211_sub_if_data *sdata = vif_to_sdata(txq->txq.vif);
-
- if (!check_sdata_in_driver(sdata))
- return;
-
- trace_drv_wake_tx_queue(local, sdata, txq);
- local->ops->wake_tx_queue(&local->hw, &txq->txq);
+ trace_drv_wake_tx_queue(local);
+ local->ops->wake_tx_queue(&local->hw);
}
static inline int drv_start_nan(struct ieee80211_local *local,
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 9675814f64db..48bf933435a8 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -832,6 +832,7 @@ struct txq_info {
struct codel_vars def_cvars;
struct codel_stats cstats;
struct sk_buff_head frags;
+ struct list_head schedule_order;
unsigned long flags;
/* keep last! */
@@ -1121,6 +1122,9 @@ struct ieee80211_local {
struct codel_vars *cvars;
struct codel_params cparams;
+ struct list_head active_txqs;
+ spinlock_t active_txq_lock;
+
const struct ieee80211_ops *ops;
/*
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 8aa1f5b6a051..9ad0556aa24b 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -616,6 +616,9 @@ struct ieee80211_hw *ieee80211_alloc_hw_nm(size_t priv_data_len,
spin_lock_init(&local->rx_path_lock);
spin_lock_init(&local->queue_stop_reason_lock);
+ INIT_LIST_HEAD(&local->active_txqs);
+ spin_lock_init(&local->active_txq_lock);
+
INIT_LIST_HEAD(&local->chanctx_list);
mutex_init(&local->chanctx_mtx);
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 9673e157bf8f..5db76db6bfc8 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -1246,12 +1246,15 @@ void ieee80211_sta_ps_deliver_wakeup(struct sta_info *sta)
drv_sta_notify(local, sdata, STA_NOTIFY_AWAKE, &sta->sta);
if (sta->sta.txq[0]) {
+ bool wake = false;
for (i = 0; i < ARRAY_SIZE(sta->sta.txq); i++) {
if (!txq_has_queue(sta->sta.txq[i]))
continue;
- drv_wake_tx_queue(local, to_txq_info(sta->sta.txq[i]));
+ wake = wake || ieee80211_schedule_txq(&local->hw, sta->sta.txq[i]);
}
+ if (wake)
+ drv_wake_tx_queue(local);
}
skb_queue_head_init(&pending);
diff --git a/net/mac80211/trace.h b/net/mac80211/trace.h
index 3d9ac17af407..531c0e7f2358 100644
--- a/net/mac80211/trace.h
+++ b/net/mac80211/trace.h
@@ -2550,33 +2550,20 @@ TRACE_EVENT(drv_tdls_recv_channel_switch,
);
TRACE_EVENT(drv_wake_tx_queue,
- TP_PROTO(struct ieee80211_local *local,
- struct ieee80211_sub_if_data *sdata,
- struct txq_info *txq),
+ TP_PROTO(struct ieee80211_local *local),
- TP_ARGS(local, sdata, txq),
+ TP_ARGS(local),
TP_STRUCT__entry(
LOCAL_ENTRY
- VIF_ENTRY
- STA_ENTRY
- __field(u8, ac)
- __field(u8, tid)
),
TP_fast_assign(
- struct ieee80211_sta *sta = txq->txq.sta;
-
LOCAL_ASSIGN;
- VIF_ASSIGN;
- STA_ASSIGN;
- __entry->ac = txq->txq.ac;
- __entry->tid = txq->txq.tid;
),
-
TP_printk(
- LOCAL_PR_FMT VIF_PR_FMT STA_PR_FMT " ac:%d tid:%d",
- LOCAL_PR_ARG, VIF_PR_ARG, STA_PR_ARG, __entry->ac, __entry->tid
+ LOCAL_PR_FMT,
+ LOCAL_PR_ARG
)
);
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 94826680cf2b..0ee6ea71d1c4 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1405,6 +1405,7 @@ void ieee80211_txq_init(struct ieee80211_sub_if_data *sdata,
codel_vars_init(&txqi->def_cvars);
codel_stats_init(&txqi->cstats);
__skb_queue_head_init(&txqi->frags);
+ INIT_LIST_HEAD(&txqi->schedule_order);
txqi->txq.vif = &sdata->vif;
@@ -1428,6 +1429,7 @@ void ieee80211_txq_purge(struct ieee80211_local *local,
fq_tin_reset(fq, tin, fq_skb_free_func);
ieee80211_purge_tx_queue(&local->hw, &txqi->frags);
+ list_del_init(&txqi->schedule_order);
}
int ieee80211_txq_setup_flows(struct ieee80211_local *local)
@@ -1524,7 +1526,8 @@ static bool ieee80211_queue_skb(struct ieee80211_local *local,
ieee80211_txq_enqueue(local, txqi, skb);
spin_unlock_bh(&fq->lock);
- drv_wake_tx_queue(local, txqi);
+ if (ieee80211_schedule_txq(&local->hw, &txqi->txq))
+ drv_wake_tx_queue(local);
return true;
}
@@ -3517,6 +3520,49 @@ struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
}
EXPORT_SYMBOL(ieee80211_tx_dequeue);
+bool ieee80211_schedule_txq(struct ieee80211_hw *hw,
+ struct ieee80211_txq *txq)
+{
+ struct ieee80211_local *local = hw_to_local(hw);
+ struct txq_info *txqi = to_txq_info(txq);
+ int ret = 0;
+
+ spin_lock_bh(&local->active_txq_lock);
+
+ if (list_empty(&txqi->schedule_order)) {
+ list_add_tail(&txqi->schedule_order, &local->active_txqs);
+ ret = 1;
+ }
+
+ spin_unlock_bh(&local->active_txq_lock);
+
+ return ret;
+}
+EXPORT_SYMBOL(ieee80211_schedule_txq);
+
+struct ieee80211_txq *ieee80211_next_txq(struct ieee80211_hw *hw)
+{
+ struct ieee80211_local *local = hw_to_local(hw);
+ struct txq_info *txqi = NULL;
+
+ spin_lock_bh(&local->active_txq_lock);
+
+ if (list_empty(&local->active_txqs))
+ goto out;
+
+ txqi = list_first_entry(&local->active_txqs, struct txq_info, schedule_order);
+ list_del_init(&txqi->schedule_order);
+
+out:
+ spin_unlock_bh(&local->active_txq_lock);
+
+ if (!txqi)
+ return NULL;
+
+ return &txqi->txq;
+}
+EXPORT_SYMBOL(ieee80211_next_txq);
+
void __ieee80211_subif_start_xmit(struct sk_buff *skb,
struct net_device *dev,
u32 info_flags)
--
2.14.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/2] mac80211: Add airtime accounting and scheduling to TXQs
2017-10-16 16:09 [PATCH 1/2] mac80211: Add TXQ scheduling API Toke Høiland-Jørgensen
@ 2017-10-16 16:09 ` Toke Høiland-Jørgensen
2017-10-17 7:07 ` Johannes Berg
2017-10-17 6:39 ` [PATCH 1/2] mac80211: Add TXQ scheduling API Johannes Berg
` (3 subsequent siblings)
4 siblings, 1 reply; 20+ messages in thread
From: Toke Høiland-Jørgensen @ 2017-10-16 16:09 UTC (permalink / raw)
To: make-wifi-fast, linux-wireless; +Cc: Toke Høiland-Jørgensen
This adds airtime accounting and scheduling to the mac80211 TXQ scheduler. A new
hardware flag, AIRTIME_ACCOUNTING, is added that drivers can set if they support
reporting airtime usage of transmissions. When this flag is set, mac80211 will
expect the actual airtime usage to be reported in the tx_time and rx_time fields
of the respective status structs.
When airtime information is present, mac80211 will schedule TXQs (through
ieee80211_next_txq()) in a way that enforces airtime fairness between active
stations. This scheduling works the same way as the ath9k in-driver airtime
fairness scheduling, which is then removed.
Only ath9k currently sets the AIRTIME_ACCOUNTING flag.
Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
---
Changes since the RFC:
- Don't try to calculate airtime usage in mac80211; instead, add a field to
struct ieee80211_rx_status for the driver to fill in.
- Fold ath9k patch into this patch, and remove all the airtime bits from
ath9k apart from the actual airtime accounting.
- Feature parity with the ath9k scheduler (in particular, the old/new
station optimisation is retained).
- Add explicit hardware flag for the driver to signal that it supports
airtime accounting and wants the scheduling.
- This has actually been tested... :)
drivers/net/wireless/ath/ath9k/ath9k.h | 7 +---
drivers/net/wireless/ath/ath9k/debug.h | 8 -----
drivers/net/wireless/ath/ath9k/debug_sta.c | 54 ------------------------------
drivers/net/wireless/ath/ath9k/init.c | 4 +--
drivers/net/wireless/ath/ath9k/recv.c | 11 ++----
drivers/net/wireless/ath/ath9k/xmit.c | 21 ++++--------
include/net/mac80211.h | 6 ++++
net/mac80211/debugfs.c | 1 +
net/mac80211/debugfs_sta.c | 29 ++++++++++++++++
net/mac80211/ieee80211_i.h | 6 +++-
net/mac80211/main.c | 3 +-
net/mac80211/rx.c | 9 +++++
net/mac80211/sta_info.c | 2 ++
net/mac80211/sta_info.h | 7 ++++
net/mac80211/status.c | 15 +++++++++
net/mac80211/tx.c | 28 +++++++++++++---
16 files changed, 112 insertions(+), 99 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index f6c53df78856..f6e8f9b1b984 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -112,8 +112,6 @@ int ath_descdma_setup(struct ath_softc *sc, struct ath_descdma *dd,
#define ATH_TXFIFO_DEPTH 8
#define ATH_TX_ERROR 0x01
-#define ATH_AIRTIME_QUANTUM 300 /* usec */
-
/* Stop tx traffic 1ms before the GO goes away */
#define ATH_P2P_PS_STOP_TIME 1000
@@ -215,6 +213,7 @@ struct ath_buf_state {
bool stale;
u16 seqno;
unsigned long bfs_paprd_timestamp;
+ u32 airtime;
};
struct ath_buf {
@@ -262,12 +261,9 @@ struct ath_node {
bool sleeping;
bool no_ps_filter;
- s64 airtime_deficit[IEEE80211_NUM_ACS];
- 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];
@@ -986,7 +982,6 @@ void ath_ant_comb_scan(struct ath_softc *sc, struct ath_rx_status *rs);
#define AIRTIME_USE_TX BIT(0)
#define AIRTIME_USE_RX BIT(1)
-#define AIRTIME_USE_NEW_QUEUES BIT(2)
#define AIRTIME_ACTIVE(flags) (!!(flags & (AIRTIME_USE_TX|AIRTIME_USE_RX)))
struct ath_softc {
diff --git a/drivers/net/wireless/ath/ath9k/debug.h b/drivers/net/wireless/ath/ath9k/debug.h
index 249f8141cd00..559d9628f280 100644
--- a/drivers/net/wireless/ath/ath9k/debug.h
+++ b/drivers/net/wireless/ath/ath9k/debug.h
@@ -319,20 +319,12 @@ 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_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_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 efc692ee67d4..89ba95287a8b 100644
--- a/drivers/net/wireless/ath/ath9k/debug_sta.c
+++ b/drivers/net/wireless/ath/ath9k/debug_sta.c
@@ -242,59 +242,6 @@ 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;
- static const char *qname[4] = {
- "VO", "VI", "BE", "BK"
- };
- u32 len = 0, size = 256;
- char *buf;
- size_t retval;
- int i;
-
- 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: ");
- for (i = 0; i < 4; i++)
- len += scnprintf(buf+len, size - len, "%s: %lld us ", qname[i], an->airtime_deficit[i]);
- if (len < size)
- buf[len++] = '\n';
-
- 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,
@@ -304,5 +251,4 @@ 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 bb7936090b91..b4c6fc49b19e 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -620,8 +620,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 |
- AIRTIME_USE_NEW_QUEUES);
+ sc->airtime_flags = (AIRTIME_USE_TX | AIRTIME_USE_RX);
sc->sc_ah = ah;
sc->dfs_detector = dfs_pattern_detector_init(common, NL80211_DFS_UNSET);
@@ -873,6 +872,7 @@ static void ath9k_set_hw_capab(struct ath_softc *sc, struct ieee80211_hw *hw)
ieee80211_hw_set(hw, HOST_BROADCAST_PS_BUFFERING);
ieee80211_hw_set(hw, SUPPORT_FAST_XMIT);
ieee80211_hw_set(hw, SUPPORTS_CLONED_SKBS);
+ ieee80211_hw_set(hw, AIRTIME_ACCOUNTING);
if (ath9k_ps_enable)
ieee80211_hw_set(hw, SUPPORTS_PS);
diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
index 2197aee2bb72..e77e5c839abc 100644
--- a/drivers/net/wireless/ath/ath9k/recv.c
+++ b/drivers/net/wireless/ath/ath9k/recv.c
@@ -1054,14 +1054,9 @@ static void ath_rx_count_airtime(struct ath_softc *sc,
len, rxs->rate_idx, is_sp);
}
- if (!!(sc->airtime_flags & AIRTIME_USE_RX)) {
- spin_lock_bh(&acq->lock);
- an->airtime_deficit[acno] -= airtime;
- if (an->airtime_deficit[acno] <= 0)
- __ath_tx_queue_tid(sc, ATH_AN_2_TID(an, tidno));
- spin_unlock_bh(&acq->lock);
- }
- ath_debug_airtime(sc, an, airtime, 0);
+ if (sc->airtime_flags & AIRTIME_USE_RX)
+ rxs->rx_time = min_t(u32, 0xffff, airtime);
+
exit:
rcu_read_unlock();
}
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index 4256701500ff..86b1d21b237e 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -638,11 +638,9 @@ 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_node *an,
- struct ath_atx_tid *tid, struct ath_buf *bf,
+static void ath_tx_count_airtime(struct ath_softc *sc, struct ath_buf *bf,
struct ath_tx_status *ts)
{
- struct ath_txq *txq = tid->txq;
u32 airtime = 0;
int i;
@@ -652,15 +650,9 @@ static void ath_tx_count_airtime(struct ath_softc *sc, struct ath_node *an,
airtime += rate_dur * bf->rates[i].count;
}
- if (sc->airtime_flags & AIRTIME_USE_TX) {
- int q = txq->mac80211_qnum;
- struct ath_acq *acq = &sc->cur_chan->acq[q];
+ if (sc->airtime_flags & AIRTIME_USE_TX)
+ bf->bf_state.airtime = airtime;
- spin_lock_bh(&acq->lock);
- an->airtime_deficit[q] -= airtime;
- spin_unlock_bh(&acq->lock);
- }
- ath_debug_airtime(sc, an, 0, airtime);
}
static void ath_tx_process_buffer(struct ath_softc *sc, struct ath_txq *txq,
@@ -690,7 +682,7 @@ static void ath_tx_process_buffer(struct ath_softc *sc, struct ath_txq *txq,
if (sta) {
struct ath_node *an = (struct ath_node *)sta->drv_priv;
tid = ath_get_skb_tid(sc, an, bf->bf_mpdu);
- ath_tx_count_airtime(sc, an, tid, bf, ts);
+ ath_tx_count_airtime(sc, bf, ts);
if (ts->ts_status & (ATH9K_TXERR_FILT | ATH9K_TXERR_XRETRY))
tid->clear_ps_filter = true;
}
@@ -2425,6 +2417,8 @@ static void ath_tx_complete_buf(struct ath_softc *sc, struct ath_buf *bf,
if (ts->ts_status & ATH9K_TXERR_FILT)
tx_info->flags |= IEEE80211_TX_STAT_TX_FILTERED;
+ tx_info->status.tx_time = min_t(u32, 0xffff, bf->bf_state.airtime);
+
dma_unmap_single(sc->dev, bf->bf_buf_addr, skb->len, DMA_TO_DEVICE);
bf->bf_buf_addr = 0;
if (sc->tx99_state)
@@ -2751,9 +2745,6 @@ void ath_tx_node_init(struct ath_softc *sc, struct ath_node *an)
struct ath_atx_tid *tid;
int tidno, acno;
- for (acno = 0; acno < IEEE80211_NUM_ACS; acno++)
- an->airtime_deficit[acno] = ATH_AIRTIME_QUANTUM;
-
for (tidno = 0; tidno < IEEE80211_NUM_TIDS; tidno++) {
tid = ath_node_to_tid(an, tidno);
tid->an = an;
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index f0373dd3d714..2bedd361511b 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1202,6 +1202,7 @@ struct ieee80211_rx_status {
u32 device_timestamp;
u32 ampdu_reference;
u32 flag;
+ u16 rx_time;
u16 freq;
u8 enc_flags;
u8 encoding:2, bw:3;
@@ -2059,6 +2060,10 @@ struct ieee80211_txq {
* The stack will not do fragmentation.
* The callback for @set_frag_threshold should be set as well.
*
+ * @IEEE80211_HW_AIRTIME_ACCOUNTING: Hardware supports accounting the airtime
+ * usage of other stations and reports it in the @tx_time and/or @rx_time
+ * fields of the TX/RX status structs.
+ *
* @NUM_IEEE80211_HW_FLAGS: number of hardware flags, used for sizing arrays
*/
enum ieee80211_hw_flags {
@@ -2101,6 +2106,7 @@ enum ieee80211_hw_flags {
IEEE80211_HW_TX_FRAG_LIST,
IEEE80211_HW_REPORTS_LOW_ACK,
IEEE80211_HW_SUPPORTS_TX_FRAG,
+ IEEE80211_HW_AIRTIME_ACCOUNTING,
/* keep last, obviously */
NUM_IEEE80211_HW_FLAGS
diff --git a/net/mac80211/debugfs.c b/net/mac80211/debugfs.c
index 5fae001f286c..20e3aa0e8464 100644
--- a/net/mac80211/debugfs.c
+++ b/net/mac80211/debugfs.c
@@ -211,6 +211,7 @@ static const char *hw_flag_names[] = {
FLAG(TX_FRAG_LIST),
FLAG(REPORTS_LOW_ACK),
FLAG(SUPPORTS_TX_FRAG),
+ FLAG(AIRTIME_ACCOUNTING),
#undef FLAG
};
diff --git a/net/mac80211/debugfs_sta.c b/net/mac80211/debugfs_sta.c
index b15412c21ac9..57bc6b544f0e 100644
--- a/net/mac80211/debugfs_sta.c
+++ b/net/mac80211/debugfs_sta.c
@@ -188,6 +188,32 @@ static ssize_t sta_aqm_read(struct file *file, char __user *userbuf,
}
STA_OPS(aqm);
+static ssize_t sta_airtime_read(struct file *file, char __user *userbuf,
+ size_t count, loff_t *ppos)
+{
+ struct sta_info *sta = file->private_data;
+ size_t bufsz = 200;
+ char *buf = kzalloc(bufsz, GFP_KERNEL), *p = buf;
+ ssize_t rv;
+
+ if (!buf)
+ return -ENOMEM;
+
+ spin_lock_bh(&sta->lock);
+
+ p += scnprintf(p, bufsz+buf-p,
+ "RX: %llu us\nTX: %llu us\nDeficit: %lld us\n",
+ sta->airtime_stats.rx_airtime,
+ sta->airtime_stats.tx_airtime,
+ sta->airtime_deficit);
+
+ spin_unlock_bh(&sta->lock);
+ rv = simple_read_from_buffer(userbuf, count, ppos, buf, p - buf);
+ kfree(buf);
+ return rv;
+}
+STA_OPS(airtime);
+
static ssize_t sta_agg_status_read(struct file *file, char __user *userbuf,
size_t count, loff_t *ppos)
{
@@ -542,6 +568,9 @@ void ieee80211_sta_debugfs_add(struct sta_info *sta)
if (local->ops->wake_tx_queue)
DEBUGFS_ADD(aqm);
+ if (ieee80211_hw_check(&local->hw, AIRTIME_ACCOUNTING))
+ DEBUGFS_ADD(airtime);
+
if (sizeof(sta->driver_buffered_tids) == sizeof(u32))
debugfs_create_x32("driver_buffered_tids", 0400,
sta->debugfs_dir,
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 48bf933435a8..7f6fe5285813 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -90,6 +90,9 @@ extern const u8 ieee80211_ac_to_qos_mask[IEEE80211_NUM_ACS];
#define IEEE80211_MAX_NAN_INSTANCE_ID 255
+/* How much to increase airtime deficit on each scheduling round */
+#define IEEE80211_AIRTIME_QUANTUM 1000 /* usec */
+
struct ieee80211_fragment_entry {
struct sk_buff_head skb_list;
unsigned long first_frag_time;
@@ -1122,7 +1125,8 @@ struct ieee80211_local {
struct codel_vars *cvars;
struct codel_params cparams;
- struct list_head active_txqs;
+ struct list_head active_txqs_new;
+ struct list_head active_txqs_old;
spinlock_t active_txq_lock;
const struct ieee80211_ops *ops;
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 9ad0556aa24b..acf73c398d9b 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -616,7 +616,8 @@ struct ieee80211_hw *ieee80211_alloc_hw_nm(size_t priv_data_len,
spin_lock_init(&local->rx_path_lock);
spin_lock_init(&local->queue_stop_reason_lock);
- INIT_LIST_HEAD(&local->active_txqs);
+ INIT_LIST_HEAD(&local->active_txqs_new);
+ INIT_LIST_HEAD(&local->active_txqs_old);
spin_lock_init(&local->active_txq_lock);
INIT_LIST_HEAD(&local->chanctx_list);
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 70e9d2ca8bbe..14cc1a5902a7 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -1637,6 +1637,15 @@ ieee80211_rx_h_sta_process(struct ieee80211_rx_data *rx)
if (ieee80211_vif_is_mesh(&rx->sdata->vif))
ieee80211_mps_rx_h_sta_process(sta, hdr);
+ /* airtime accounting */
+ if (ieee80211_hw_check(&sta->local->hw, AIRTIME_ACCOUNTING) &&
+ status->rx_time) {
+ spin_lock_bh(&sta->lock);
+ sta->airtime_stats.rx_airtime += status->rx_time;
+ sta->airtime_deficit -= status->rx_time;
+ spin_unlock_bh(&sta->lock);
+ }
+
/*
* Drop (qos-)data::nullfunc frames silently, since they
* are used only to control station power saving mode.
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 5db76db6bfc8..51fe010af20e 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -433,6 +433,8 @@ struct sta_info *sta_info_alloc(struct ieee80211_sub_if_data *sdata,
sta->cparams.interval = MS2TIME(100);
sta->cparams.ecn = true;
+ sta->airtime_deficit = IEEE80211_AIRTIME_QUANTUM;
+
sta_dbg(sdata, "Allocated STA %pM\n", sta->sta.addr);
return sta;
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 5c54acd10562..5d1802f22550 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -550,6 +550,13 @@ struct sta_info {
} tx_stats;
u16 tid_seq[IEEE80211_QOS_CTL_TID_MASK + 1];
+ /* Airtime stats and deficit, protected by lock */
+ struct {
+ u64 rx_airtime;
+ u64 tx_airtime;
+ } airtime_stats;
+ s64 airtime_deficit;
+
/*
* Aggregation information, locked with lock.
*/
diff --git a/net/mac80211/status.c b/net/mac80211/status.c
index da7427a41529..2f1111532be0 100644
--- a/net/mac80211/status.c
+++ b/net/mac80211/status.c
@@ -823,6 +823,13 @@ static void __ieee80211_tx_status(struct ieee80211_hw *hw,
ieee80211_lost_packet(sta, info);
}
}
+
+ if (info->status.tx_time) {
+ spin_lock_bh(&sta->lock);
+ sta->airtime_stats.tx_airtime += info->status.tx_time;
+ sta->airtime_deficit -= info->status.tx_time;
+ spin_unlock_bh(&sta->lock);
+ }
}
/* SNMP counters
@@ -947,6 +954,14 @@ void ieee80211_tx_status_ext(struct ieee80211_hw *hw,
sta->status_stats.retry_failed++;
sta->status_stats.retry_count += retry_count;
+ if (ieee80211_hw_check(&local->hw, AIRTIME_ACCOUNTING) &&
+ info->status.tx_time) {
+ spin_lock_bh(&sta->lock);
+ sta->airtime_stats.tx_airtime += info->status.tx_time;
+ sta->airtime_deficit -= info->status.tx_time;
+ spin_unlock_bh(&sta->lock);
+ }
+
if (acked) {
sta->status_stats.last_ack = jiffies;
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 0ee6ea71d1c4..937d48a1ba7a 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -3530,7 +3530,7 @@ bool ieee80211_schedule_txq(struct ieee80211_hw *hw,
spin_lock_bh(&local->active_txq_lock);
if (list_empty(&txqi->schedule_order)) {
- list_add_tail(&txqi->schedule_order, &local->active_txqs);
+ list_add_tail(&txqi->schedule_order, &local->active_txqs_new);
ret = 1;
}
@@ -3544,13 +3544,33 @@ struct ieee80211_txq *ieee80211_next_txq(struct ieee80211_hw *hw)
{
struct ieee80211_local *local = hw_to_local(hw);
struct txq_info *txqi = NULL;
+ struct list_head *head;
spin_lock_bh(&local->active_txq_lock);
- if (list_empty(&local->active_txqs))
- goto out;
+begin:
+ head = &local->active_txqs_new;
+ if (list_empty(head)) {
+ head = &local->active_txqs_old;
+ if (list_empty(head))
+ goto out;
+ }
+
+ txqi = list_first_entry(head, struct txq_info, schedule_order);
+
+ if (txqi->txq.sta) {
+ struct sta_info *sta = container_of(txqi->txq.sta, struct sta_info, sta);
+
+ spin_lock_bh(&sta->lock);
+ if (sta->airtime_deficit < 0) {
+ sta->airtime_deficit += IEEE80211_AIRTIME_QUANTUM;
+ list_move_tail(&txqi->schedule_order, &local->active_txqs_old);
+ spin_unlock_bh(&sta->lock);
+ goto begin;
+ }
+ spin_unlock_bh(&sta->lock);
+ }
- txqi = list_first_entry(&local->active_txqs, struct txq_info, schedule_order);
list_del_init(&txqi->schedule_order);
out:
--
2.14.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] mac80211: Add TXQ scheduling API
2017-10-16 16:09 [PATCH 1/2] mac80211: Add TXQ scheduling API Toke Høiland-Jørgensen
2017-10-16 16:09 ` [PATCH 2/2] mac80211: Add airtime accounting and scheduling to TXQs Toke Høiland-Jørgensen
@ 2017-10-17 6:39 ` Johannes Berg
2017-10-27 14:14 ` [PATCH v2 1/3] " Toke Høiland-Jørgensen
` (2 subsequent siblings)
4 siblings, 0 replies; 20+ messages in thread
From: Johannes Berg @ 2017-10-17 6:39 UTC (permalink / raw)
To: Toke Høiland-Jørgensen, make-wifi-fast, linux-wireless
> /* protects shared structure data */
> spinlock_t data_lock;
> - /* protects: ar->txqs, artxq->list */
> - spinlock_t txqs_lock;
>
> - struct list_head txqs;
I don't see you removing the artxq->list member, but surely it can't be
used any more now, without a list?
[snip] that's about all I can comment on ath*k :)
> +/**
> + * ieee80211_schedule_txq - add txq to scheduling loop
> + *
> + * @hw: pointer as obtained from ieee80211_alloc_hw()
> + * @txq: pointer obtained from station or virtual interface
or more likely next_txq()? :)
> + * Returns true if the txq was actually added to the scheduling,
> + * false otherwise.
Perhaps use %true/%false.
> if (sta->sta.txq[0]) {
> + bool wake = false;
please add a blank line after this new line
> for (i = 0; i < ARRAY_SIZE(sta->sta.txq); i++) {
> if (!txq_has_queue(sta->sta.txq[i]))
> continue;
>
> - drv_wake_tx_queue(local, to_txq_info(sta->sta.txq[i]));
> + wake = wake || ieee80211_schedule_txq(&local->hw, sta->sta.txq[i]);
> }
> + if (wake)
> + drv_wake_tx_queue(local);
> }
Are you sure you want to skip the call to ieee80211_schedule_txq() if
wake is true? I think you don't, but boolean short-circuit evaluation
would lead to that, afaict?
So it better be written as
if (ieee80211_schedule_txq(...))
wake = true;
No need to even check wake anyway, since it can only ever go from false
to true.
(Also, that line is getting a bit long)
> skb_queue_head_init(&pending);
> diff --git a/net/mac80211/trace.h b/net/mac80211/trace.h
> index 3d9ac17af407..531c0e7f2358 100644
> --- a/net/mac80211/trace.h
> +++ b/net/mac80211/trace.h
> @@ -2550,33 +2550,20 @@ TRACE_EVENT(drv_tdls_recv_channel_switch,
> );
>
> TRACE_EVENT(drv_wake_tx_queue,
> - TP_PROTO(struct ieee80211_local *local,
> - struct ieee80211_sub_if_data *sdata,
> - struct txq_info *txq),
> + TP_PROTO(struct ieee80211_local *local),
You should just replace all of this with
DEFINE_EVENT(local_only_evt, drv_wake_tx_queue,
TP_PROTO(struct ieee80211_local *local),
TP_ARGS(local)
);
now.
> +bool ieee80211_schedule_txq(struct ieee80211_hw *hw,
> + struct ieee80211_txq *txq)
> +{
> + struct ieee80211_local *local = hw_to_local(hw);
> + struct txq_info *txqi = to_txq_info(txq);
> + int ret = 0;
bool/false/true please.
> +struct ieee80211_txq *ieee80211_next_txq(struct ieee80211_hw *hw)
> +{
> + struct ieee80211_local *local = hw_to_local(hw);
> + struct txq_info *txqi = NULL;
> +
> + spin_lock_bh(&local->active_txq_lock);
> +
> + if (list_empty(&local->active_txqs))
> + goto out;
> +
> + txqi = list_first_entry(&local->active_txqs, struct txq_info, schedule_order);
that line seems pretty long, but I haven't counted :)
johannes
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] mac80211: Add airtime accounting and scheduling to TXQs
2017-10-16 16:09 ` [PATCH 2/2] mac80211: Add airtime accounting and scheduling to TXQs Toke Høiland-Jørgensen
@ 2017-10-17 7:07 ` Johannes Berg
2017-10-17 7:34 ` Toke Høiland-Jørgensen
0 siblings, 1 reply; 20+ messages in thread
From: Johannes Berg @ 2017-10-17 7:07 UTC (permalink / raw)
To: Toke Høiland-Jørgensen, make-wifi-fast, linux-wireless
> Only ath9k currently sets the AIRTIME_ACCOUNTING flag.
I think you should actually make that a separate patch.
In the previous patch that's not possible or it breaks things, but this
patch just adds a new feature that drivers _may_ use, they don't have
to since it doesn't change the API, so you should split the patches.
[snip ath9k, I don't really know anything about it]
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -1202,6 +1202,7 @@ struct ieee80211_rx_status {
> u32 device_timestamp;
> u32 ampdu_reference;
> u32 flag;
> + u16 rx_time;
I'd prefer this were called "airtime" or such, and you clearly need to
add documentation regarding this field. In particular, you should point
to the IEEE80211_HW_AIRTIME_ACCOUNTING hw flag, and document what
should be taken into account here (preamble length, IFS, rts/cts/ack,
etc.?) and additionally how this should be handled wrt. A-MPDU (and A-
MSDU where decapsulated by the device.)
For aggregation, either form, I expect you just want to see 0 for all
but the very first frame? but that may be very difficult for some
drivers to implement.
> +++ b/net/mac80211/rx.c
> @@ -1637,6 +1637,15 @@ ieee80211_rx_h_sta_process(struct
> ieee80211_rx_data *rx)
> if (ieee80211_vif_is_mesh(&rx->sdata->vif))
> ieee80211_mps_rx_h_sta_process(sta, hdr);
>
> + /* airtime accounting */
> + if (ieee80211_hw_check(&sta->local->hw, AIRTIME_ACCOUNTING)
> &&
Is there much point in this check? I think we can assume drivers are
well-behaved and just leave the field at 0 if they don't support it?
> + status->rx_time) {
> + spin_lock_bh(&sta->lock);
> + sta->airtime_stats.rx_airtime += status->rx_time;
> + sta->airtime_deficit -= status->rx_time;
> + spin_unlock_bh(&sta->lock);
> + }
I can't say I'm a big fan of the locking here, we have multi-queue RX
where we spread the load across multiple CPUs, and this will lead to
massive cache-line bouncing. Maybe we can make it per-CPU or something?
This gets tricky though, so perhaps we can defer that to a separate
patch to make it multi-queue aware.
Perhaps for such drivers it'd be better anyway to report the used RX
airtime per station *separately*, as part of some kind of statistics
API, rather than inline through RX - there's no need for the inline
reporting after all as long as the values are updated frequently
enough.
> +++ b/net/mac80211/status.c
> @@ -823,6 +823,13 @@ static void __ieee80211_tx_status(struct
> ieee80211_hw *hw,
> ieee80211_lost_packet(sta, info);
> }
> }
> +
> + if (info->status.tx_time) {
> + spin_lock_bh(&sta->lock);
> + sta->airtime_stats.tx_airtime += info->status.tx_time;
> + sta->airtime_deficit -= info->status.tx_time;
> + spin_unlock_bh(&sta->lock);
> + }
> }
Those lines also seem pretty long, and the concerns from above apply.
Here you also don't have the hw_check,
> /* SNMP counters
> @@ -947,6 +954,14 @@ void ieee80211_tx_status_ext(struct ieee80211_hw
> *hw,
> sta->status_stats.retry_failed++;
> sta->status_stats.retry_count += retry_count;
>
> + if (ieee80211_hw_check(&local->hw, AIRTIME_ACCOUNTING) &&
but here you do?
tx_time already existed anyway though, for other purposes, so you
probably need the check (or do useless work if the driver uses tx_time
for WMM-AC but not for airtime accounting).
> if (list_empty(&txqi->schedule_order)) {
> - list_add_tail(&txqi->schedule_order, &local->active_txqs);
> + list_add_tail(&txqi->schedule_order, &local->active_txqs_new);
> ret = 1;
> }
lines seem long?
> + txqi = list_first_entry(head, struct txq_info,
> schedule_order);
> +
> + if (txqi->txq.sta) {
> + struct sta_info *sta = container_of(txqi->txq.sta,
> struct sta_info, sta);
> +
> + spin_lock_bh(&sta->lock);
> + if (sta->airtime_deficit < 0) {
> + sta->airtime_deficit +=
> IEEE80211_AIRTIME_QUANTUM;
> + list_move_tail(&txqi->schedule_order,
> &local->active_txqs_old);
> + spin_unlock_bh(&sta->lock);
> + goto begin;
> + }
> + spin_unlock_bh(&sta->lock);
> + }
ditto here.
johannes
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] mac80211: Add airtime accounting and scheduling to TXQs
2017-10-17 7:07 ` Johannes Berg
@ 2017-10-17 7:34 ` Toke Høiland-Jørgensen
2017-10-17 10:00 ` Johannes Berg
0 siblings, 1 reply; 20+ messages in thread
From: Toke Høiland-Jørgensen @ 2017-10-17 7:34 UTC (permalink / raw)
To: Johannes Berg, make-wifi-fast, linux-wireless
Johannes Berg <johannes@sipsolutions.net> writes:
>> Only ath9k currently sets the AIRTIME_ACCOUNTING flag.
>
> I think you should actually make that a separate patch.
>
> In the previous patch that's not possible or it breaks things, but
> this patch just adds a new feature that drivers _may_ use, they don't
> have to since it doesn't change the API, so you should split the
> patches.
Yeah, I did that initially. The reason I ended up squashing them is that
this patch moved the per-station 'airtime' debugfs-entry that was
previously created by ath9k into mac80211. I assumed it would create
problems if both the driver and mac80211 tried to create the same file;
not sure how to handle that if it's split into two patches?
> [snip ath9k, I don't really know anything about it]
>
>> --- a/include/net/mac80211.h
>> +++ b/include/net/mac80211.h
>> @@ -1202,6 +1202,7 @@ struct ieee80211_rx_status {
>> u32 device_timestamp;
>> u32 ampdu_reference;
>> u32 flag;
>> + u16 rx_time;
>
> I'd prefer this were called "airtime" or such,
Right. I picked rx_time for symmetry with the tx side; should I rename
that as well, then, or is asymmetry fine?
> and you clearly need to add documentation regarding this field. In
> particular, you should point to the IEEE80211_HW_AIRTIME_ACCOUNTING hw
> flag, and document what should be taken into account here (preamble
> length, IFS, rts/cts/ack, etc.?) and additionally how this should be
> handled wrt. A-MPDU (and A- MSDU where decapsulated by the device.)
Ah yes, of course; will add some documentation.
> For aggregation, either form, I expect you just want to see 0 for all
> but the very first frame? but that may be very difficult for some
> drivers to implement.
Well yeah, either report the airtime of the whole aggregate on the first
frame and zero on the rest, or report the aggregate overhead on the
first frame and the data+padding time on each frame. Depends on the
information the driver has handy / can calculate, I guess.
>> +++ b/net/mac80211/rx.c
>> @@ -1637,6 +1637,15 @@ ieee80211_rx_h_sta_process(struct
>> ieee80211_rx_data *rx)
>> if (ieee80211_vif_is_mesh(&rx->sdata->vif))
>> ieee80211_mps_rx_h_sta_process(sta, hdr);
>>
>> + /* airtime accounting */
>> + if (ieee80211_hw_check(&sta->local->hw, AIRTIME_ACCOUNTING)
>> &&
>
> Is there much point in this check? I think we can assume drivers are
> well-behaved and just leave the field at 0 if they don't support it?
I guess it could be omitted for the RX side, yeah. I added the flag
because the tx_time field overlaps with other fields in the tx_info
struct and so can be non-zero even if the driver doesn't add airtime
information (but seems I botched the TX side check as you noted below).
>> + status->rx_time) {
>> + spin_lock_bh(&sta->lock);
>> + sta->airtime_stats.rx_airtime += status->rx_time;
>> + sta->airtime_deficit -= status->rx_time;
>> + spin_unlock_bh(&sta->lock);
>> + }
>
> I can't say I'm a big fan of the locking here, we have multi-queue RX
> where we spread the load across multiple CPUs, and this will lead to
> massive cache-line bouncing. Maybe we can make it per-CPU or
> something?
A per-CPU counter on RX and a separate task that sums those into the
global counter, perhaps?
> This gets tricky though, so perhaps we can defer that to a separate
> patch to make it multi-queue aware.
ACK. I also don't have any hardware to test this, so would probably be
good to leave that as a separate entry once we're convinced that the
rest is correct.
> Perhaps for such drivers it'd be better anyway to report the used RX
> airtime per station *separately*, as part of some kind of statistics
> API, rather than inline through RX - there's no need for the inline
> reporting after all as long as the values are updated frequently
> enough.
Ah yes, that would be an option as well.
>> +++ b/net/mac80211/status.c
>> @@ -823,6 +823,13 @@ static void __ieee80211_tx_status(struct
>> ieee80211_hw *hw,
>> ieee80211_lost_packet(sta, info);
>> }
>> }
>> +
>> + if (info->status.tx_time) {
>> + spin_lock_bh(&sta->lock);
>> + sta->airtime_stats.tx_airtime += info->status.tx_time;
>> + sta->airtime_deficit -= info->status.tx_time;
>> + spin_unlock_bh(&sta->lock);
>> + }
>> }
>
> Those lines also seem pretty long, and the concerns from above apply.
>
> Here you also don't have the hw_check,
Yeah, oops. :)
[snip]
Will fix the rest of your comments and resend. Thanks!
-Toke
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] mac80211: Add airtime accounting and scheduling to TXQs
2017-10-17 7:34 ` Toke Høiland-Jørgensen
@ 2017-10-17 10:00 ` Johannes Berg
2017-10-17 10:09 ` Toke Høiland-Jørgensen
0 siblings, 1 reply; 20+ messages in thread
From: Johannes Berg @ 2017-10-17 10:00 UTC (permalink / raw)
To: Toke Høiland-Jørgensen, make-wifi-fast, linux-wireless
On Tue, 2017-10-17 at 09:34 +0200, Toke Høiland-Jørgensen wrote:
> Yeah, I did that initially. The reason I ended up squashing them is
> that
> this patch moved the per-station 'airtime' debugfs-entry that was
> previously created by ath9k into mac80211. I assumed it would create
> problems if both the driver and mac80211 tried to create the same
> file;
> not sure how to handle that if it's split into two patches?
That might indeed be problematic, so perhaps just use a different name?
Then again, if adding the entry fails in ath9k, nothing will happen
since it doesn't even check the return value - so I think it doesn't
actually matter.
> > I'd prefer this were called "airtime" or such,
>
> Right. I picked rx_time for symmetry with the tx side; should I
> rename that as well, then, or is asymmetry fine?
Let's not change the tx side in this patch, but we can clean it up
later I guess.
johannes
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] mac80211: Add airtime accounting and scheduling to TXQs
2017-10-17 10:00 ` Johannes Berg
@ 2017-10-17 10:09 ` Toke Høiland-Jørgensen
0 siblings, 0 replies; 20+ messages in thread
From: Toke Høiland-Jørgensen @ 2017-10-17 10:09 UTC (permalink / raw)
To: Johannes Berg, make-wifi-fast, linux-wireless
Johannes Berg <johannes@sipsolutions.net> writes:
> On Tue, 2017-10-17 at 09:34 +0200, Toke H=C3=B8iland-J=C3=B8rgensen wrote:
>
>> Yeah, I did that initially. The reason I ended up squashing them is
>> that
>> this patch moved the per-station 'airtime' debugfs-entry that was
>> previously created by ath9k into mac80211. I assumed it would create
>> problems if both the driver and mac80211 tried to create the same
>> file;
>> not sure how to handle that if it's split into two patches?
>
> That might indeed be problematic, so perhaps just use a different
> name?
Well I would like to keep compatibility with my tooling that reads the
values :)
> Then again, if adding the entry fails in ath9k, nothing will happen
> since it doesn't even check the return value - so I think it doesn't
> actually matter.
Right, I'll try that, and otherwise create a separate patch to ath9k
that removes the debugfs entry before applying the mac80211 patch.
>> > I'd prefer this were called "airtime" or such,
>>=20
>> Right. I picked rx_time for symmetry with the tx side; should I
>> rename that as well, then, or is asymmetry fine?
>
> Let's not change the tx side in this patch, but we can clean it up
> later I guess.
ACK.
-Toke
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 1/3] mac80211: Add TXQ scheduling API
2017-10-16 16:09 [PATCH 1/2] mac80211: Add TXQ scheduling API Toke Høiland-Jørgensen
2017-10-16 16:09 ` [PATCH 2/2] mac80211: Add airtime accounting and scheduling to TXQs Toke Høiland-Jørgensen
2017-10-17 6:39 ` [PATCH 1/2] mac80211: Add TXQ scheduling API Johannes Berg
@ 2017-10-27 14:14 ` Toke Høiland-Jørgensen
2017-10-31 0:14 ` kbuild test robot
` (4 more replies)
2017-10-27 14:14 ` [PATCH v2 2/3] mac80211: Add airtime account and scheduling to TXQs Toke Høiland-Jørgensen
2017-10-27 14:14 ` [PATCH v2 3/3] ath9k: Switch to mac80211 airtime API Toke Høiland-Jørgensen
4 siblings, 5 replies; 20+ messages in thread
From: Toke Høiland-Jørgensen @ 2017-10-27 14:14 UTC (permalink / raw)
To: make-wifi-fast, linux-wireless; +Cc: Toke Høiland-Jørgensen
This adds an API to mac80211 to handle scheduling of TXQs and changes the
interface between driver and mac80211 for TXQ handling as follows:
- The wake_tx_queue callback interface no longer includes the TXQ. Instead,
the driver is expected to retrieve that from ieee80211_next_txq()
- Two new mac80211 functions are added: ieee80211_next_txq() and
ieee80211_schedule_txq(). The former returns the next TXQ that should be
scheduled, and is how the driver gets a queue to pull packets from. The
latter is called internally by mac80211 to start scheduling a queue, and
the driver is supposed to call it to re-schedule the TXQ after it is
finished pulling packets from it (unless the queue emptied).
The ath9k and ath10k drivers are changed to use the new API.
Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
---
Changes since v1:
- Also remove artxq->list member from ath10k
- Don't skip ieee80211_schedule_txq() calls due to boolean
short-circuiting
- Some documentation and checkpatch fixes
drivers/net/wireless/ath/ath10k/core.c | 2 -
drivers/net/wireless/ath/ath10k/core.h | 4 -
drivers/net/wireless/ath/ath10k/mac.c | 55 +++------
drivers/net/wireless/ath/ath9k/ath9k.h | 6 +-
drivers/net/wireless/ath/ath9k/main.c | 2 +-
drivers/net/wireless/ath/ath9k/xmit.c | 210 ++++++++-------------------------
include/net/mac80211.h | 37 +++++-
net/mac80211/agg-tx.c | 6 +-
net/mac80211/driver-ops.h | 12 +-
net/mac80211/ieee80211_i.h | 5 +
net/mac80211/main.c | 3 +
net/mac80211/sta_info.c | 7 +-
net/mac80211/trace.h | 32 +----
net/mac80211/tx.c | 49 +++++++-
14 files changed, 173 insertions(+), 257 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index a4f635820f35..759df3297d48 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -2561,9 +2561,7 @@ struct ath10k *ath10k_core_create(size_t priv_size, struct device *dev,
mutex_init(&ar->conf_mutex);
spin_lock_init(&ar->data_lock);
- spin_lock_init(&ar->txqs_lock);
- INIT_LIST_HEAD(&ar->txqs);
INIT_LIST_HEAD(&ar->peers);
init_waitqueue_head(&ar->peer_mapping_wq);
init_waitqueue_head(&ar->htt.empty_tx_wq);
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 949ebb3e967b..3c3b158cdb20 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -347,7 +347,6 @@ struct ath10k_peer {
};
struct ath10k_txq {
- struct list_head list;
unsigned long num_fw_queued;
unsigned long num_push_allowed;
};
@@ -892,10 +891,7 @@ struct ath10k {
/* protects shared structure data */
spinlock_t data_lock;
- /* protects: ar->txqs, artxq->list */
- spinlock_t txqs_lock;
- struct list_head txqs;
struct list_head arvifs;
struct list_head peers;
struct ath10k_peer *peer_map[ATH10K_MAX_NUM_PEER_IDS];
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 5683f1a5330e..f9c70d8c9a09 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -3820,12 +3820,10 @@ static void ath10k_mac_txq_init(struct ieee80211_txq *txq)
return;
artxq = (void *)txq->drv_priv;
- INIT_LIST_HEAD(&artxq->list);
}
static void ath10k_mac_txq_unref(struct ath10k *ar, struct ieee80211_txq *txq)
{
- struct ath10k_txq *artxq;
struct ath10k_skb_cb *cb;
struct sk_buff *msdu;
int msdu_id;
@@ -3833,12 +3831,6 @@ static void ath10k_mac_txq_unref(struct ath10k *ar, struct ieee80211_txq *txq)
if (!txq)
return;
- artxq = (void *)txq->drv_priv;
- spin_lock_bh(&ar->txqs_lock);
- if (!list_empty(&artxq->list))
- list_del_init(&artxq->list);
- spin_unlock_bh(&ar->txqs_lock);
-
spin_lock_bh(&ar->htt.tx_lock);
idr_for_each_entry(&ar->htt.pending_tx, msdu, msdu_id) {
cb = ATH10K_SKB_CB(msdu);
@@ -3968,23 +3960,17 @@ int ath10k_mac_tx_push_txq(struct ieee80211_hw *hw,
void ath10k_mac_tx_push_pending(struct ath10k *ar)
{
struct ieee80211_hw *hw = ar->hw;
- struct ieee80211_txq *txq;
- struct ath10k_txq *artxq;
- struct ath10k_txq *last;
+ struct ieee80211_txq *txq, *first = NULL;
int ret;
int max;
if (ar->htt.num_pending_tx >= (ar->htt.max_num_pending_tx / 2))
return;
- spin_lock_bh(&ar->txqs_lock);
rcu_read_lock();
- last = list_last_entry(&ar->txqs, struct ath10k_txq, list);
- while (!list_empty(&ar->txqs)) {
- artxq = list_first_entry(&ar->txqs, struct ath10k_txq, list);
- txq = container_of((void *)artxq, struct ieee80211_txq,
- drv_priv);
+ txq = ieee80211_next_txq(hw);
+ while (txq) {
/* Prevent aggressive sta/tid taking over tx queue */
max = 16;
@@ -3995,18 +3981,21 @@ void ath10k_mac_tx_push_pending(struct ath10k *ar)
break;
}
- list_del_init(&artxq->list);
if (ret != -ENOENT)
- list_add_tail(&artxq->list, &ar->txqs);
+ ieee80211_schedule_txq(hw, txq);
ath10k_htt_tx_txq_update(hw, txq);
- if (artxq == last || (ret < 0 && ret != -ENOENT))
+ if (first == txq || (ret < 0 && ret != -ENOENT))
break;
+
+ if (!first)
+ first = txq;
+
+ txq = ieee80211_next_txq(hw);
}
rcu_read_unlock();
- spin_unlock_bh(&ar->txqs_lock);
}
/************/
@@ -4240,34 +4229,22 @@ static void ath10k_mac_op_tx(struct ieee80211_hw *hw,
}
}
-static void ath10k_mac_op_wake_tx_queue(struct ieee80211_hw *hw,
- struct ieee80211_txq *txq)
+static void ath10k_mac_op_wake_tx_queue(struct ieee80211_hw *hw)
{
- struct ath10k *ar = hw->priv;
- struct ath10k_txq *artxq = (void *)txq->drv_priv;
- struct ieee80211_txq *f_txq;
- struct ath10k_txq *f_artxq;
+ struct ieee80211_txq *txq;
int ret = 0;
int max = 16;
- spin_lock_bh(&ar->txqs_lock);
- if (list_empty(&artxq->list))
- list_add_tail(&artxq->list, &ar->txqs);
-
- f_artxq = list_first_entry(&ar->txqs, struct ath10k_txq, list);
- f_txq = container_of((void *)f_artxq, struct ieee80211_txq, drv_priv);
- list_del_init(&f_artxq->list);
+ txq = ieee80211_next_txq(hw);
- while (ath10k_mac_tx_can_push(hw, f_txq) && max--) {
- ret = ath10k_mac_tx_push_txq(hw, f_txq);
+ while (ath10k_mac_tx_can_push(hw, txq) && max--) {
+ ret = ath10k_mac_tx_push_txq(hw, txq);
if (ret)
break;
}
if (ret != -ENOENT)
- list_add_tail(&f_artxq->list, &ar->txqs);
- spin_unlock_bh(&ar->txqs_lock);
+ ieee80211_schedule_txq(hw, txq);
- ath10k_htt_tx_txq_update(hw, f_txq);
ath10k_htt_tx_txq_update(hw, txq);
}
diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index cf076719c27e..f6c53df78856 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -246,7 +246,6 @@ struct ath_atx_tid {
s8 bar_index;
bool active;
bool clear_ps_filter;
- bool has_queued;
};
void __ath_tx_queue_tid(struct ath_softc *sc, struct ath_atx_tid *tid);
@@ -591,8 +590,7 @@ bool ath_drain_all_txq(struct ath_softc *sc);
void ath_draintxq(struct ath_softc *sc, struct ath_txq *txq);
void ath_tx_node_init(struct ath_softc *sc, struct ath_node *an);
void ath_tx_node_cleanup(struct ath_softc *sc, struct ath_node *an);
-void ath_txq_schedule(struct ath_softc *sc, struct ath_txq *txq);
-void ath_txq_schedule_all(struct ath_softc *sc);
+void ath_txq_schedule(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);
@@ -618,7 +616,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 *queue);
+void ath9k_wake_tx_queue(struct ieee80211_hw *hw);
/********/
/* VIFs */
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 8b4ac7f0a09b..7c2ba21fd972 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -265,7 +265,7 @@ static bool ath_complete_reset(struct ath_softc *sc, bool start)
}
work:
ath_restart_work(sc);
- ath_txq_schedule_all(sc);
+ ath_txq_schedule(sc);
}
sc->gtt_cnt = 0;
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index 396bf05c6bf6..bd438062a6db 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -112,62 +112,11 @@ void ath_txq_unlock_complete(struct ath_softc *sc, struct ath_txq *txq)
ath_tx_status(hw, skb);
}
-void __ath_tx_queue_tid(struct ath_softc *sc, struct ath_atx_tid *tid)
-{
- 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;
- u8 acno = TID_TO_WME_AC(tid->tidno);
-
- if (!ctx || !list_empty(&tid->list))
- return;
-
-
- acq = &ctx->acq[acno];
- if ((sc->airtime_flags & AIRTIME_USE_NEW_QUEUES) &&
- tid->an->airtime_deficit[acno] > 0)
- tid_list = &acq->acq_new;
- else
- tid_list = &acq->acq_old;
-
- list_add_tail(&tid->list, tid_list);
-}
-
-void ath_tx_queue_tid(struct ath_softc *sc, struct ath_atx_tid *tid)
-{
- struct ath_vif *avp = (struct ath_vif *) tid->an->vif->drv_priv;
- struct ath_chanctx *ctx = avp->chanctx;
- struct ath_acq *acq;
-
- if (!ctx || !list_empty(&tid->list))
- return;
-
- acq = &ctx->acq[TID_TO_WME_AC(tid->tidno)];
- spin_lock_bh(&acq->lock);
- __ath_tx_queue_tid(sc, tid);
- spin_unlock_bh(&acq->lock);
-}
-
-
-void ath9k_wake_tx_queue(struct ieee80211_hw *hw, struct ieee80211_txq *queue)
+void ath9k_wake_tx_queue(struct ieee80211_hw *hw)
{
struct ath_softc *sc = hw->priv;
- struct ath_common *common = ath9k_hw_common(sc->sc_ah);
- struct ath_atx_tid *tid = (struct ath_atx_tid *) queue->drv_priv;
- struct ath_txq *txq = tid->txq;
-
- ath_dbg(common, QUEUE, "Waking TX queue: %pM (%d)\n",
- queue->sta ? queue->sta->addr : queue->vif->addr,
- tid->tidno);
-
- ath_txq_lock(sc, txq);
- tid->has_queued = true;
- ath_tx_queue_tid(sc, tid);
- ath_txq_schedule(sc, txq);
-
- ath_txq_unlock(sc, txq);
+ ath_txq_schedule(sc);
}
static struct ath_frame_info *get_frame_info(struct sk_buff *skb)
@@ -230,14 +179,9 @@ ath_tid_pull(struct ath_atx_tid *tid)
struct ath_frame_info *fi;
int q;
- if (!tid->has_queued)
- return NULL;
-
skb = ieee80211_tx_dequeue(hw, txq);
- if (!skb) {
- tid->has_queued = false;
+ if (!skb)
return NULL;
- }
if (ath_tx_prepare(hw, skb, &txctl)) {
ieee80211_free_txskb(hw, skb);
@@ -254,12 +198,6 @@ ath_tid_pull(struct ath_atx_tid *tid)
return skb;
}
-
-static bool ath_tid_has_buffered(struct ath_atx_tid *tid)
-{
- return !skb_queue_empty(&tid->retry_q) || tid->has_queued;
-}
-
static struct sk_buff *ath_tid_dequeue(struct ath_atx_tid *tid)
{
struct sk_buff *skb;
@@ -671,7 +609,10 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
skb_queue_splice_tail(&bf_pending, &tid->retry_q);
if (!an->sleeping) {
- ath_tx_queue_tid(sc, tid);
+ struct ieee80211_txq *queue = container_of(
+ (void *)tid, struct ieee80211_txq, drv_priv);
+
+ ieee80211_schedule_txq(sc->hw, queue);
if (ts->ts_status & (ATH9K_TXERR_FILT | ATH9K_TXERR_XRETRY))
tid->clear_ps_filter = true;
@@ -719,8 +660,6 @@ static void ath_tx_count_airtime(struct ath_softc *sc, struct ath_node *an,
spin_lock_bh(&acq->lock);
an->airtime_deficit[q] -= airtime;
- if (an->airtime_deficit[q] <= 0)
- __ath_tx_queue_tid(sc, tid);
spin_unlock_bh(&acq->lock);
}
ath_debug_airtime(sc, an, 0, airtime);
@@ -770,8 +709,6 @@ static void ath_tx_process_buffer(struct ath_softc *sc, struct ath_txq *txq,
} else
ath_tx_complete_aggr(sc, txq, bf, bf_head, sta, tid, ts, txok);
- if (!flush)
- ath_txq_schedule(sc, txq);
}
static bool ath_lookup_legacy(struct ath_buf *bf)
@@ -1506,8 +1443,8 @@ ath_tx_form_burst(struct ath_softc *sc, struct ath_txq *txq,
} while (1);
}
-static bool ath_tx_sched_aggr(struct ath_softc *sc, struct ath_txq *txq,
- struct ath_atx_tid *tid)
+static int ath_tx_sched_aggr(struct ath_softc *sc, struct ath_txq *txq,
+ struct ath_atx_tid *tid)
{
struct ath_buf *bf;
struct ieee80211_tx_info *tx_info;
@@ -1515,21 +1452,18 @@ static bool ath_tx_sched_aggr(struct ath_softc *sc, struct ath_txq *txq,
int aggr_len = 0;
bool aggr;
- if (!ath_tid_has_buffered(tid))
- return false;
-
INIT_LIST_HEAD(&bf_q);
bf = ath_tx_get_tid_subframe(sc, txq, tid);
if (!bf)
- return false;
+ return -ENOENT;
tx_info = IEEE80211_SKB_CB(bf->bf_mpdu);
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)) {
__skb_queue_tail(&tid->retry_q, bf->bf_mpdu);
- return false;
+ return -ENOBUFS;
}
ath_set_rates(tid->an->vif, tid->an->sta, bf);
@@ -1539,7 +1473,7 @@ static bool ath_tx_sched_aggr(struct ath_softc *sc, struct ath_txq *txq,
ath_tx_form_burst(sc, txq, tid, &bf_q, bf);
if (list_empty(&bf_q))
- return false;
+ return -ENOENT;
if (tid->clear_ps_filter || tid->an->no_ps_filter) {
tid->clear_ps_filter = false;
@@ -1548,7 +1482,7 @@ static bool ath_tx_sched_aggr(struct ath_softc *sc, struct ath_txq *txq,
ath_tx_fill_desc(sc, bf, txq, aggr_len);
ath_tx_txqaddbuf(sc, txq, &bf_q, false);
- return true;
+ return 0;
}
int ath_tx_aggr_start(struct ath_softc *sc, struct ieee80211_sta *sta,
@@ -1611,52 +1545,49 @@ void ath_tx_aggr_sleep(struct ieee80211_sta *sta, struct ath_softc *sc,
{
struct ath_common *common = ath9k_hw_common(sc->sc_ah);
struct ath_atx_tid *tid;
- struct ath_txq *txq;
+ struct ieee80211_txq *queue;
int tidno;
ath_dbg(common, XMIT, "%s called\n", __func__);
for (tidno = 0; tidno < IEEE80211_NUM_TIDS; tidno++) {
tid = ath_node_to_tid(an, tidno);
- txq = tid->txq;
-
- ath_txq_lock(sc, txq);
-
- if (list_empty(&tid->list)) {
- ath_txq_unlock(sc, txq);
- continue;
- }
+ queue = container_of((void *)tid,
+ struct ieee80211_txq, drv_priv);
if (!skb_queue_empty(&tid->retry_q))
ieee80211_sta_set_buffered(sta, tid->tidno, true);
- list_del_init(&tid->list);
-
- ath_txq_unlock(sc, txq);
}
}
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 *queue;
struct ath_atx_tid *tid;
struct ath_txq *txq;
int tidno;
+ bool sched, wake = false;
ath_dbg(common, XMIT, "%s called\n", __func__);
for (tidno = 0; tidno < IEEE80211_NUM_TIDS; tidno++) {
tid = ath_node_to_tid(an, tidno);
txq = tid->txq;
+ queue = container_of((void *)tid,
+ struct ieee80211_txq, drv_priv);
ath_txq_lock(sc, txq);
tid->clear_ps_filter = true;
- if (ath_tid_has_buffered(tid)) {
- ath_tx_queue_tid(sc, tid);
- ath_txq_schedule(sc, txq);
- }
- ath_txq_unlock_complete(sc, txq);
+ sched = !skb_queue_empty(&tid->retry_q);
+ ath_txq_unlock(sc, txq);
+
+ if (sched && ieee80211_schedule_txq(sc->hw, queue))
+ wake = true;
}
+ if (wake)
+ ath_txq_schedule(sc);
}
void ath9k_release_buffered_frames(struct ieee80211_hw *hw,
@@ -1948,86 +1879,44 @@ void ath_tx_cleanupq(struct ath_softc *sc, struct ath_txq *txq)
/* For each acq entry, for each tid, try to schedule packets
* for transmit until ampdu_depth has reached min Q depth.
*/
-void ath_txq_schedule(struct ath_softc *sc, struct ath_txq *txq)
+void ath_txq_schedule(struct ath_softc *sc)
{
+ struct ieee80211_hw *hw = sc->hw;
struct ath_common *common = ath9k_hw_common(sc->sc_ah);
+ struct ieee80211_txq *queue;
struct ath_atx_tid *tid;
- struct list_head *tid_list;
- struct ath_acq *acq;
- bool active = AIRTIME_ACTIVE(sc->airtime_flags);
+ struct ath_txq *txq;
+ int ret = 0;
- if (txq->mac80211_qnum < 0)
+ if (test_bit(ATH_OP_HW_RESET, &common->op_flags))
return;
- if (test_bit(ATH_OP_HW_RESET, &common->op_flags))
+ queue = ieee80211_next_txq(hw);
+ if (!queue)
return;
- spin_lock_bh(&sc->chan_lock);
- rcu_read_lock();
- acq = &sc->cur_chan->acq[txq->mac80211_qnum];
+ tid = (struct ath_atx_tid *)queue->drv_priv;
+ txq = tid->txq;
- if (sc->cur_chan->stopped)
+ ath_txq_lock(sc, txq);
+ if (txq->mac80211_qnum < 0)
goto out;
-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);
-
- if (active && tid->an->airtime_deficit[txq->mac80211_qnum] <= 0) {
- spin_lock_bh(&acq->lock);
- tid->an->airtime_deficit[txq->mac80211_qnum] += ATH_AIRTIME_QUANTUM;
- list_move_tail(&tid->list, &acq->acq_old);
- spin_unlock_bh(&acq->lock);
- goto begin;
- }
-
- if (!ath_tid_has_buffered(tid)) {
- spin_lock_bh(&acq->lock);
- 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);
- }
- spin_unlock_bh(&acq->lock);
- goto begin;
- }
-
+ spin_lock_bh(&sc->chan_lock);
+ rcu_read_lock();
- /*
- * If we succeed in scheduling something, immediately restart to make
- * sure we keep the HW busy.
- */
- if(ath_tx_sched_aggr(sc, txq, tid)) {
- if (!active) {
- spin_lock_bh(&acq->lock);
- list_move_tail(&tid->list, &acq->acq_old);
- spin_unlock_bh(&acq->lock);
- }
- goto begin;
- }
+ if (!sc->cur_chan->stopped)
+ ret = ath_tx_sched_aggr(sc, txq, tid);
-out:
rcu_read_unlock();
spin_unlock_bh(&sc->chan_lock);
-}
-void ath_txq_schedule_all(struct ath_softc *sc)
-{
- struct ath_txq *txq;
- int i;
+out:
- for (i = 0; i < IEEE80211_NUM_ACS; i++) {
- txq = sc->tx.txq_map[i];
+ if (ret != -ENOENT)
+ ieee80211_schedule_txq(hw, queue);
- spin_lock_bh(&txq->axq_lock);
- ath_txq_schedule(sc, txq);
- spin_unlock_bh(&txq->axq_lock);
- }
+ ath_txq_unlock(sc, txq);
}
/***********/
@@ -2645,7 +2534,6 @@ static void ath_tx_processq(struct ath_softc *sc, struct ath_txq *txq)
if (list_empty(&txq->axq_q)) {
txq->axq_link = NULL;
- ath_txq_schedule(sc, txq);
break;
}
bf = list_first_entry(&txq->axq_q, struct ath_buf, list);
@@ -2697,6 +2585,7 @@ static void ath_tx_processq(struct ath_softc *sc, struct ath_txq *txq)
ath_tx_process_buffer(sc, txq, &ts, bf, &bf_head);
}
ath_txq_unlock_complete(sc, txq);
+ ath_txq_schedule(sc);
}
void ath_tx_tasklet(struct ath_softc *sc)
@@ -2711,6 +2600,7 @@ void ath_tx_tasklet(struct ath_softc *sc)
ath_tx_processq(sc, &sc->tx.txq[i]);
}
rcu_read_unlock();
+ ath_txq_schedule(sc);
}
void ath_tx_edma_tasklet(struct ath_softc *sc)
@@ -2796,6 +2686,7 @@ void ath_tx_edma_tasklet(struct ath_softc *sc)
ath_txq_unlock_complete(sc, txq);
}
rcu_read_unlock();
+ ath_txq_schedule(sc);
}
/*****************/
@@ -2875,7 +2766,6 @@ 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->has_queued = false;
__skb_queue_head_init(&tid->retry_q);
INIT_LIST_HEAD(&tid->list);
acno = TID_TO_WME_AC(tidno);
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index cc9073e45be9..715f45501aff 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -105,9 +105,12 @@
* The driver is expected to initialize its private per-queue data for stations
* and interfaces in the .add_interface and .sta_add ops.
*
- * The driver can't access the queue directly. To dequeue a frame, it calls
- * ieee80211_tx_dequeue(). Whenever mac80211 adds a new frame to a queue, it
- * calls the .wake_tx_queue driver op.
+ * The driver can't access the queue directly. To obtain the next queue to pull
+ * frames from, the driver calls ieee80211_next_txq(). To dequeue a frame from a
+ * txq, it calls ieee80211_tx_dequeue(). Whenever mac80211 adds a new frame to a
+ * queue, it calls the .wake_tx_queue driver op. The driver is expected to
+ * re-schedule the txq using ieee80211_schedule_txq() if it is still active
+ * after the driver has finished pulling packets from it.
*
* For AP powersave TIM handling, the driver only needs to indicate if it has
* buffered packets in the driver specific data structures by calling
@@ -3723,8 +3726,7 @@ struct ieee80211_ops {
struct ieee80211_vif *vif,
struct ieee80211_tdls_ch_sw_params *params);
- void (*wake_tx_queue)(struct ieee80211_hw *hw,
- struct ieee80211_txq *txq);
+ void (*wake_tx_queue)(struct ieee80211_hw *hw);
void (*sync_rx_queues)(struct ieee80211_hw *hw);
int (*start_nan)(struct ieee80211_hw *hw,
@@ -5869,13 +5871,36 @@ void ieee80211_unreserve_tid(struct ieee80211_sta *sta, u8 tid);
* ieee80211_tx_dequeue - dequeue a packet from a software tx queue
*
* @hw: pointer as obtained from ieee80211_alloc_hw()
- * @txq: pointer obtained from station or virtual interface
+ * @txq: pointer obtained from ieee80211_next_txq()
*
* Returns the skb if successful, %NULL if no frame was available.
*/
struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
struct ieee80211_txq *txq);
+/**
+ * ieee80211_schedule_txq - add txq to scheduling loop
+ *
+ * @hw: pointer as obtained from ieee80211_alloc_hw()
+ * @txq: pointer obtained from station or virtual interface
+ *
+ * Returns %true if the txq was actually added to the scheduling,
+ * %false otherwise.
+ */
+bool ieee80211_schedule_txq(struct ieee80211_hw *hw,
+ struct ieee80211_txq *txq);
+
+/**
+ * ieee80211_next_txq - get next tx queue to pull packets from
+ *
+ * @hw: pointer as obtained from ieee80211_alloc_hw()
+ *
+ * Returns the next txq if successful, %NULL if no queue is eligible. If a txq
+ * is returned, it will have been removed from the scheduler queue and needs to
+ * be re-scheduled with ieee80211_schedule_txq() to continue to be active.
+ */
+struct ieee80211_txq *ieee80211_next_txq(struct ieee80211_hw *hw);
+
/**
* ieee80211_txq_get_depth - get pending frame/byte count of given txq
*
diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c
index bef516ec47f9..569b5b5c6d70 100644
--- a/net/mac80211/agg-tx.c
+++ b/net/mac80211/agg-tx.c
@@ -226,9 +226,13 @@ ieee80211_agg_start_txq(struct sta_info *sta, int tid, bool enable)
clear_bit(IEEE80211_TXQ_AMPDU, &txqi->flags);
clear_bit(IEEE80211_TXQ_STOP, &txqi->flags);
+
+ if (!ieee80211_schedule_txq(&sta->sdata->local->hw, txq))
+ return;
+
local_bh_disable();
rcu_read_lock();
- drv_wake_tx_queue(sta->sdata->local, txqi);
+ drv_wake_tx_queue(sta->sdata->local);
rcu_read_unlock();
local_bh_enable();
}
diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
index 09f77e4a8a79..e20a9e2acd53 100644
--- a/net/mac80211/driver-ops.h
+++ b/net/mac80211/driver-ops.h
@@ -1157,16 +1157,10 @@ drv_tdls_recv_channel_switch(struct ieee80211_local *local,
trace_drv_return_void(local);
}
-static inline void drv_wake_tx_queue(struct ieee80211_local *local,
- struct txq_info *txq)
+static inline void drv_wake_tx_queue(struct ieee80211_local *local)
{
- struct ieee80211_sub_if_data *sdata = vif_to_sdata(txq->txq.vif);
-
- if (!check_sdata_in_driver(sdata))
- return;
-
- trace_drv_wake_tx_queue(local, sdata, txq);
- local->ops->wake_tx_queue(&local->hw, &txq->txq);
+ trace_drv_wake_tx_queue(local);
+ local->ops->wake_tx_queue(&local->hw);
}
static inline int drv_start_nan(struct ieee80211_local *local,
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 9675814f64db..95be548b1d4f 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -832,6 +832,7 @@ struct txq_info {
struct codel_vars def_cvars;
struct codel_stats cstats;
struct sk_buff_head frags;
+ struct list_head schedule_order;
unsigned long flags;
/* keep last! */
@@ -1121,6 +1122,10 @@ struct ieee80211_local {
struct codel_vars *cvars;
struct codel_params cparams;
+ /* protects active_txqs and txqi->schedule_order */
+ spinlock_t active_txq_lock;
+ struct list_head active_txqs;
+
const struct ieee80211_ops *ops;
/*
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 8aa1f5b6a051..9ad0556aa24b 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -616,6 +616,9 @@ struct ieee80211_hw *ieee80211_alloc_hw_nm(size_t priv_data_len,
spin_lock_init(&local->rx_path_lock);
spin_lock_init(&local->queue_stop_reason_lock);
+ INIT_LIST_HEAD(&local->active_txqs);
+ spin_lock_init(&local->active_txq_lock);
+
INIT_LIST_HEAD(&local->chanctx_list);
mutex_init(&local->chanctx_mtx);
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 9673e157bf8f..da3737d4778f 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -1246,12 +1246,17 @@ void ieee80211_sta_ps_deliver_wakeup(struct sta_info *sta)
drv_sta_notify(local, sdata, STA_NOTIFY_AWAKE, &sta->sta);
if (sta->sta.txq[0]) {
+ bool wake = false;
+
for (i = 0; i < ARRAY_SIZE(sta->sta.txq); i++) {
if (!txq_has_queue(sta->sta.txq[i]))
continue;
- drv_wake_tx_queue(local, to_txq_info(sta->sta.txq[i]));
+ if (ieee80211_schedule_txq(&local->hw, sta->sta.txq[i]))
+ wake = true;
}
+ if (wake)
+ drv_wake_tx_queue(local);
}
skb_queue_head_init(&pending);
diff --git a/net/mac80211/trace.h b/net/mac80211/trace.h
index 3d9ac17af407..f51eade947ee 100644
--- a/net/mac80211/trace.h
+++ b/net/mac80211/trace.h
@@ -2549,35 +2549,9 @@ TRACE_EVENT(drv_tdls_recv_channel_switch,
)
);
-TRACE_EVENT(drv_wake_tx_queue,
- TP_PROTO(struct ieee80211_local *local,
- struct ieee80211_sub_if_data *sdata,
- struct txq_info *txq),
-
- TP_ARGS(local, sdata, txq),
-
- TP_STRUCT__entry(
- LOCAL_ENTRY
- VIF_ENTRY
- STA_ENTRY
- __field(u8, ac)
- __field(u8, tid)
- ),
-
- TP_fast_assign(
- struct ieee80211_sta *sta = txq->txq.sta;
-
- LOCAL_ASSIGN;
- VIF_ASSIGN;
- STA_ASSIGN;
- __entry->ac = txq->txq.ac;
- __entry->tid = txq->txq.tid;
- ),
-
- TP_printk(
- LOCAL_PR_FMT VIF_PR_FMT STA_PR_FMT " ac:%d tid:%d",
- LOCAL_PR_ARG, VIF_PR_ARG, STA_PR_ARG, __entry->ac, __entry->tid
- )
+DEFINE_EVENT(local_only_evt, drv_wake_tx_queue,
+ TP_PROTO(struct ieee80211_local *local),
+ TP_ARGS(local)
);
#endif /* !__MAC80211_DRIVER_TRACE || TRACE_HEADER_MULTI_READ */
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 94826680cf2b..824d87e6d3eb 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1405,6 +1405,7 @@ void ieee80211_txq_init(struct ieee80211_sub_if_data *sdata,
codel_vars_init(&txqi->def_cvars);
codel_stats_init(&txqi->cstats);
__skb_queue_head_init(&txqi->frags);
+ INIT_LIST_HEAD(&txqi->schedule_order);
txqi->txq.vif = &sdata->vif;
@@ -1428,6 +1429,7 @@ void ieee80211_txq_purge(struct ieee80211_local *local,
fq_tin_reset(fq, tin, fq_skb_free_func);
ieee80211_purge_tx_queue(&local->hw, &txqi->frags);
+ list_del_init(&txqi->schedule_order);
}
int ieee80211_txq_setup_flows(struct ieee80211_local *local)
@@ -1524,7 +1526,8 @@ static bool ieee80211_queue_skb(struct ieee80211_local *local,
ieee80211_txq_enqueue(local, txqi, skb);
spin_unlock_bh(&fq->lock);
- drv_wake_tx_queue(local, txqi);
+ if (ieee80211_schedule_txq(&local->hw, &txqi->txq))
+ drv_wake_tx_queue(local);
return true;
}
@@ -3517,6 +3520,50 @@ struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
}
EXPORT_SYMBOL(ieee80211_tx_dequeue);
+bool ieee80211_schedule_txq(struct ieee80211_hw *hw,
+ struct ieee80211_txq *txq)
+{
+ struct ieee80211_local *local = hw_to_local(hw);
+ struct txq_info *txqi = to_txq_info(txq);
+ bool ret = false;
+
+ spin_lock_bh(&local->active_txq_lock);
+
+ if (list_empty(&txqi->schedule_order)) {
+ list_add_tail(&txqi->schedule_order, &local->active_txqs);
+ ret = true;
+ }
+
+ spin_unlock_bh(&local->active_txq_lock);
+
+ return ret;
+}
+EXPORT_SYMBOL(ieee80211_schedule_txq);
+
+struct ieee80211_txq *ieee80211_next_txq(struct ieee80211_hw *hw)
+{
+ struct ieee80211_local *local = hw_to_local(hw);
+ struct txq_info *txqi = NULL;
+
+ spin_lock_bh(&local->active_txq_lock);
+
+ if (list_empty(&local->active_txqs))
+ goto out;
+
+ txqi = list_first_entry(&local->active_txqs,
+ struct txq_info, schedule_order);
+ list_del_init(&txqi->schedule_order);
+
+out:
+ spin_unlock_bh(&local->active_txq_lock);
+
+ if (!txqi)
+ return NULL;
+
+ return &txqi->txq;
+}
+EXPORT_SYMBOL(ieee80211_next_txq);
+
void __ieee80211_subif_start_xmit(struct sk_buff *skb,
struct net_device *dev,
u32 info_flags)
--
2.14.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 2/3] mac80211: Add airtime account and scheduling to TXQs
2017-10-16 16:09 [PATCH 1/2] mac80211: Add TXQ scheduling API Toke Høiland-Jørgensen
` (2 preceding siblings ...)
2017-10-27 14:14 ` [PATCH v2 1/3] " Toke Høiland-Jørgensen
@ 2017-10-27 14:14 ` Toke Høiland-Jørgensen
2017-10-27 14:14 ` [PATCH v2 3/3] ath9k: Switch to mac80211 airtime API Toke Høiland-Jørgensen
4 siblings, 0 replies; 20+ messages in thread
From: Toke Høiland-Jørgensen @ 2017-10-27 14:14 UTC (permalink / raw)
To: make-wifi-fast, linux-wireless; +Cc: Toke Høiland-Jørgensen
This adds airtime accounting and scheduling to the mac80211 TXQ scheduler. A new
hardware flag, AIRTIME_ACCOUNTING, is added that drivers can set if they support
reporting airtime usage of transmissions. When this flag is set, mac80211 will
expect the actual airtime usage to be reported in the tx_time and rx_time fields
of the respective status structs.
When airtime information is present, mac80211 will schedule TXQs (through
ieee80211_next_txq()) in a way that enforces airtime fairness between active
stations. This scheduling works the same way as the ath9k in-driver airtime
fairness scheduling.
Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
---
Changes since v1:
- Split ath9k changes into separate patch
- Rename RX status variable to 'airtime', add documentation
- Add documentation on usage to the IEEE80211_HW_AIRTIME_ACCOUNTING hw
accounting flag
- Remove HW flag check on RX side, add it where it was missing on TX
side
- Fix checkpatch issues
This still implements the basic fairness that is more or less equivalent
to the previous implementation in ath9k. I figure any changes to a
different mechanism should be made separately anyway, and I don't
believe they will require any changes to the driver/mac80211 API.
include/net/mac80211.h | 24 ++++++++++++++++++++++++
net/mac80211/debugfs.c | 1 +
net/mac80211/debugfs_sta.c | 29 +++++++++++++++++++++++++++++
net/mac80211/ieee80211_i.h | 9 +++++++--
net/mac80211/main.c | 3 ++-
net/mac80211/rx.c | 8 ++++++++
net/mac80211/sta_info.c | 2 ++
net/mac80211/sta_info.h | 7 +++++++
net/mac80211/status.c | 16 ++++++++++++++++
net/mac80211/tx.c | 31 ++++++++++++++++++++++++++-----
10 files changed, 122 insertions(+), 8 deletions(-)
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 715f45501aff..453653d082af 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1188,6 +1188,8 @@ enum mac80211_rx_encoding {
* HT or VHT is used (%RX_FLAG_HT/%RX_FLAG_VHT)
* @nss: number of streams (VHT and HE only)
* @flag: %RX_FLAG_\*
+ * @airtime: Duration of frame in usec. See @IEEE80211_HW_AIRTIME_ACCOUNTING for
+ * how to use this.
* @encoding: &enum mac80211_rx_encoding
* @bw: &enum rate_info_bw
* @enc_flags: uses bits from &enum mac80211_rx_encoding_flags
@@ -1202,6 +1204,7 @@ struct ieee80211_rx_status {
u32 device_timestamp;
u32 ampdu_reference;
u32 flag;
+ u16 airtime;
u16 freq;
u8 enc_flags;
u8 encoding:2, bw:3;
@@ -2059,6 +2062,26 @@ struct ieee80211_txq {
* The stack will not do fragmentation.
* The callback for @set_frag_threshold should be set as well.
*
+ * @IEEE80211_HW_AIRTIME_ACCOUNTING: Hardware supports accounting the airtime
+ * usage of other stations and reports it in the @tx_time and/or @airtime
+ * fields of the TX/RX status structs.
+ * When setting this flag, the driver should ensure that the respective
+ * fields in the TX and RX status structs are always either zero or
+ * contains a valid duration for the frame in usec. The driver can choose
+ * to report either or both of TX and RX airtime, but it is recommended to
+ * report both.
+ * The reported airtime should as a minimum include all time that is spent
+ * transmitting to the remote station, including overhead and padding, but
+ * not including time spent waiting for a TXOP. If the time is not reported
+ * by the hardware it can in some cases be calculated from the rate and
+ * known frame composition. When possible, the time should include any
+ * failed transmission attempts.
+ * For aggregated frames, there are two possible strategies to report the
+ * airtime: Either include the airtime of the entire aggregate in the first
+ * (or last) frame and leave the others at zero. Alternatively, include the
+ * overhead of the full aggregate in the first or last frame and report the
+ * time of each frame + padding not including the full aggregate overhead.
+ *
* @NUM_IEEE80211_HW_FLAGS: number of hardware flags, used for sizing arrays
*/
enum ieee80211_hw_flags {
@@ -2101,6 +2124,7 @@ enum ieee80211_hw_flags {
IEEE80211_HW_TX_FRAG_LIST,
IEEE80211_HW_REPORTS_LOW_ACK,
IEEE80211_HW_SUPPORTS_TX_FRAG,
+ IEEE80211_HW_AIRTIME_ACCOUNTING,
/* keep last, obviously */
NUM_IEEE80211_HW_FLAGS
diff --git a/net/mac80211/debugfs.c b/net/mac80211/debugfs.c
index 5fae001f286c..20e3aa0e8464 100644
--- a/net/mac80211/debugfs.c
+++ b/net/mac80211/debugfs.c
@@ -211,6 +211,7 @@ static const char *hw_flag_names[] = {
FLAG(TX_FRAG_LIST),
FLAG(REPORTS_LOW_ACK),
FLAG(SUPPORTS_TX_FRAG),
+ FLAG(AIRTIME_ACCOUNTING),
#undef FLAG
};
diff --git a/net/mac80211/debugfs_sta.c b/net/mac80211/debugfs_sta.c
index b15412c21ac9..40dba446836f 100644
--- a/net/mac80211/debugfs_sta.c
+++ b/net/mac80211/debugfs_sta.c
@@ -188,6 +188,32 @@ static ssize_t sta_aqm_read(struct file *file, char __user *userbuf,
}
STA_OPS(aqm);
+static ssize_t sta_airtime_read(struct file *file, char __user *userbuf,
+ size_t count, loff_t *ppos)
+{
+ struct sta_info *sta = file->private_data;
+ size_t bufsz = 200;
+ char *buf = kzalloc(bufsz, GFP_KERNEL), *p = buf;
+ ssize_t rv;
+
+ if (!buf)
+ return -ENOMEM;
+
+ spin_lock_bh(&sta->lock);
+
+ p += scnprintf(p, bufsz + buf - p,
+ "RX: %llu us\nTX: %llu us\nDeficit: %lld us\n",
+ sta->airtime_stats.rx_airtime,
+ sta->airtime_stats.tx_airtime,
+ sta->airtime_deficit);
+
+ spin_unlock_bh(&sta->lock);
+ rv = simple_read_from_buffer(userbuf, count, ppos, buf, p - buf);
+ kfree(buf);
+ return rv;
+}
+STA_OPS(airtime);
+
static ssize_t sta_agg_status_read(struct file *file, char __user *userbuf,
size_t count, loff_t *ppos)
{
@@ -542,6 +568,9 @@ void ieee80211_sta_debugfs_add(struct sta_info *sta)
if (local->ops->wake_tx_queue)
DEBUGFS_ADD(aqm);
+ if (ieee80211_hw_check(&local->hw, AIRTIME_ACCOUNTING))
+ DEBUGFS_ADD(airtime);
+
if (sizeof(sta->driver_buffered_tids) == sizeof(u32))
debugfs_create_x32("driver_buffered_tids", 0400,
sta->debugfs_dir,
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 95be548b1d4f..e74aeff7662a 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -90,6 +90,9 @@ extern const u8 ieee80211_ac_to_qos_mask[IEEE80211_NUM_ACS];
#define IEEE80211_MAX_NAN_INSTANCE_ID 255
+/* How much to increase airtime deficit on each scheduling round */
+#define IEEE80211_AIRTIME_QUANTUM 1000 /* usec */
+
struct ieee80211_fragment_entry {
struct sk_buff_head skb_list;
unsigned long first_frag_time;
@@ -1122,9 +1125,11 @@ struct ieee80211_local {
struct codel_vars *cvars;
struct codel_params cparams;
- /* protects active_txqs and txqi->schedule_order */
+ /* protects active_txqs_{new,old} and txqi->schedule_order */
spinlock_t active_txq_lock;
- struct list_head active_txqs;
+ struct list_head active_txqs_new;
+ struct list_head active_txqs_old;
+
const struct ieee80211_ops *ops;
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 9ad0556aa24b..acf73c398d9b 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -616,7 +616,8 @@ struct ieee80211_hw *ieee80211_alloc_hw_nm(size_t priv_data_len,
spin_lock_init(&local->rx_path_lock);
spin_lock_init(&local->queue_stop_reason_lock);
- INIT_LIST_HEAD(&local->active_txqs);
+ INIT_LIST_HEAD(&local->active_txqs_new);
+ INIT_LIST_HEAD(&local->active_txqs_old);
spin_lock_init(&local->active_txq_lock);
INIT_LIST_HEAD(&local->chanctx_list);
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 70e9d2ca8bbe..a6166850b971 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -1637,6 +1637,14 @@ ieee80211_rx_h_sta_process(struct ieee80211_rx_data *rx)
if (ieee80211_vif_is_mesh(&rx->sdata->vif))
ieee80211_mps_rx_h_sta_process(sta, hdr);
+ /* airtime accounting */
+ if (status->airtime) {
+ spin_lock_bh(&sta->lock);
+ sta->airtime_stats.rx_airtime += status->airtime;
+ sta->airtime_deficit -= status->airtime;
+ spin_unlock_bh(&sta->lock);
+ }
+
/*
* Drop (qos-)data::nullfunc frames silently, since they
* are used only to control station power saving mode.
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index da3737d4778f..e8511922a7c7 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -433,6 +433,8 @@ struct sta_info *sta_info_alloc(struct ieee80211_sub_if_data *sdata,
sta->cparams.interval = MS2TIME(100);
sta->cparams.ecn = true;
+ sta->airtime_deficit = IEEE80211_AIRTIME_QUANTUM;
+
sta_dbg(sdata, "Allocated STA %pM\n", sta->sta.addr);
return sta;
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 5c54acd10562..5d1802f22550 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -550,6 +550,13 @@ struct sta_info {
} tx_stats;
u16 tid_seq[IEEE80211_QOS_CTL_TID_MASK + 1];
+ /* Airtime stats and deficit, protected by lock */
+ struct {
+ u64 rx_airtime;
+ u64 tx_airtime;
+ } airtime_stats;
+ s64 airtime_deficit;
+
/*
* Aggregation information, locked with lock.
*/
diff --git a/net/mac80211/status.c b/net/mac80211/status.c
index da7427a41529..cf7956f7a040 100644
--- a/net/mac80211/status.c
+++ b/net/mac80211/status.c
@@ -823,6 +823,14 @@ static void __ieee80211_tx_status(struct ieee80211_hw *hw,
ieee80211_lost_packet(sta, info);
}
}
+
+ if (ieee80211_hw_check(&local->hw, AIRTIME_ACCOUNTING) &&
+ info->status.tx_time) {
+ spin_lock_bh(&sta->lock);
+ sta->airtime_stats.tx_airtime += info->status.tx_time;
+ sta->airtime_deficit -= info->status.tx_time;
+ spin_unlock_bh(&sta->lock);
+ }
}
/* SNMP counters
@@ -947,6 +955,14 @@ void ieee80211_tx_status_ext(struct ieee80211_hw *hw,
sta->status_stats.retry_failed++;
sta->status_stats.retry_count += retry_count;
+ if (ieee80211_hw_check(&local->hw, AIRTIME_ACCOUNTING) &&
+ info->status.tx_time) {
+ spin_lock_bh(&sta->lock);
+ sta->airtime_stats.tx_airtime += info->status.tx_time;
+ sta->airtime_deficit -= info->status.tx_time;
+ spin_unlock_bh(&sta->lock);
+ }
+
if (acked) {
sta->status_stats.last_ack = jiffies;
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 824d87e6d3eb..409e84509625 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -3530,7 +3530,7 @@ bool ieee80211_schedule_txq(struct ieee80211_hw *hw,
spin_lock_bh(&local->active_txq_lock);
if (list_empty(&txqi->schedule_order)) {
- list_add_tail(&txqi->schedule_order, &local->active_txqs);
+ list_add_tail(&txqi->schedule_order, &local->active_txqs_new);
ret = true;
}
@@ -3544,14 +3544,35 @@ struct ieee80211_txq *ieee80211_next_txq(struct ieee80211_hw *hw)
{
struct ieee80211_local *local = hw_to_local(hw);
struct txq_info *txqi = NULL;
+ struct list_head *head;
spin_lock_bh(&local->active_txq_lock);
- if (list_empty(&local->active_txqs))
- goto out;
+begin:
+ head = &local->active_txqs_new;
+ if (list_empty(head)) {
+ head = &local->active_txqs_old;
+ if (list_empty(head))
+ goto out;
+ }
+
+ txqi = list_first_entry(head, struct txq_info, schedule_order);
+
+ if (txqi->txq.sta) {
+ struct sta_info *sta = container_of(txqi->txq.sta,
+ struct sta_info, sta);
+
+ spin_lock_bh(&sta->lock);
+ if (sta->airtime_deficit < 0) {
+ sta->airtime_deficit += IEEE80211_AIRTIME_QUANTUM;
+ list_move_tail(&txqi->schedule_order,
+ &local->active_txqs_old);
+ spin_unlock_bh(&sta->lock);
+ goto begin;
+ }
+ spin_unlock_bh(&sta->lock);
+ }
- txqi = list_first_entry(&local->active_txqs,
- struct txq_info, schedule_order);
list_del_init(&txqi->schedule_order);
out:
--
2.14.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 3/3] ath9k: Switch to mac80211 airtime API
2017-10-16 16:09 [PATCH 1/2] mac80211: Add TXQ scheduling API Toke Høiland-Jørgensen
` (3 preceding siblings ...)
2017-10-27 14:14 ` [PATCH v2 2/3] mac80211: Add airtime account and scheduling to TXQs Toke Høiland-Jørgensen
@ 2017-10-27 14:14 ` Toke Høiland-Jørgensen
4 siblings, 0 replies; 20+ messages in thread
From: Toke Høiland-Jørgensen @ 2017-10-27 14:14 UTC (permalink / raw)
To: make-wifi-fast, linux-wireless; +Cc: Toke Høiland-Jørgensen
This removes the in-driver airtime fairness scheduling from ath9k and
switches the driver to use the API introduced in mac80211 in the
previous commit.
Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
---
Changes since v1:
- Split out ath9k changes into separate patch
drivers/net/wireless/ath/ath9k/ath9k.h | 7 +---
drivers/net/wireless/ath/ath9k/debug.h | 8 -----
drivers/net/wireless/ath/ath9k/debug_sta.c | 54 ------------------------------
drivers/net/wireless/ath/ath9k/init.c | 4 +--
drivers/net/wireless/ath/ath9k/recv.c | 11 ++----
drivers/net/wireless/ath/ath9k/xmit.c | 21 ++++--------
6 files changed, 12 insertions(+), 93 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index f6c53df78856..f6e8f9b1b984 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -112,8 +112,6 @@ int ath_descdma_setup(struct ath_softc *sc, struct ath_descdma *dd,
#define ATH_TXFIFO_DEPTH 8
#define ATH_TX_ERROR 0x01
-#define ATH_AIRTIME_QUANTUM 300 /* usec */
-
/* Stop tx traffic 1ms before the GO goes away */
#define ATH_P2P_PS_STOP_TIME 1000
@@ -215,6 +213,7 @@ struct ath_buf_state {
bool stale;
u16 seqno;
unsigned long bfs_paprd_timestamp;
+ u32 airtime;
};
struct ath_buf {
@@ -262,12 +261,9 @@ struct ath_node {
bool sleeping;
bool no_ps_filter;
- s64 airtime_deficit[IEEE80211_NUM_ACS];
- 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];
@@ -986,7 +982,6 @@ void ath_ant_comb_scan(struct ath_softc *sc, struct ath_rx_status *rs);
#define AIRTIME_USE_TX BIT(0)
#define AIRTIME_USE_RX BIT(1)
-#define AIRTIME_USE_NEW_QUEUES BIT(2)
#define AIRTIME_ACTIVE(flags) (!!(flags & (AIRTIME_USE_TX|AIRTIME_USE_RX)))
struct ath_softc {
diff --git a/drivers/net/wireless/ath/ath9k/debug.h b/drivers/net/wireless/ath/ath9k/debug.h
index 249f8141cd00..559d9628f280 100644
--- a/drivers/net/wireless/ath/ath9k/debug.h
+++ b/drivers/net/wireless/ath/ath9k/debug.h
@@ -319,20 +319,12 @@ 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_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_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 efc692ee67d4..89ba95287a8b 100644
--- a/drivers/net/wireless/ath/ath9k/debug_sta.c
+++ b/drivers/net/wireless/ath/ath9k/debug_sta.c
@@ -242,59 +242,6 @@ 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;
- static const char *qname[4] = {
- "VO", "VI", "BE", "BK"
- };
- u32 len = 0, size = 256;
- char *buf;
- size_t retval;
- int i;
-
- 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: ");
- for (i = 0; i < 4; i++)
- len += scnprintf(buf+len, size - len, "%s: %lld us ", qname[i], an->airtime_deficit[i]);
- if (len < size)
- buf[len++] = '\n';
-
- 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,
@@ -304,5 +251,4 @@ 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 bb7936090b91..b4c6fc49b19e 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -620,8 +620,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 |
- AIRTIME_USE_NEW_QUEUES);
+ sc->airtime_flags = (AIRTIME_USE_TX | AIRTIME_USE_RX);
sc->sc_ah = ah;
sc->dfs_detector = dfs_pattern_detector_init(common, NL80211_DFS_UNSET);
@@ -873,6 +872,7 @@ static void ath9k_set_hw_capab(struct ath_softc *sc, struct ieee80211_hw *hw)
ieee80211_hw_set(hw, HOST_BROADCAST_PS_BUFFERING);
ieee80211_hw_set(hw, SUPPORT_FAST_XMIT);
ieee80211_hw_set(hw, SUPPORTS_CLONED_SKBS);
+ ieee80211_hw_set(hw, AIRTIME_ACCOUNTING);
if (ath9k_ps_enable)
ieee80211_hw_set(hw, SUPPORTS_PS);
diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
index 2197aee2bb72..69a4ba5d4013 100644
--- a/drivers/net/wireless/ath/ath9k/recv.c
+++ b/drivers/net/wireless/ath/ath9k/recv.c
@@ -1054,14 +1054,9 @@ static void ath_rx_count_airtime(struct ath_softc *sc,
len, rxs->rate_idx, is_sp);
}
- if (!!(sc->airtime_flags & AIRTIME_USE_RX)) {
- spin_lock_bh(&acq->lock);
- an->airtime_deficit[acno] -= airtime;
- if (an->airtime_deficit[acno] <= 0)
- __ath_tx_queue_tid(sc, ATH_AN_2_TID(an, tidno));
- spin_unlock_bh(&acq->lock);
- }
- ath_debug_airtime(sc, an, airtime, 0);
+ if (!!(sc->airtime_flags & AIRTIME_USE_RX))
+ rxs->airtime = airtime;
+
exit:
rcu_read_unlock();
}
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index bd438062a6db..5ffd44a4b3de 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -640,11 +640,9 @@ 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_node *an,
- struct ath_atx_tid *tid, struct ath_buf *bf,
+static void ath_tx_count_airtime(struct ath_softc *sc, struct ath_buf *bf,
struct ath_tx_status *ts)
{
- struct ath_txq *txq = tid->txq;
u32 airtime = 0;
int i;
@@ -654,15 +652,9 @@ static void ath_tx_count_airtime(struct ath_softc *sc, struct ath_node *an,
airtime += rate_dur * bf->rates[i].count;
}
- if (sc->airtime_flags & AIRTIME_USE_TX) {
- int q = txq->mac80211_qnum;
- struct ath_acq *acq = &sc->cur_chan->acq[q];
+ if (sc->airtime_flags & AIRTIME_USE_TX)
+ bf->bf_state.airtime = airtime;
- spin_lock_bh(&acq->lock);
- an->airtime_deficit[q] -= airtime;
- spin_unlock_bh(&acq->lock);
- }
- ath_debug_airtime(sc, an, 0, airtime);
}
static void ath_tx_process_buffer(struct ath_softc *sc, struct ath_txq *txq,
@@ -692,7 +684,7 @@ static void ath_tx_process_buffer(struct ath_softc *sc, struct ath_txq *txq,
if (sta) {
struct ath_node *an = (struct ath_node *)sta->drv_priv;
tid = ath_get_skb_tid(sc, an, bf->bf_mpdu);
- ath_tx_count_airtime(sc, an, tid, bf, ts);
+ ath_tx_count_airtime(sc, bf, ts);
if (ts->ts_status & (ATH9K_TXERR_FILT | ATH9K_TXERR_XRETRY))
tid->clear_ps_filter = true;
}
@@ -2428,6 +2420,8 @@ static void ath_tx_complete_buf(struct ath_softc *sc, struct ath_buf *bf,
if (ts->ts_status & ATH9K_TXERR_FILT)
tx_info->flags |= IEEE80211_TX_STAT_TX_FILTERED;
+ tx_info->status.tx_time = min_t(u32, 0xffff, bf->bf_state.airtime);
+
dma_unmap_single(sc->dev, bf->bf_buf_addr, skb->len, DMA_TO_DEVICE);
bf->bf_buf_addr = 0;
if (sc->tx99_state)
@@ -2754,9 +2748,6 @@ void ath_tx_node_init(struct ath_softc *sc, struct ath_node *an)
struct ath_atx_tid *tid;
int tidno, acno;
- for (acno = 0; acno < IEEE80211_NUM_ACS; acno++)
- an->airtime_deficit[acno] = ATH_AIRTIME_QUANTUM;
-
for (tidno = 0; tidno < IEEE80211_NUM_TIDS; tidno++) {
tid = ath_node_to_tid(an, tidno);
tid->an = an;
--
2.14.2
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/3] mac80211: Add TXQ scheduling API
2017-10-27 14:14 ` [PATCH v2 1/3] " Toke Høiland-Jørgensen
@ 2017-10-31 0:14 ` kbuild test robot
2017-10-31 0:45 ` kbuild test robot
` (3 subsequent siblings)
4 siblings, 0 replies; 20+ messages in thread
From: kbuild test robot @ 2017-10-31 0:14 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: kbuild-all, make-wifi-fast, linux-wireless,
Toke Høiland-Jørgensen
[-- Attachment #1: Type: text/plain, Size: 1181 bytes --]
Hi Toke,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on mac80211/master]
[also build test ERROR on v4.14-rc7 next-20171018]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Toke-H-iland-J-rgensen/mac80211-Add-TXQ-scheduling-API/20171030-220903
base: https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211.git master
config: x86_64-allyesdebian (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
Note: the linux-review/Toke-H-iland-J-rgensen/mac80211-Add-TXQ-scheduling-API/20171030-220903 HEAD aa503ef75660b7c9cb9ede292f03047372517d4e builds fine.
It only hurts bisectibility.
All errors (new ones prefixed by >>):
drivers/net/wireless/ath/ath9k/recv.o: In function `ath_rx_tasklet':
>> recv.c:(.text+0x16bc): undefined reference to `__ath_tx_queue_tid'
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 39531 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/3] mac80211: Add TXQ scheduling API
2017-10-27 14:14 ` [PATCH v2 1/3] " Toke Høiland-Jørgensen
2017-10-31 0:14 ` kbuild test robot
@ 2017-10-31 0:45 ` kbuild test robot
2017-10-31 11:27 ` [PATCH v3 " Toke Høiland-Jørgensen
` (2 subsequent siblings)
4 siblings, 0 replies; 20+ messages in thread
From: kbuild test robot @ 2017-10-31 0:45 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: kbuild-all, make-wifi-fast, linux-wireless,
Toke Høiland-Jørgensen
[-- Attachment #1: Type: text/plain, Size: 1273 bytes --]
Hi Toke,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on mac80211/master]
[also build test ERROR on v4.14-rc7]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Toke-H-iland-J-rgensen/mac80211-Add-TXQ-scheduling-API/20171030-220903
base: https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211.git master
config: mips-ip27_defconfig (attached as .config)
compiler: mips64-linux-gnuabi64-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=mips
Note: the linux-review/Toke-H-iland-J-rgensen/mac80211-Add-TXQ-scheduling-API/20171030-220903 HEAD aa503ef75660b7c9cb9ede292f03047372517d4e builds fine.
It only hurts bisectibility.
All errors (new ones prefixed by >>):
>> ERROR: "__ath_tx_queue_tid" [drivers/net/wireless/ath/ath9k/ath9k.ko] undefined!
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 15984 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v3 1/3] mac80211: Add TXQ scheduling API
2017-10-27 14:14 ` [PATCH v2 1/3] " Toke Høiland-Jørgensen
2017-10-31 0:14 ` kbuild test robot
2017-10-31 0:45 ` kbuild test robot
@ 2017-10-31 11:27 ` Toke Høiland-Jørgensen
2017-12-14 11:33 ` Felix Fietkau
2017-10-31 11:27 ` [PATCH v3 2/3] mac80211: Add airtime account and scheduling to TXQs Toke Høiland-Jørgensen
2017-10-31 11:27 ` [PATCH v3 3/3] ath9k: Switch to mac80211 airtime API Toke Høiland-Jørgensen
4 siblings, 1 reply; 20+ messages in thread
From: Toke Høiland-Jørgensen @ 2017-10-31 11:27 UTC (permalink / raw)
To: make-wifi-fast, linux-wireless; +Cc: Toke Høiland-Jørgensen
This adds an API to mac80211 to handle scheduling of TXQs and changes the
interface between driver and mac80211 for TXQ handling as follows:
- The wake_tx_queue callback interface no longer includes the TXQ. Instead,
the driver is expected to retrieve that from ieee80211_next_txq()
- Two new mac80211 functions are added: ieee80211_next_txq() and
ieee80211_schedule_txq(). The former returns the next TXQ that should be
scheduled, and is how the driver gets a queue to pull packets from. The
latter is called internally by mac80211 to start scheduling a queue, and
the driver is supposed to call it to re-schedule the TXQ after it is
finished pulling packets from it (unless the queue emptied).
The ath9k and ath10k drivers are changed to use the new API.
Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
---
Changes since v2:
- Fix build error of first patch in the series reported by the kbuild
bot
drivers/net/wireless/ath/ath10k/core.c | 2 -
drivers/net/wireless/ath/ath10k/core.h | 4 -
drivers/net/wireless/ath/ath10k/mac.c | 55 +++------
drivers/net/wireless/ath/ath9k/ath9k.h | 9 +-
drivers/net/wireless/ath/ath9k/main.c | 2 +-
drivers/net/wireless/ath/ath9k/recv.c | 2 -
drivers/net/wireless/ath/ath9k/xmit.c | 210 ++++++++-------------------------
include/net/mac80211.h | 37 +++++-
net/mac80211/agg-tx.c | 6 +-
net/mac80211/driver-ops.h | 12 +-
net/mac80211/ieee80211_i.h | 5 +
net/mac80211/main.c | 3 +
net/mac80211/sta_info.c | 7 +-
net/mac80211/trace.h | 32 +----
net/mac80211/tx.c | 49 +++++++-
15 files changed, 173 insertions(+), 262 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index a4f635820f35..759df3297d48 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -2561,9 +2561,7 @@ struct ath10k *ath10k_core_create(size_t priv_size, struct device *dev,
mutex_init(&ar->conf_mutex);
spin_lock_init(&ar->data_lock);
- spin_lock_init(&ar->txqs_lock);
- INIT_LIST_HEAD(&ar->txqs);
INIT_LIST_HEAD(&ar->peers);
init_waitqueue_head(&ar->peer_mapping_wq);
init_waitqueue_head(&ar->htt.empty_tx_wq);
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 949ebb3e967b..3c3b158cdb20 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -347,7 +347,6 @@ struct ath10k_peer {
};
struct ath10k_txq {
- struct list_head list;
unsigned long num_fw_queued;
unsigned long num_push_allowed;
};
@@ -892,10 +891,7 @@ struct ath10k {
/* protects shared structure data */
spinlock_t data_lock;
- /* protects: ar->txqs, artxq->list */
- spinlock_t txqs_lock;
- struct list_head txqs;
struct list_head arvifs;
struct list_head peers;
struct ath10k_peer *peer_map[ATH10K_MAX_NUM_PEER_IDS];
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 5683f1a5330e..f9c70d8c9a09 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -3820,12 +3820,10 @@ static void ath10k_mac_txq_init(struct ieee80211_txq *txq)
return;
artxq = (void *)txq->drv_priv;
- INIT_LIST_HEAD(&artxq->list);
}
static void ath10k_mac_txq_unref(struct ath10k *ar, struct ieee80211_txq *txq)
{
- struct ath10k_txq *artxq;
struct ath10k_skb_cb *cb;
struct sk_buff *msdu;
int msdu_id;
@@ -3833,12 +3831,6 @@ static void ath10k_mac_txq_unref(struct ath10k *ar, struct ieee80211_txq *txq)
if (!txq)
return;
- artxq = (void *)txq->drv_priv;
- spin_lock_bh(&ar->txqs_lock);
- if (!list_empty(&artxq->list))
- list_del_init(&artxq->list);
- spin_unlock_bh(&ar->txqs_lock);
-
spin_lock_bh(&ar->htt.tx_lock);
idr_for_each_entry(&ar->htt.pending_tx, msdu, msdu_id) {
cb = ATH10K_SKB_CB(msdu);
@@ -3968,23 +3960,17 @@ int ath10k_mac_tx_push_txq(struct ieee80211_hw *hw,
void ath10k_mac_tx_push_pending(struct ath10k *ar)
{
struct ieee80211_hw *hw = ar->hw;
- struct ieee80211_txq *txq;
- struct ath10k_txq *artxq;
- struct ath10k_txq *last;
+ struct ieee80211_txq *txq, *first = NULL;
int ret;
int max;
if (ar->htt.num_pending_tx >= (ar->htt.max_num_pending_tx / 2))
return;
- spin_lock_bh(&ar->txqs_lock);
rcu_read_lock();
- last = list_last_entry(&ar->txqs, struct ath10k_txq, list);
- while (!list_empty(&ar->txqs)) {
- artxq = list_first_entry(&ar->txqs, struct ath10k_txq, list);
- txq = container_of((void *)artxq, struct ieee80211_txq,
- drv_priv);
+ txq = ieee80211_next_txq(hw);
+ while (txq) {
/* Prevent aggressive sta/tid taking over tx queue */
max = 16;
@@ -3995,18 +3981,21 @@ void ath10k_mac_tx_push_pending(struct ath10k *ar)
break;
}
- list_del_init(&artxq->list);
if (ret != -ENOENT)
- list_add_tail(&artxq->list, &ar->txqs);
+ ieee80211_schedule_txq(hw, txq);
ath10k_htt_tx_txq_update(hw, txq);
- if (artxq == last || (ret < 0 && ret != -ENOENT))
+ if (first == txq || (ret < 0 && ret != -ENOENT))
break;
+
+ if (!first)
+ first = txq;
+
+ txq = ieee80211_next_txq(hw);
}
rcu_read_unlock();
- spin_unlock_bh(&ar->txqs_lock);
}
/************/
@@ -4240,34 +4229,22 @@ static void ath10k_mac_op_tx(struct ieee80211_hw *hw,
}
}
-static void ath10k_mac_op_wake_tx_queue(struct ieee80211_hw *hw,
- struct ieee80211_txq *txq)
+static void ath10k_mac_op_wake_tx_queue(struct ieee80211_hw *hw)
{
- struct ath10k *ar = hw->priv;
- struct ath10k_txq *artxq = (void *)txq->drv_priv;
- struct ieee80211_txq *f_txq;
- struct ath10k_txq *f_artxq;
+ struct ieee80211_txq *txq;
int ret = 0;
int max = 16;
- spin_lock_bh(&ar->txqs_lock);
- if (list_empty(&artxq->list))
- list_add_tail(&artxq->list, &ar->txqs);
-
- f_artxq = list_first_entry(&ar->txqs, struct ath10k_txq, list);
- f_txq = container_of((void *)f_artxq, struct ieee80211_txq, drv_priv);
- list_del_init(&f_artxq->list);
+ txq = ieee80211_next_txq(hw);
- while (ath10k_mac_tx_can_push(hw, f_txq) && max--) {
- ret = ath10k_mac_tx_push_txq(hw, f_txq);
+ while (ath10k_mac_tx_can_push(hw, txq) && max--) {
+ ret = ath10k_mac_tx_push_txq(hw, txq);
if (ret)
break;
}
if (ret != -ENOENT)
- list_add_tail(&f_artxq->list, &ar->txqs);
- spin_unlock_bh(&ar->txqs_lock);
+ ieee80211_schedule_txq(hw, txq);
- ath10k_htt_tx_txq_update(hw, f_txq);
ath10k_htt_tx_txq_update(hw, txq);
}
diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index cf076719c27e..a5088792b769 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -246,12 +246,8 @@ struct ath_atx_tid {
s8 bar_index;
bool active;
bool clear_ps_filter;
- bool has_queued;
};
-void __ath_tx_queue_tid(struct ath_softc *sc, struct ath_atx_tid *tid);
-void ath_tx_queue_tid(struct ath_softc *sc, struct ath_atx_tid *tid);
-
struct ath_node {
struct ath_softc *sc;
struct ieee80211_sta *sta; /* station struct we're part of */
@@ -591,8 +587,7 @@ bool ath_drain_all_txq(struct ath_softc *sc);
void ath_draintxq(struct ath_softc *sc, struct ath_txq *txq);
void ath_tx_node_init(struct ath_softc *sc, struct ath_node *an);
void ath_tx_node_cleanup(struct ath_softc *sc, struct ath_node *an);
-void ath_txq_schedule(struct ath_softc *sc, struct ath_txq *txq);
-void ath_txq_schedule_all(struct ath_softc *sc);
+void ath_txq_schedule(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);
@@ -618,7 +613,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 *queue);
+void ath9k_wake_tx_queue(struct ieee80211_hw *hw);
/********/
/* VIFs */
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 8b4ac7f0a09b..7c2ba21fd972 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -265,7 +265,7 @@ static bool ath_complete_reset(struct ath_softc *sc, bool start)
}
work:
ath_restart_work(sc);
- ath_txq_schedule_all(sc);
+ ath_txq_schedule(sc);
}
sc->gtt_cnt = 0;
diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
index 2197aee2bb72..a768e841524d 100644
--- a/drivers/net/wireless/ath/ath9k/recv.c
+++ b/drivers/net/wireless/ath/ath9k/recv.c
@@ -1057,8 +1057,6 @@ static void ath_rx_count_airtime(struct ath_softc *sc,
if (!!(sc->airtime_flags & AIRTIME_USE_RX)) {
spin_lock_bh(&acq->lock);
an->airtime_deficit[acno] -= airtime;
- if (an->airtime_deficit[acno] <= 0)
- __ath_tx_queue_tid(sc, ATH_AN_2_TID(an, tidno));
spin_unlock_bh(&acq->lock);
}
ath_debug_airtime(sc, an, airtime, 0);
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index 396bf05c6bf6..bd438062a6db 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -112,62 +112,11 @@ void ath_txq_unlock_complete(struct ath_softc *sc, struct ath_txq *txq)
ath_tx_status(hw, skb);
}
-void __ath_tx_queue_tid(struct ath_softc *sc, struct ath_atx_tid *tid)
-{
- 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;
- u8 acno = TID_TO_WME_AC(tid->tidno);
-
- if (!ctx || !list_empty(&tid->list))
- return;
-
-
- acq = &ctx->acq[acno];
- if ((sc->airtime_flags & AIRTIME_USE_NEW_QUEUES) &&
- tid->an->airtime_deficit[acno] > 0)
- tid_list = &acq->acq_new;
- else
- tid_list = &acq->acq_old;
-
- list_add_tail(&tid->list, tid_list);
-}
-
-void ath_tx_queue_tid(struct ath_softc *sc, struct ath_atx_tid *tid)
-{
- struct ath_vif *avp = (struct ath_vif *) tid->an->vif->drv_priv;
- struct ath_chanctx *ctx = avp->chanctx;
- struct ath_acq *acq;
-
- if (!ctx || !list_empty(&tid->list))
- return;
-
- acq = &ctx->acq[TID_TO_WME_AC(tid->tidno)];
- spin_lock_bh(&acq->lock);
- __ath_tx_queue_tid(sc, tid);
- spin_unlock_bh(&acq->lock);
-}
-
-
-void ath9k_wake_tx_queue(struct ieee80211_hw *hw, struct ieee80211_txq *queue)
+void ath9k_wake_tx_queue(struct ieee80211_hw *hw)
{
struct ath_softc *sc = hw->priv;
- struct ath_common *common = ath9k_hw_common(sc->sc_ah);
- struct ath_atx_tid *tid = (struct ath_atx_tid *) queue->drv_priv;
- struct ath_txq *txq = tid->txq;
-
- ath_dbg(common, QUEUE, "Waking TX queue: %pM (%d)\n",
- queue->sta ? queue->sta->addr : queue->vif->addr,
- tid->tidno);
-
- ath_txq_lock(sc, txq);
- tid->has_queued = true;
- ath_tx_queue_tid(sc, tid);
- ath_txq_schedule(sc, txq);
-
- ath_txq_unlock(sc, txq);
+ ath_txq_schedule(sc);
}
static struct ath_frame_info *get_frame_info(struct sk_buff *skb)
@@ -230,14 +179,9 @@ ath_tid_pull(struct ath_atx_tid *tid)
struct ath_frame_info *fi;
int q;
- if (!tid->has_queued)
- return NULL;
-
skb = ieee80211_tx_dequeue(hw, txq);
- if (!skb) {
- tid->has_queued = false;
+ if (!skb)
return NULL;
- }
if (ath_tx_prepare(hw, skb, &txctl)) {
ieee80211_free_txskb(hw, skb);
@@ -254,12 +198,6 @@ ath_tid_pull(struct ath_atx_tid *tid)
return skb;
}
-
-static bool ath_tid_has_buffered(struct ath_atx_tid *tid)
-{
- return !skb_queue_empty(&tid->retry_q) || tid->has_queued;
-}
-
static struct sk_buff *ath_tid_dequeue(struct ath_atx_tid *tid)
{
struct sk_buff *skb;
@@ -671,7 +609,10 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
skb_queue_splice_tail(&bf_pending, &tid->retry_q);
if (!an->sleeping) {
- ath_tx_queue_tid(sc, tid);
+ struct ieee80211_txq *queue = container_of(
+ (void *)tid, struct ieee80211_txq, drv_priv);
+
+ ieee80211_schedule_txq(sc->hw, queue);
if (ts->ts_status & (ATH9K_TXERR_FILT | ATH9K_TXERR_XRETRY))
tid->clear_ps_filter = true;
@@ -719,8 +660,6 @@ static void ath_tx_count_airtime(struct ath_softc *sc, struct ath_node *an,
spin_lock_bh(&acq->lock);
an->airtime_deficit[q] -= airtime;
- if (an->airtime_deficit[q] <= 0)
- __ath_tx_queue_tid(sc, tid);
spin_unlock_bh(&acq->lock);
}
ath_debug_airtime(sc, an, 0, airtime);
@@ -770,8 +709,6 @@ static void ath_tx_process_buffer(struct ath_softc *sc, struct ath_txq *txq,
} else
ath_tx_complete_aggr(sc, txq, bf, bf_head, sta, tid, ts, txok);
- if (!flush)
- ath_txq_schedule(sc, txq);
}
static bool ath_lookup_legacy(struct ath_buf *bf)
@@ -1506,8 +1443,8 @@ ath_tx_form_burst(struct ath_softc *sc, struct ath_txq *txq,
} while (1);
}
-static bool ath_tx_sched_aggr(struct ath_softc *sc, struct ath_txq *txq,
- struct ath_atx_tid *tid)
+static int ath_tx_sched_aggr(struct ath_softc *sc, struct ath_txq *txq,
+ struct ath_atx_tid *tid)
{
struct ath_buf *bf;
struct ieee80211_tx_info *tx_info;
@@ -1515,21 +1452,18 @@ static bool ath_tx_sched_aggr(struct ath_softc *sc, struct ath_txq *txq,
int aggr_len = 0;
bool aggr;
- if (!ath_tid_has_buffered(tid))
- return false;
-
INIT_LIST_HEAD(&bf_q);
bf = ath_tx_get_tid_subframe(sc, txq, tid);
if (!bf)
- return false;
+ return -ENOENT;
tx_info = IEEE80211_SKB_CB(bf->bf_mpdu);
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)) {
__skb_queue_tail(&tid->retry_q, bf->bf_mpdu);
- return false;
+ return -ENOBUFS;
}
ath_set_rates(tid->an->vif, tid->an->sta, bf);
@@ -1539,7 +1473,7 @@ static bool ath_tx_sched_aggr(struct ath_softc *sc, struct ath_txq *txq,
ath_tx_form_burst(sc, txq, tid, &bf_q, bf);
if (list_empty(&bf_q))
- return false;
+ return -ENOENT;
if (tid->clear_ps_filter || tid->an->no_ps_filter) {
tid->clear_ps_filter = false;
@@ -1548,7 +1482,7 @@ static bool ath_tx_sched_aggr(struct ath_softc *sc, struct ath_txq *txq,
ath_tx_fill_desc(sc, bf, txq, aggr_len);
ath_tx_txqaddbuf(sc, txq, &bf_q, false);
- return true;
+ return 0;
}
int ath_tx_aggr_start(struct ath_softc *sc, struct ieee80211_sta *sta,
@@ -1611,52 +1545,49 @@ void ath_tx_aggr_sleep(struct ieee80211_sta *sta, struct ath_softc *sc,
{
struct ath_common *common = ath9k_hw_common(sc->sc_ah);
struct ath_atx_tid *tid;
- struct ath_txq *txq;
+ struct ieee80211_txq *queue;
int tidno;
ath_dbg(common, XMIT, "%s called\n", __func__);
for (tidno = 0; tidno < IEEE80211_NUM_TIDS; tidno++) {
tid = ath_node_to_tid(an, tidno);
- txq = tid->txq;
-
- ath_txq_lock(sc, txq);
-
- if (list_empty(&tid->list)) {
- ath_txq_unlock(sc, txq);
- continue;
- }
+ queue = container_of((void *)tid,
+ struct ieee80211_txq, drv_priv);
if (!skb_queue_empty(&tid->retry_q))
ieee80211_sta_set_buffered(sta, tid->tidno, true);
- list_del_init(&tid->list);
-
- ath_txq_unlock(sc, txq);
}
}
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 *queue;
struct ath_atx_tid *tid;
struct ath_txq *txq;
int tidno;
+ bool sched, wake = false;
ath_dbg(common, XMIT, "%s called\n", __func__);
for (tidno = 0; tidno < IEEE80211_NUM_TIDS; tidno++) {
tid = ath_node_to_tid(an, tidno);
txq = tid->txq;
+ queue = container_of((void *)tid,
+ struct ieee80211_txq, drv_priv);
ath_txq_lock(sc, txq);
tid->clear_ps_filter = true;
- if (ath_tid_has_buffered(tid)) {
- ath_tx_queue_tid(sc, tid);
- ath_txq_schedule(sc, txq);
- }
- ath_txq_unlock_complete(sc, txq);
+ sched = !skb_queue_empty(&tid->retry_q);
+ ath_txq_unlock(sc, txq);
+
+ if (sched && ieee80211_schedule_txq(sc->hw, queue))
+ wake = true;
}
+ if (wake)
+ ath_txq_schedule(sc);
}
void ath9k_release_buffered_frames(struct ieee80211_hw *hw,
@@ -1948,86 +1879,44 @@ void ath_tx_cleanupq(struct ath_softc *sc, struct ath_txq *txq)
/* For each acq entry, for each tid, try to schedule packets
* for transmit until ampdu_depth has reached min Q depth.
*/
-void ath_txq_schedule(struct ath_softc *sc, struct ath_txq *txq)
+void ath_txq_schedule(struct ath_softc *sc)
{
+ struct ieee80211_hw *hw = sc->hw;
struct ath_common *common = ath9k_hw_common(sc->sc_ah);
+ struct ieee80211_txq *queue;
struct ath_atx_tid *tid;
- struct list_head *tid_list;
- struct ath_acq *acq;
- bool active = AIRTIME_ACTIVE(sc->airtime_flags);
+ struct ath_txq *txq;
+ int ret = 0;
- if (txq->mac80211_qnum < 0)
+ if (test_bit(ATH_OP_HW_RESET, &common->op_flags))
return;
- if (test_bit(ATH_OP_HW_RESET, &common->op_flags))
+ queue = ieee80211_next_txq(hw);
+ if (!queue)
return;
- spin_lock_bh(&sc->chan_lock);
- rcu_read_lock();
- acq = &sc->cur_chan->acq[txq->mac80211_qnum];
+ tid = (struct ath_atx_tid *)queue->drv_priv;
+ txq = tid->txq;
- if (sc->cur_chan->stopped)
+ ath_txq_lock(sc, txq);
+ if (txq->mac80211_qnum < 0)
goto out;
-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);
-
- if (active && tid->an->airtime_deficit[txq->mac80211_qnum] <= 0) {
- spin_lock_bh(&acq->lock);
- tid->an->airtime_deficit[txq->mac80211_qnum] += ATH_AIRTIME_QUANTUM;
- list_move_tail(&tid->list, &acq->acq_old);
- spin_unlock_bh(&acq->lock);
- goto begin;
- }
-
- if (!ath_tid_has_buffered(tid)) {
- spin_lock_bh(&acq->lock);
- 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);
- }
- spin_unlock_bh(&acq->lock);
- goto begin;
- }
-
+ spin_lock_bh(&sc->chan_lock);
+ rcu_read_lock();
- /*
- * If we succeed in scheduling something, immediately restart to make
- * sure we keep the HW busy.
- */
- if(ath_tx_sched_aggr(sc, txq, tid)) {
- if (!active) {
- spin_lock_bh(&acq->lock);
- list_move_tail(&tid->list, &acq->acq_old);
- spin_unlock_bh(&acq->lock);
- }
- goto begin;
- }
+ if (!sc->cur_chan->stopped)
+ ret = ath_tx_sched_aggr(sc, txq, tid);
-out:
rcu_read_unlock();
spin_unlock_bh(&sc->chan_lock);
-}
-void ath_txq_schedule_all(struct ath_softc *sc)
-{
- struct ath_txq *txq;
- int i;
+out:
- for (i = 0; i < IEEE80211_NUM_ACS; i++) {
- txq = sc->tx.txq_map[i];
+ if (ret != -ENOENT)
+ ieee80211_schedule_txq(hw, queue);
- spin_lock_bh(&txq->axq_lock);
- ath_txq_schedule(sc, txq);
- spin_unlock_bh(&txq->axq_lock);
- }
+ ath_txq_unlock(sc, txq);
}
/***********/
@@ -2645,7 +2534,6 @@ static void ath_tx_processq(struct ath_softc *sc, struct ath_txq *txq)
if (list_empty(&txq->axq_q)) {
txq->axq_link = NULL;
- ath_txq_schedule(sc, txq);
break;
}
bf = list_first_entry(&txq->axq_q, struct ath_buf, list);
@@ -2697,6 +2585,7 @@ static void ath_tx_processq(struct ath_softc *sc, struct ath_txq *txq)
ath_tx_process_buffer(sc, txq, &ts, bf, &bf_head);
}
ath_txq_unlock_complete(sc, txq);
+ ath_txq_schedule(sc);
}
void ath_tx_tasklet(struct ath_softc *sc)
@@ -2711,6 +2600,7 @@ void ath_tx_tasklet(struct ath_softc *sc)
ath_tx_processq(sc, &sc->tx.txq[i]);
}
rcu_read_unlock();
+ ath_txq_schedule(sc);
}
void ath_tx_edma_tasklet(struct ath_softc *sc)
@@ -2796,6 +2686,7 @@ void ath_tx_edma_tasklet(struct ath_softc *sc)
ath_txq_unlock_complete(sc, txq);
}
rcu_read_unlock();
+ ath_txq_schedule(sc);
}
/*****************/
@@ -2875,7 +2766,6 @@ 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->has_queued = false;
__skb_queue_head_init(&tid->retry_q);
INIT_LIST_HEAD(&tid->list);
acno = TID_TO_WME_AC(tidno);
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index cc9073e45be9..715f45501aff 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -105,9 +105,12 @@
* The driver is expected to initialize its private per-queue data for stations
* and interfaces in the .add_interface and .sta_add ops.
*
- * The driver can't access the queue directly. To dequeue a frame, it calls
- * ieee80211_tx_dequeue(). Whenever mac80211 adds a new frame to a queue, it
- * calls the .wake_tx_queue driver op.
+ * The driver can't access the queue directly. To obtain the next queue to pull
+ * frames from, the driver calls ieee80211_next_txq(). To dequeue a frame from a
+ * txq, it calls ieee80211_tx_dequeue(). Whenever mac80211 adds a new frame to a
+ * queue, it calls the .wake_tx_queue driver op. The driver is expected to
+ * re-schedule the txq using ieee80211_schedule_txq() if it is still active
+ * after the driver has finished pulling packets from it.
*
* For AP powersave TIM handling, the driver only needs to indicate if it has
* buffered packets in the driver specific data structures by calling
@@ -3723,8 +3726,7 @@ struct ieee80211_ops {
struct ieee80211_vif *vif,
struct ieee80211_tdls_ch_sw_params *params);
- void (*wake_tx_queue)(struct ieee80211_hw *hw,
- struct ieee80211_txq *txq);
+ void (*wake_tx_queue)(struct ieee80211_hw *hw);
void (*sync_rx_queues)(struct ieee80211_hw *hw);
int (*start_nan)(struct ieee80211_hw *hw,
@@ -5869,13 +5871,36 @@ void ieee80211_unreserve_tid(struct ieee80211_sta *sta, u8 tid);
* ieee80211_tx_dequeue - dequeue a packet from a software tx queue
*
* @hw: pointer as obtained from ieee80211_alloc_hw()
- * @txq: pointer obtained from station or virtual interface
+ * @txq: pointer obtained from ieee80211_next_txq()
*
* Returns the skb if successful, %NULL if no frame was available.
*/
struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
struct ieee80211_txq *txq);
+/**
+ * ieee80211_schedule_txq - add txq to scheduling loop
+ *
+ * @hw: pointer as obtained from ieee80211_alloc_hw()
+ * @txq: pointer obtained from station or virtual interface
+ *
+ * Returns %true if the txq was actually added to the scheduling,
+ * %false otherwise.
+ */
+bool ieee80211_schedule_txq(struct ieee80211_hw *hw,
+ struct ieee80211_txq *txq);
+
+/**
+ * ieee80211_next_txq - get next tx queue to pull packets from
+ *
+ * @hw: pointer as obtained from ieee80211_alloc_hw()
+ *
+ * Returns the next txq if successful, %NULL if no queue is eligible. If a txq
+ * is returned, it will have been removed from the scheduler queue and needs to
+ * be re-scheduled with ieee80211_schedule_txq() to continue to be active.
+ */
+struct ieee80211_txq *ieee80211_next_txq(struct ieee80211_hw *hw);
+
/**
* ieee80211_txq_get_depth - get pending frame/byte count of given txq
*
diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c
index bef516ec47f9..569b5b5c6d70 100644
--- a/net/mac80211/agg-tx.c
+++ b/net/mac80211/agg-tx.c
@@ -226,9 +226,13 @@ ieee80211_agg_start_txq(struct sta_info *sta, int tid, bool enable)
clear_bit(IEEE80211_TXQ_AMPDU, &txqi->flags);
clear_bit(IEEE80211_TXQ_STOP, &txqi->flags);
+
+ if (!ieee80211_schedule_txq(&sta->sdata->local->hw, txq))
+ return;
+
local_bh_disable();
rcu_read_lock();
- drv_wake_tx_queue(sta->sdata->local, txqi);
+ drv_wake_tx_queue(sta->sdata->local);
rcu_read_unlock();
local_bh_enable();
}
diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
index 09f77e4a8a79..e20a9e2acd53 100644
--- a/net/mac80211/driver-ops.h
+++ b/net/mac80211/driver-ops.h
@@ -1157,16 +1157,10 @@ drv_tdls_recv_channel_switch(struct ieee80211_local *local,
trace_drv_return_void(local);
}
-static inline void drv_wake_tx_queue(struct ieee80211_local *local,
- struct txq_info *txq)
+static inline void drv_wake_tx_queue(struct ieee80211_local *local)
{
- struct ieee80211_sub_if_data *sdata = vif_to_sdata(txq->txq.vif);
-
- if (!check_sdata_in_driver(sdata))
- return;
-
- trace_drv_wake_tx_queue(local, sdata, txq);
- local->ops->wake_tx_queue(&local->hw, &txq->txq);
+ trace_drv_wake_tx_queue(local);
+ local->ops->wake_tx_queue(&local->hw);
}
static inline int drv_start_nan(struct ieee80211_local *local,
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 9675814f64db..95be548b1d4f 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -832,6 +832,7 @@ struct txq_info {
struct codel_vars def_cvars;
struct codel_stats cstats;
struct sk_buff_head frags;
+ struct list_head schedule_order;
unsigned long flags;
/* keep last! */
@@ -1121,6 +1122,10 @@ struct ieee80211_local {
struct codel_vars *cvars;
struct codel_params cparams;
+ /* protects active_txqs and txqi->schedule_order */
+ spinlock_t active_txq_lock;
+ struct list_head active_txqs;
+
const struct ieee80211_ops *ops;
/*
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 8aa1f5b6a051..9ad0556aa24b 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -616,6 +616,9 @@ struct ieee80211_hw *ieee80211_alloc_hw_nm(size_t priv_data_len,
spin_lock_init(&local->rx_path_lock);
spin_lock_init(&local->queue_stop_reason_lock);
+ INIT_LIST_HEAD(&local->active_txqs);
+ spin_lock_init(&local->active_txq_lock);
+
INIT_LIST_HEAD(&local->chanctx_list);
mutex_init(&local->chanctx_mtx);
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 9673e157bf8f..da3737d4778f 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -1246,12 +1246,17 @@ void ieee80211_sta_ps_deliver_wakeup(struct sta_info *sta)
drv_sta_notify(local, sdata, STA_NOTIFY_AWAKE, &sta->sta);
if (sta->sta.txq[0]) {
+ bool wake = false;
+
for (i = 0; i < ARRAY_SIZE(sta->sta.txq); i++) {
if (!txq_has_queue(sta->sta.txq[i]))
continue;
- drv_wake_tx_queue(local, to_txq_info(sta->sta.txq[i]));
+ if (ieee80211_schedule_txq(&local->hw, sta->sta.txq[i]))
+ wake = true;
}
+ if (wake)
+ drv_wake_tx_queue(local);
}
skb_queue_head_init(&pending);
diff --git a/net/mac80211/trace.h b/net/mac80211/trace.h
index 3d9ac17af407..f51eade947ee 100644
--- a/net/mac80211/trace.h
+++ b/net/mac80211/trace.h
@@ -2549,35 +2549,9 @@ TRACE_EVENT(drv_tdls_recv_channel_switch,
)
);
-TRACE_EVENT(drv_wake_tx_queue,
- TP_PROTO(struct ieee80211_local *local,
- struct ieee80211_sub_if_data *sdata,
- struct txq_info *txq),
-
- TP_ARGS(local, sdata, txq),
-
- TP_STRUCT__entry(
- LOCAL_ENTRY
- VIF_ENTRY
- STA_ENTRY
- __field(u8, ac)
- __field(u8, tid)
- ),
-
- TP_fast_assign(
- struct ieee80211_sta *sta = txq->txq.sta;
-
- LOCAL_ASSIGN;
- VIF_ASSIGN;
- STA_ASSIGN;
- __entry->ac = txq->txq.ac;
- __entry->tid = txq->txq.tid;
- ),
-
- TP_printk(
- LOCAL_PR_FMT VIF_PR_FMT STA_PR_FMT " ac:%d tid:%d",
- LOCAL_PR_ARG, VIF_PR_ARG, STA_PR_ARG, __entry->ac, __entry->tid
- )
+DEFINE_EVENT(local_only_evt, drv_wake_tx_queue,
+ TP_PROTO(struct ieee80211_local *local),
+ TP_ARGS(local)
);
#endif /* !__MAC80211_DRIVER_TRACE || TRACE_HEADER_MULTI_READ */
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 94826680cf2b..824d87e6d3eb 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1405,6 +1405,7 @@ void ieee80211_txq_init(struct ieee80211_sub_if_data *sdata,
codel_vars_init(&txqi->def_cvars);
codel_stats_init(&txqi->cstats);
__skb_queue_head_init(&txqi->frags);
+ INIT_LIST_HEAD(&txqi->schedule_order);
txqi->txq.vif = &sdata->vif;
@@ -1428,6 +1429,7 @@ void ieee80211_txq_purge(struct ieee80211_local *local,
fq_tin_reset(fq, tin, fq_skb_free_func);
ieee80211_purge_tx_queue(&local->hw, &txqi->frags);
+ list_del_init(&txqi->schedule_order);
}
int ieee80211_txq_setup_flows(struct ieee80211_local *local)
@@ -1524,7 +1526,8 @@ static bool ieee80211_queue_skb(struct ieee80211_local *local,
ieee80211_txq_enqueue(local, txqi, skb);
spin_unlock_bh(&fq->lock);
- drv_wake_tx_queue(local, txqi);
+ if (ieee80211_schedule_txq(&local->hw, &txqi->txq))
+ drv_wake_tx_queue(local);
return true;
}
@@ -3517,6 +3520,50 @@ struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
}
EXPORT_SYMBOL(ieee80211_tx_dequeue);
+bool ieee80211_schedule_txq(struct ieee80211_hw *hw,
+ struct ieee80211_txq *txq)
+{
+ struct ieee80211_local *local = hw_to_local(hw);
+ struct txq_info *txqi = to_txq_info(txq);
+ bool ret = false;
+
+ spin_lock_bh(&local->active_txq_lock);
+
+ if (list_empty(&txqi->schedule_order)) {
+ list_add_tail(&txqi->schedule_order, &local->active_txqs);
+ ret = true;
+ }
+
+ spin_unlock_bh(&local->active_txq_lock);
+
+ return ret;
+}
+EXPORT_SYMBOL(ieee80211_schedule_txq);
+
+struct ieee80211_txq *ieee80211_next_txq(struct ieee80211_hw *hw)
+{
+ struct ieee80211_local *local = hw_to_local(hw);
+ struct txq_info *txqi = NULL;
+
+ spin_lock_bh(&local->active_txq_lock);
+
+ if (list_empty(&local->active_txqs))
+ goto out;
+
+ txqi = list_first_entry(&local->active_txqs,
+ struct txq_info, schedule_order);
+ list_del_init(&txqi->schedule_order);
+
+out:
+ spin_unlock_bh(&local->active_txq_lock);
+
+ if (!txqi)
+ return NULL;
+
+ return &txqi->txq;
+}
+EXPORT_SYMBOL(ieee80211_next_txq);
+
void __ieee80211_subif_start_xmit(struct sk_buff *skb,
struct net_device *dev,
u32 info_flags)
--
2.14.3
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 2/3] mac80211: Add airtime account and scheduling to TXQs
2017-10-27 14:14 ` [PATCH v2 1/3] " Toke Høiland-Jørgensen
` (2 preceding siblings ...)
2017-10-31 11:27 ` [PATCH v3 " Toke Høiland-Jørgensen
@ 2017-10-31 11:27 ` Toke Høiland-Jørgensen
2017-10-31 11:27 ` [PATCH v3 3/3] ath9k: Switch to mac80211 airtime API Toke Høiland-Jørgensen
4 siblings, 0 replies; 20+ messages in thread
From: Toke Høiland-Jørgensen @ 2017-10-31 11:27 UTC (permalink / raw)
To: make-wifi-fast, linux-wireless; +Cc: Toke Høiland-Jørgensen
This adds airtime accounting and scheduling to the mac80211 TXQ
scheduler. A new hardware flag, AIRTIME_ACCOUNTING, is added that
drivers can set if they support reporting airtime usage of
transmissions. When this flag is set, mac80211 will expect the actual
airtime usage to be reported in the tx_time and rx_time fields of the
respective status structs.
When airtime information is present, mac80211 will schedule TXQs
(through ieee80211_next_txq()) in a way that enforces airtime fairness
between active stations. This scheduling works the same way as the ath9k
in-driver airtime fairness scheduling.
Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
---
include/net/mac80211.h | 24 ++++++++++++++++++++++++
net/mac80211/debugfs.c | 1 +
net/mac80211/debugfs_sta.c | 29 +++++++++++++++++++++++++++++
net/mac80211/ieee80211_i.h | 9 +++++++--
net/mac80211/main.c | 3 ++-
net/mac80211/rx.c | 8 ++++++++
net/mac80211/sta_info.c | 2 ++
net/mac80211/sta_info.h | 7 +++++++
net/mac80211/status.c | 16 ++++++++++++++++
net/mac80211/tx.c | 31 ++++++++++++++++++++++++++-----
10 files changed, 122 insertions(+), 8 deletions(-)
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 715f45501aff..453653d082af 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1188,6 +1188,8 @@ enum mac80211_rx_encoding {
* HT or VHT is used (%RX_FLAG_HT/%RX_FLAG_VHT)
* @nss: number of streams (VHT and HE only)
* @flag: %RX_FLAG_\*
+ * @airtime: Duration of frame in usec. See @IEEE80211_HW_AIRTIME_ACCOUNTING for
+ * how to use this.
* @encoding: &enum mac80211_rx_encoding
* @bw: &enum rate_info_bw
* @enc_flags: uses bits from &enum mac80211_rx_encoding_flags
@@ -1202,6 +1204,7 @@ struct ieee80211_rx_status {
u32 device_timestamp;
u32 ampdu_reference;
u32 flag;
+ u16 airtime;
u16 freq;
u8 enc_flags;
u8 encoding:2, bw:3;
@@ -2059,6 +2062,26 @@ struct ieee80211_txq {
* The stack will not do fragmentation.
* The callback for @set_frag_threshold should be set as well.
*
+ * @IEEE80211_HW_AIRTIME_ACCOUNTING: Hardware supports accounting the airtime
+ * usage of other stations and reports it in the @tx_time and/or @airtime
+ * fields of the TX/RX status structs.
+ * When setting this flag, the driver should ensure that the respective
+ * fields in the TX and RX status structs are always either zero or
+ * contains a valid duration for the frame in usec. The driver can choose
+ * to report either or both of TX and RX airtime, but it is recommended to
+ * report both.
+ * The reported airtime should as a minimum include all time that is spent
+ * transmitting to the remote station, including overhead and padding, but
+ * not including time spent waiting for a TXOP. If the time is not reported
+ * by the hardware it can in some cases be calculated from the rate and
+ * known frame composition. When possible, the time should include any
+ * failed transmission attempts.
+ * For aggregated frames, there are two possible strategies to report the
+ * airtime: Either include the airtime of the entire aggregate in the first
+ * (or last) frame and leave the others at zero. Alternatively, include the
+ * overhead of the full aggregate in the first or last frame and report the
+ * time of each frame + padding not including the full aggregate overhead.
+ *
* @NUM_IEEE80211_HW_FLAGS: number of hardware flags, used for sizing arrays
*/
enum ieee80211_hw_flags {
@@ -2101,6 +2124,7 @@ enum ieee80211_hw_flags {
IEEE80211_HW_TX_FRAG_LIST,
IEEE80211_HW_REPORTS_LOW_ACK,
IEEE80211_HW_SUPPORTS_TX_FRAG,
+ IEEE80211_HW_AIRTIME_ACCOUNTING,
/* keep last, obviously */
NUM_IEEE80211_HW_FLAGS
diff --git a/net/mac80211/debugfs.c b/net/mac80211/debugfs.c
index 5fae001f286c..20e3aa0e8464 100644
--- a/net/mac80211/debugfs.c
+++ b/net/mac80211/debugfs.c
@@ -211,6 +211,7 @@ static const char *hw_flag_names[] = {
FLAG(TX_FRAG_LIST),
FLAG(REPORTS_LOW_ACK),
FLAG(SUPPORTS_TX_FRAG),
+ FLAG(AIRTIME_ACCOUNTING),
#undef FLAG
};
diff --git a/net/mac80211/debugfs_sta.c b/net/mac80211/debugfs_sta.c
index b15412c21ac9..40dba446836f 100644
--- a/net/mac80211/debugfs_sta.c
+++ b/net/mac80211/debugfs_sta.c
@@ -188,6 +188,32 @@ static ssize_t sta_aqm_read(struct file *file, char __user *userbuf,
}
STA_OPS(aqm);
+static ssize_t sta_airtime_read(struct file *file, char __user *userbuf,
+ size_t count, loff_t *ppos)
+{
+ struct sta_info *sta = file->private_data;
+ size_t bufsz = 200;
+ char *buf = kzalloc(bufsz, GFP_KERNEL), *p = buf;
+ ssize_t rv;
+
+ if (!buf)
+ return -ENOMEM;
+
+ spin_lock_bh(&sta->lock);
+
+ p += scnprintf(p, bufsz + buf - p,
+ "RX: %llu us\nTX: %llu us\nDeficit: %lld us\n",
+ sta->airtime_stats.rx_airtime,
+ sta->airtime_stats.tx_airtime,
+ sta->airtime_deficit);
+
+ spin_unlock_bh(&sta->lock);
+ rv = simple_read_from_buffer(userbuf, count, ppos, buf, p - buf);
+ kfree(buf);
+ return rv;
+}
+STA_OPS(airtime);
+
static ssize_t sta_agg_status_read(struct file *file, char __user *userbuf,
size_t count, loff_t *ppos)
{
@@ -542,6 +568,9 @@ void ieee80211_sta_debugfs_add(struct sta_info *sta)
if (local->ops->wake_tx_queue)
DEBUGFS_ADD(aqm);
+ if (ieee80211_hw_check(&local->hw, AIRTIME_ACCOUNTING))
+ DEBUGFS_ADD(airtime);
+
if (sizeof(sta->driver_buffered_tids) == sizeof(u32))
debugfs_create_x32("driver_buffered_tids", 0400,
sta->debugfs_dir,
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 95be548b1d4f..e74aeff7662a 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -90,6 +90,9 @@ extern const u8 ieee80211_ac_to_qos_mask[IEEE80211_NUM_ACS];
#define IEEE80211_MAX_NAN_INSTANCE_ID 255
+/* How much to increase airtime deficit on each scheduling round */
+#define IEEE80211_AIRTIME_QUANTUM 1000 /* usec */
+
struct ieee80211_fragment_entry {
struct sk_buff_head skb_list;
unsigned long first_frag_time;
@@ -1122,9 +1125,11 @@ struct ieee80211_local {
struct codel_vars *cvars;
struct codel_params cparams;
- /* protects active_txqs and txqi->schedule_order */
+ /* protects active_txqs_{new,old} and txqi->schedule_order */
spinlock_t active_txq_lock;
- struct list_head active_txqs;
+ struct list_head active_txqs_new;
+ struct list_head active_txqs_old;
+
const struct ieee80211_ops *ops;
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 9ad0556aa24b..acf73c398d9b 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -616,7 +616,8 @@ struct ieee80211_hw *ieee80211_alloc_hw_nm(size_t priv_data_len,
spin_lock_init(&local->rx_path_lock);
spin_lock_init(&local->queue_stop_reason_lock);
- INIT_LIST_HEAD(&local->active_txqs);
+ INIT_LIST_HEAD(&local->active_txqs_new);
+ INIT_LIST_HEAD(&local->active_txqs_old);
spin_lock_init(&local->active_txq_lock);
INIT_LIST_HEAD(&local->chanctx_list);
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 70e9d2ca8bbe..a6166850b971 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -1637,6 +1637,14 @@ ieee80211_rx_h_sta_process(struct ieee80211_rx_data *rx)
if (ieee80211_vif_is_mesh(&rx->sdata->vif))
ieee80211_mps_rx_h_sta_process(sta, hdr);
+ /* airtime accounting */
+ if (status->airtime) {
+ spin_lock_bh(&sta->lock);
+ sta->airtime_stats.rx_airtime += status->airtime;
+ sta->airtime_deficit -= status->airtime;
+ spin_unlock_bh(&sta->lock);
+ }
+
/*
* Drop (qos-)data::nullfunc frames silently, since they
* are used only to control station power saving mode.
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index da3737d4778f..e8511922a7c7 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -433,6 +433,8 @@ struct sta_info *sta_info_alloc(struct ieee80211_sub_if_data *sdata,
sta->cparams.interval = MS2TIME(100);
sta->cparams.ecn = true;
+ sta->airtime_deficit = IEEE80211_AIRTIME_QUANTUM;
+
sta_dbg(sdata, "Allocated STA %pM\n", sta->sta.addr);
return sta;
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 5c54acd10562..5d1802f22550 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -550,6 +550,13 @@ struct sta_info {
} tx_stats;
u16 tid_seq[IEEE80211_QOS_CTL_TID_MASK + 1];
+ /* Airtime stats and deficit, protected by lock */
+ struct {
+ u64 rx_airtime;
+ u64 tx_airtime;
+ } airtime_stats;
+ s64 airtime_deficit;
+
/*
* Aggregation information, locked with lock.
*/
diff --git a/net/mac80211/status.c b/net/mac80211/status.c
index da7427a41529..cf7956f7a040 100644
--- a/net/mac80211/status.c
+++ b/net/mac80211/status.c
@@ -823,6 +823,14 @@ static void __ieee80211_tx_status(struct ieee80211_hw *hw,
ieee80211_lost_packet(sta, info);
}
}
+
+ if (ieee80211_hw_check(&local->hw, AIRTIME_ACCOUNTING) &&
+ info->status.tx_time) {
+ spin_lock_bh(&sta->lock);
+ sta->airtime_stats.tx_airtime += info->status.tx_time;
+ sta->airtime_deficit -= info->status.tx_time;
+ spin_unlock_bh(&sta->lock);
+ }
}
/* SNMP counters
@@ -947,6 +955,14 @@ void ieee80211_tx_status_ext(struct ieee80211_hw *hw,
sta->status_stats.retry_failed++;
sta->status_stats.retry_count += retry_count;
+ if (ieee80211_hw_check(&local->hw, AIRTIME_ACCOUNTING) &&
+ info->status.tx_time) {
+ spin_lock_bh(&sta->lock);
+ sta->airtime_stats.tx_airtime += info->status.tx_time;
+ sta->airtime_deficit -= info->status.tx_time;
+ spin_unlock_bh(&sta->lock);
+ }
+
if (acked) {
sta->status_stats.last_ack = jiffies;
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 824d87e6d3eb..409e84509625 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -3530,7 +3530,7 @@ bool ieee80211_schedule_txq(struct ieee80211_hw *hw,
spin_lock_bh(&local->active_txq_lock);
if (list_empty(&txqi->schedule_order)) {
- list_add_tail(&txqi->schedule_order, &local->active_txqs);
+ list_add_tail(&txqi->schedule_order, &local->active_txqs_new);
ret = true;
}
@@ -3544,14 +3544,35 @@ struct ieee80211_txq *ieee80211_next_txq(struct ieee80211_hw *hw)
{
struct ieee80211_local *local = hw_to_local(hw);
struct txq_info *txqi = NULL;
+ struct list_head *head;
spin_lock_bh(&local->active_txq_lock);
- if (list_empty(&local->active_txqs))
- goto out;
+begin:
+ head = &local->active_txqs_new;
+ if (list_empty(head)) {
+ head = &local->active_txqs_old;
+ if (list_empty(head))
+ goto out;
+ }
+
+ txqi = list_first_entry(head, struct txq_info, schedule_order);
+
+ if (txqi->txq.sta) {
+ struct sta_info *sta = container_of(txqi->txq.sta,
+ struct sta_info, sta);
+
+ spin_lock_bh(&sta->lock);
+ if (sta->airtime_deficit < 0) {
+ sta->airtime_deficit += IEEE80211_AIRTIME_QUANTUM;
+ list_move_tail(&txqi->schedule_order,
+ &local->active_txqs_old);
+ spin_unlock_bh(&sta->lock);
+ goto begin;
+ }
+ spin_unlock_bh(&sta->lock);
+ }
- txqi = list_first_entry(&local->active_txqs,
- struct txq_info, schedule_order);
list_del_init(&txqi->schedule_order);
out:
--
2.14.3
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 3/3] ath9k: Switch to mac80211 airtime API
2017-10-27 14:14 ` [PATCH v2 1/3] " Toke Høiland-Jørgensen
` (3 preceding siblings ...)
2017-10-31 11:27 ` [PATCH v3 2/3] mac80211: Add airtime account and scheduling to TXQs Toke Høiland-Jørgensen
@ 2017-10-31 11:27 ` Toke Høiland-Jørgensen
4 siblings, 0 replies; 20+ messages in thread
From: Toke Høiland-Jørgensen @ 2017-10-31 11:27 UTC (permalink / raw)
To: make-wifi-fast, linux-wireless; +Cc: Toke Høiland-Jørgensen
This removes the in-driver airtime fairness scheduling from ath9k and
switches the driver to use the API introduced in mac80211 in the
previous commit.
Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
---
drivers/net/wireless/ath/ath9k/ath9k.h | 7 +---
drivers/net/wireless/ath/ath9k/debug.h | 8 -----
drivers/net/wireless/ath/ath9k/debug_sta.c | 54 ------------------------------
drivers/net/wireless/ath/ath9k/init.c | 4 +--
drivers/net/wireless/ath/ath9k/recv.c | 9 ++---
drivers/net/wireless/ath/ath9k/xmit.c | 21 ++++--------
6 files changed, 12 insertions(+), 91 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index a5088792b769..7c7dece6cb91 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -112,8 +112,6 @@ int ath_descdma_setup(struct ath_softc *sc, struct ath_descdma *dd,
#define ATH_TXFIFO_DEPTH 8
#define ATH_TX_ERROR 0x01
-#define ATH_AIRTIME_QUANTUM 300 /* usec */
-
/* Stop tx traffic 1ms before the GO goes away */
#define ATH_P2P_PS_STOP_TIME 1000
@@ -215,6 +213,7 @@ struct ath_buf_state {
bool stale;
u16 seqno;
unsigned long bfs_paprd_timestamp;
+ u32 airtime;
};
struct ath_buf {
@@ -259,12 +258,9 @@ struct ath_node {
bool sleeping;
bool no_ps_filter;
- s64 airtime_deficit[IEEE80211_NUM_ACS];
- 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];
@@ -983,7 +979,6 @@ void ath_ant_comb_scan(struct ath_softc *sc, struct ath_rx_status *rs);
#define AIRTIME_USE_TX BIT(0)
#define AIRTIME_USE_RX BIT(1)
-#define AIRTIME_USE_NEW_QUEUES BIT(2)
#define AIRTIME_ACTIVE(flags) (!!(flags & (AIRTIME_USE_TX|AIRTIME_USE_RX)))
struct ath_softc {
diff --git a/drivers/net/wireless/ath/ath9k/debug.h b/drivers/net/wireless/ath/ath9k/debug.h
index 249f8141cd00..559d9628f280 100644
--- a/drivers/net/wireless/ath/ath9k/debug.h
+++ b/drivers/net/wireless/ath/ath9k/debug.h
@@ -319,20 +319,12 @@ 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_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_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 efc692ee67d4..89ba95287a8b 100644
--- a/drivers/net/wireless/ath/ath9k/debug_sta.c
+++ b/drivers/net/wireless/ath/ath9k/debug_sta.c
@@ -242,59 +242,6 @@ 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;
- static const char *qname[4] = {
- "VO", "VI", "BE", "BK"
- };
- u32 len = 0, size = 256;
- char *buf;
- size_t retval;
- int i;
-
- 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: ");
- for (i = 0; i < 4; i++)
- len += scnprintf(buf+len, size - len, "%s: %lld us ", qname[i], an->airtime_deficit[i]);
- if (len < size)
- buf[len++] = '\n';
-
- 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,
@@ -304,5 +251,4 @@ 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 bb7936090b91..b4c6fc49b19e 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -620,8 +620,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 |
- AIRTIME_USE_NEW_QUEUES);
+ sc->airtime_flags = (AIRTIME_USE_TX | AIRTIME_USE_RX);
sc->sc_ah = ah;
sc->dfs_detector = dfs_pattern_detector_init(common, NL80211_DFS_UNSET);
@@ -873,6 +872,7 @@ static void ath9k_set_hw_capab(struct ath_softc *sc, struct ieee80211_hw *hw)
ieee80211_hw_set(hw, HOST_BROADCAST_PS_BUFFERING);
ieee80211_hw_set(hw, SUPPORT_FAST_XMIT);
ieee80211_hw_set(hw, SUPPORTS_CLONED_SKBS);
+ ieee80211_hw_set(hw, AIRTIME_ACCOUNTING);
if (ath9k_ps_enable)
ieee80211_hw_set(hw, SUPPORTS_PS);
diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
index a768e841524d..69a4ba5d4013 100644
--- a/drivers/net/wireless/ath/ath9k/recv.c
+++ b/drivers/net/wireless/ath/ath9k/recv.c
@@ -1054,12 +1054,9 @@ static void ath_rx_count_airtime(struct ath_softc *sc,
len, rxs->rate_idx, is_sp);
}
- if (!!(sc->airtime_flags & AIRTIME_USE_RX)) {
- spin_lock_bh(&acq->lock);
- an->airtime_deficit[acno] -= airtime;
- spin_unlock_bh(&acq->lock);
- }
- ath_debug_airtime(sc, an, airtime, 0);
+ if (!!(sc->airtime_flags & AIRTIME_USE_RX))
+ rxs->airtime = airtime;
+
exit:
rcu_read_unlock();
}
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index bd438062a6db..5ffd44a4b3de 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -640,11 +640,9 @@ 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_node *an,
- struct ath_atx_tid *tid, struct ath_buf *bf,
+static void ath_tx_count_airtime(struct ath_softc *sc, struct ath_buf *bf,
struct ath_tx_status *ts)
{
- struct ath_txq *txq = tid->txq;
u32 airtime = 0;
int i;
@@ -654,15 +652,9 @@ static void ath_tx_count_airtime(struct ath_softc *sc, struct ath_node *an,
airtime += rate_dur * bf->rates[i].count;
}
- if (sc->airtime_flags & AIRTIME_USE_TX) {
- int q = txq->mac80211_qnum;
- struct ath_acq *acq = &sc->cur_chan->acq[q];
+ if (sc->airtime_flags & AIRTIME_USE_TX)
+ bf->bf_state.airtime = airtime;
- spin_lock_bh(&acq->lock);
- an->airtime_deficit[q] -= airtime;
- spin_unlock_bh(&acq->lock);
- }
- ath_debug_airtime(sc, an, 0, airtime);
}
static void ath_tx_process_buffer(struct ath_softc *sc, struct ath_txq *txq,
@@ -692,7 +684,7 @@ static void ath_tx_process_buffer(struct ath_softc *sc, struct ath_txq *txq,
if (sta) {
struct ath_node *an = (struct ath_node *)sta->drv_priv;
tid = ath_get_skb_tid(sc, an, bf->bf_mpdu);
- ath_tx_count_airtime(sc, an, tid, bf, ts);
+ ath_tx_count_airtime(sc, bf, ts);
if (ts->ts_status & (ATH9K_TXERR_FILT | ATH9K_TXERR_XRETRY))
tid->clear_ps_filter = true;
}
@@ -2428,6 +2420,8 @@ static void ath_tx_complete_buf(struct ath_softc *sc, struct ath_buf *bf,
if (ts->ts_status & ATH9K_TXERR_FILT)
tx_info->flags |= IEEE80211_TX_STAT_TX_FILTERED;
+ tx_info->status.tx_time = min_t(u32, 0xffff, bf->bf_state.airtime);
+
dma_unmap_single(sc->dev, bf->bf_buf_addr, skb->len, DMA_TO_DEVICE);
bf->bf_buf_addr = 0;
if (sc->tx99_state)
@@ -2754,9 +2748,6 @@ void ath_tx_node_init(struct ath_softc *sc, struct ath_node *an)
struct ath_atx_tid *tid;
int tidno, acno;
- for (acno = 0; acno < IEEE80211_NUM_ACS; acno++)
- an->airtime_deficit[acno] = ATH_AIRTIME_QUANTUM;
-
for (tidno = 0; tidno < IEEE80211_NUM_TIDS; tidno++) {
tid = ath_node_to_tid(an, tidno);
tid->an = an;
--
2.14.3
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v3 1/3] mac80211: Add TXQ scheduling API
2017-10-31 11:27 ` [PATCH v3 " Toke Høiland-Jørgensen
@ 2017-12-14 11:33 ` Felix Fietkau
2017-12-14 12:15 ` Toke Høiland-Jørgensen
0 siblings, 1 reply; 20+ messages in thread
From: Felix Fietkau @ 2017-12-14 11:33 UTC (permalink / raw)
To: Toke Høiland-Jørgensen, make-wifi-fast, linux-wireless
Cc: Johannes Berg
On 2017-10-31 12:27, Toke Høiland-Jørgensen wrote:
> This adds an API to mac80211 to handle scheduling of TXQs and changes the
> interface between driver and mac80211 for TXQ handling as follows:
>
> - The wake_tx_queue callback interface no longer includes the TXQ. Instead,
> the driver is expected to retrieve that from ieee80211_next_txq()
>
> - Two new mac80211 functions are added: ieee80211_next_txq() and
> ieee80211_schedule_txq(). The former returns the next TXQ that should be
> scheduled, and is how the driver gets a queue to pull packets from. The
> latter is called internally by mac80211 to start scheduling a queue, and
> the driver is supposed to call it to re-schedule the TXQ after it is
> finished pulling packets from it (unless the queue emptied).
>
> The ath9k and ath10k drivers are changed to use the new API.
>
> Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
Sorry that I didn't have time to give this a thorough review earlier,
since I was pretty busy with other projects. Now that I'm working on
porting mt76 to this new API, some things in this patch strike me as
rather odd, and there might be some bugs and nasty limitations here:
In the new API you can no longer select txq entries by hardware queue.
When using multiple WMM queues, this could lead to station entries being
requeued unnecessarily (because there is no room in the hw queue for the
txq entry that ieee80211_next_txq happens to return).
Since ieee80211_next_txq also refills the airtime fairness quantum, this
might lead to some minor fairness issues.
In ath9k the code used to have a loop that goes through all pending txq
entries until it has filled the hw queues again. You replaced that with
some calls to ath_txq_schedule which now only considers one single txq.
There are several reasons why this queue could potentially not be serviced:
- ieee80211_tx_dequeue returned no frame
- frame does not fit within BA window
- txq was for another queue which is already filled
Depending on the exact circumstances with enough stations this might
lead to hardware queues getting starved.
In ath10k the issues are different. You left a loop to service queues in
place, and if I'm reading it correctly, you could be facing an infinite
loop here:
Let's assume that ath10k_mac_tx_push_pending gets called and there are
multiple txqs pending.
The first txq has 16 frames pending, gets serviced, requeued.
Second txq has lots of frames pending, 16 frames are pushed out, the txq
is requeued.
Back to the first one, ath10k_mac_tx_push_txq returns -ENOENT, so no
requeue.
Back to the second one, hardware queues are filled,
ath10k_mac_tx_can_push returns false, so ret stays 0, txq gets requeued.
By now first == txq can never happen anymore because the first txq is
not scheduled anymore.
Seems like an infinite loop to me.
I think dealing with these corner cases will be easier if we support
filtering by queue in ieee80211_next_txq, so I will send a patch for that.
- Felix
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 1/3] mac80211: Add TXQ scheduling API
2017-12-14 11:33 ` Felix Fietkau
@ 2017-12-14 12:15 ` Toke Høiland-Jørgensen
2017-12-14 12:44 ` Felix Fietkau
0 siblings, 1 reply; 20+ messages in thread
From: Toke Høiland-Jørgensen @ 2017-12-14 12:15 UTC (permalink / raw)
To: Felix Fietkau, make-wifi-fast, linux-wireless; +Cc: Johannes Berg
Felix Fietkau <nbd@nbd.name> writes:
> On 2017-10-31 12:27, Toke H=C3=B8iland-J=C3=B8rgensen wrote:
>> This adds an API to mac80211 to handle scheduling of TXQs and changes the
>> interface between driver and mac80211 for TXQ handling as follows:
>>=20
>> - The wake_tx_queue callback interface no longer includes the TXQ. Inste=
ad,
>> the driver is expected to retrieve that from ieee80211_next_txq()
>>=20
>> - Two new mac80211 functions are added: ieee80211_next_txq() and
>> ieee80211_schedule_txq(). The former returns the next TXQ that should =
be
>> scheduled, and is how the driver gets a queue to pull packets from. The
>> latter is called internally by mac80211 to start scheduling a queue, a=
nd
>> the driver is supposed to call it to re-schedule the TXQ after it is
>> finished pulling packets from it (unless the queue emptied).
>>=20
>> The ath9k and ath10k drivers are changed to use the new API.
>>=20
>> Signed-off-by: Toke H=C3=B8iland-J=C3=B8rgensen <toke@toke.dk>
> Sorry that I didn't have time to give this a thorough review earlier,
> since I was pretty busy with other projects. Now that I'm working on
> porting mt76 to this new API, some things in this patch strike me as
> rather odd, and there might be some bugs and nasty limitations here:
>
> In the new API you can no longer select txq entries by hardware queue.
> When using multiple WMM queues, this could lead to station entries being
> requeued unnecessarily (because there is no room in the hw queue for the
> txq entry that ieee80211_next_txq happens to return).
Yeah, there's some tension between enforcing fairness (which is a per
station thing) and the WMM queueing stuff (which gives priority based on
WMM levels and ignores stations). There are basically two ways to
resolve this: Prioritising fairness or prioritising WMM levels. In the
former case, we first select which station to transmit to, and then
select the highest WMM priority level queued *for that station* (the
last part of this is missing from the code as it is now). In the latter
case, we keep scheduling per-WMM, then enforce fairness within that.
The former case has the potential to lead to starved hardware queues,
while the latter leads to unfairness. We had a bit of discussion of
which is better at netdev, but did not resolve it. Personally, I think
prioritising fairness is better, but I'm willing to be convinced
otherwise by data :). So my plan is to implement that fully and try it
out, then evaluate based on actual experiments...
> Since ieee80211_next_txq also refills the airtime fairness quantum, this
> might lead to some minor fairness issues.
I'm planning to change the way the scheduler works anyway, so this issue
should go away. Haven't had time to do that yet, unfortunately.
> In ath9k the code used to have a loop that goes through all pending txq
> entries until it has filled the hw queues again. You replaced that with
> some calls to ath_txq_schedule which now only considers one single txq.
> There are several reasons why this queue could potentially not be service=
d:
> - ieee80211_tx_dequeue returned no frame
> - frame does not fit within BA window
> - txq was for another queue which is already filled
> Depending on the exact circumstances with enough stations this might
> lead to hardware queues getting starved.
Well, that loop was already removed when I implemented the in-driver
fairness scheduler. We can't really avoid those cases entirely if we
want to enforce fairness (the BAW case in particular; if the only
eligible station from an airtime PoW has a full BAW, you have to
throttle and can potentially starve hwqs). However, don't think this
happens much in practice (or we would have seen performance regressions
by now).
The 'txq was for another queue' case is new with this patch, but that
goes back to the WMM/fairness tention above.
> In ath10k the issues are different. You left a loop to service queues in
> place, and if I'm reading it correctly, you could be facing an infinite
> loop here:
> Let's assume that ath10k_mac_tx_push_pending gets called and there are
> multiple txqs pending.
> The first txq has 16 frames pending, gets serviced, requeued.
> Second txq has lots of frames pending, 16 frames are pushed out, the txq
> is requeued.
> Back to the first one, ath10k_mac_tx_push_txq returns -ENOENT, so no
> requeue.
> Back to the second one, hardware queues are filled,
> ath10k_mac_tx_can_push returns false, so ret stays 0, txq gets requeued.
> By now first =3D=3D txq can never happen anymore because the first txq is
> not scheduled anymore.
> Seems like an infinite loop to me.
Hmm, you may be right here. I don't have the hardware to test this on
ath10k :/
> I think dealing with these corner cases will be easier if we support
> filtering by queue in ieee80211_next_txq, so I will send a patch for
> that.
Not necessarily opposed to a flag (or however you end up implementing
this). We may have to ignore it if we want to prioritise fairness, as
noted above. However, having the flag could potentially make this user
configurable. I guess I'll have a look at your patch :)
-Toke
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 1/3] mac80211: Add TXQ scheduling API
2017-12-14 12:15 ` Toke Høiland-Jørgensen
@ 2017-12-14 12:44 ` Felix Fietkau
2017-12-14 14:34 ` Toke Høiland-Jørgensen
2017-12-19 9:44 ` Johannes Berg
0 siblings, 2 replies; 20+ messages in thread
From: Felix Fietkau @ 2017-12-14 12:44 UTC (permalink / raw)
To: Toke Høiland-Jørgensen, make-wifi-fast, linux-wireless
Cc: Johannes Berg
On 2017-12-14 13:15, Toke Høiland-Jørgensen wrote:
> Felix Fietkau <nbd@nbd.name> writes:
>
>> On 2017-10-31 12:27, Toke Høiland-Jørgensen wrote:
>>> This adds an API to mac80211 to handle scheduling of TXQs and changes the
>>> interface between driver and mac80211 for TXQ handling as follows:
>>>
>>> - The wake_tx_queue callback interface no longer includes the TXQ. Instead,
>>> the driver is expected to retrieve that from ieee80211_next_txq()
>>>
>>> - Two new mac80211 functions are added: ieee80211_next_txq() and
>>> ieee80211_schedule_txq(). The former returns the next TXQ that should be
>>> scheduled, and is how the driver gets a queue to pull packets from. The
>>> latter is called internally by mac80211 to start scheduling a queue, and
>>> the driver is supposed to call it to re-schedule the TXQ after it is
>>> finished pulling packets from it (unless the queue emptied).
>>>
>>> The ath9k and ath10k drivers are changed to use the new API.
>>>
>>> Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
>> Sorry that I didn't have time to give this a thorough review earlier,
>> since I was pretty busy with other projects. Now that I'm working on
>> porting mt76 to this new API, some things in this patch strike me as
>> rather odd, and there might be some bugs and nasty limitations here:
>>
>> In the new API you can no longer select txq entries by hardware queue.
>> When using multiple WMM queues, this could lead to station entries being
>> requeued unnecessarily (because there is no room in the hw queue for the
>> txq entry that ieee80211_next_txq happens to return).
>
> Yeah, there's some tension between enforcing fairness (which is a per
> station thing) and the WMM queueing stuff (which gives priority based on
> WMM levels and ignores stations). There are basically two ways to
> resolve this: Prioritising fairness or prioritising WMM levels. In the
> former case, we first select which station to transmit to, and then
> select the highest WMM priority level queued *for that station* (the
> last part of this is missing from the code as it is now). In the latter
> case, we keep scheduling per-WMM, then enforce fairness within that.
>
> The former case has the potential to lead to starved hardware queues,
> while the latter leads to unfairness. We had a bit of discussion of
> which is better at netdev, but did not resolve it. Personally, I think
> prioritising fairness is better, but I'm willing to be convinced
> otherwise by data :). So my plan is to implement that fully and try it
> out, then evaluate based on actual experiments...
I don't really see how the approach taken in the current code is
actually prioritising fairness by starving hardware queues, since any
txq that can't be serviced in the current call because of queue depth
will be requeued. So based on the traffic pattern this might actually
lead to *more* unfairness.
>> Since ieee80211_next_txq also refills the airtime fairness quantum, this
>> might lead to some minor fairness issues.
>
> I'm planning to change the way the scheduler works anyway, so this issue
> should go away. Haven't had time to do that yet, unfortunately.Well, the problem is that the new code inside ath9k is a lot harder to
verify for correct behavior (regarding the queue starvation issue), and
I would really like to avoid nasty regressions that will be a nightmare
to debug.
>> In ath9k the code used to have a loop that goes through all pending txq
>> entries until it has filled the hw queues again. You replaced that with
>> some calls to ath_txq_schedule which now only considers one single txq.
>> There are several reasons why this queue could potentially not be serviced:
>> - ieee80211_tx_dequeue returned no frame
>> - frame does not fit within BA window
>> - txq was for another queue which is already filled
>> Depending on the exact circumstances with enough stations this might
>> lead to hardware queues getting starved.
>
> Well, that loop was already removed when I implemented the in-driver
> fairness scheduler.
Now that's not true. I'm looking at the diff for "mac80211: Add TXQ
scheduling API", and it removes these lines:
- /*
- * If we succeed in scheduling something, immediately restart to
make
- * sure we keep the HW busy.
- */
- if(ath_tx_sched_aggr(sc, txq, tid)) {
- if (!active) {
- spin_lock_bh(&acq->lock);
- list_move_tail(&tid->list, &acq->acq_old);
- spin_unlock_bh(&acq->lock);
- }
- goto begin;
- }
> We can't really avoid those cases entirely if we
> want to enforce fairness (the BAW case in particular; if the only
> eligible station from an airtime PoW has a full BAW, you have to
> throttle and can potentially starve hwqs). However, don't think this
> happens much in practice (or we would have seen performance regressions
> by now).I think so far you simply haven't had enough users hammering on the new
code yet.
> The 'txq was for another queue' case is new with this patch, but that
> goes back to the WMM/fairness tention above.
I agree that this is something we need to figure out. One aspect that I
have a major problem with is the fact that this *really* intrusive patch
that completely changes the way that ath9k schedules tx queues is hidden
behind this "API change" patch.
The way the API stands now, mt76 would require an equally big rework to
a different scheduling model which I'm not comfortable with at this point.
I would like to suggest the following to resolve these issues:
First we should revert these patches.
We can respin them shortly after in a modified form where
ieee80211_next_txq takes a 'queue' argument.
I'm almost done with the incremental change for that, and it also
supports passing -1 for queue so incrementally switching to the
scheduling that you're proposing will also work.
With that in place we can replace the ath9k change with a much smaller
patch that is easier to verify for correctness and won't introduce the
potential regressions that I pointed out.
I will take care of the mt76 porting today and I'll also help with
sorting out the ath10k issues.
Is that acceptable to you?
- Felix
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 1/3] mac80211: Add TXQ scheduling API
2017-12-14 12:44 ` Felix Fietkau
@ 2017-12-14 14:34 ` Toke Høiland-Jørgensen
2017-12-19 9:44 ` Johannes Berg
1 sibling, 0 replies; 20+ messages in thread
From: Toke Høiland-Jørgensen @ 2017-12-14 14:34 UTC (permalink / raw)
To: Felix Fietkau, make-wifi-fast, linux-wireless; +Cc: Johannes Berg
Felix Fietkau <nbd@nbd.name> writes:
> On 2017-12-14 13:15, Toke H=C3=B8iland-J=C3=B8rgensen wrote:
>> Felix Fietkau <nbd@nbd.name> writes:
>>=20
>>> On 2017-10-31 12:27, Toke H=C3=B8iland-J=C3=B8rgensen wrote:
>>>> This adds an API to mac80211 to handle scheduling of TXQs and changes =
the
>>>> interface between driver and mac80211 for TXQ handling as follows:
>>>>=20
>>>> - The wake_tx_queue callback interface no longer includes the TXQ. Ins=
tead,
>>>> the driver is expected to retrieve that from ieee80211_next_txq()
>>>>=20
>>>> - Two new mac80211 functions are added: ieee80211_next_txq() and
>>>> ieee80211_schedule_txq(). The former returns the next TXQ that shoul=
d be
>>>> scheduled, and is how the driver gets a queue to pull packets from. =
The
>>>> latter is called internally by mac80211 to start scheduling a queue,=
and
>>>> the driver is supposed to call it to re-schedule the TXQ after it is
>>>> finished pulling packets from it (unless the queue emptied).
>>>>=20
>>>> The ath9k and ath10k drivers are changed to use the new API.
>>>>=20
>>>> Signed-off-by: Toke H=C3=B8iland-J=C3=B8rgensen <toke@toke.dk>
>>> Sorry that I didn't have time to give this a thorough review earlier,
>>> since I was pretty busy with other projects. Now that I'm working on
>>> porting mt76 to this new API, some things in this patch strike me as
>>> rather odd, and there might be some bugs and nasty limitations here:
>>>
>>> In the new API you can no longer select txq entries by hardware queue.
>>> When using multiple WMM queues, this could lead to station entries being
>>> requeued unnecessarily (because there is no room in the hw queue for the
>>> txq entry that ieee80211_next_txq happens to return).
>>=20
>> Yeah, there's some tension between enforcing fairness (which is a per
>> station thing) and the WMM queueing stuff (which gives priority based on
>> WMM levels and ignores stations). There are basically two ways to
>> resolve this: Prioritising fairness or prioritising WMM levels. In the
>> former case, we first select which station to transmit to, and then
>> select the highest WMM priority level queued *for that station* (the
>> last part of this is missing from the code as it is now). In the latter
>> case, we keep scheduling per-WMM, then enforce fairness within that.
>>=20
>> The former case has the potential to lead to starved hardware queues,
>> while the latter leads to unfairness. We had a bit of discussion of
>> which is better at netdev, but did not resolve it. Personally, I think
>> prioritising fairness is better, but I'm willing to be convinced
>> otherwise by data :). So my plan is to implement that fully and try it
>> out, then evaluate based on actual experiments...
>
> I don't really see how the approach taken in the current code is
> actually prioritising fairness by starving hardware queues, since any
> txq that can't be serviced in the current call because of queue depth
> will be requeued. So based on the traffic pattern this might actually
> lead to *more* unfairness.
Yeah, I'm not quite happy with the current state of the code as far as
that is concerned. I thought I would have had time to revisit this, but
then a bunch of other stuff came up and I never got around to it... Will
come back to this soon I hope... :/
>>> Since ieee80211_next_txq also refills the airtime fairness quantum, this
>>> might lead to some minor fairness issues.
>>=20
>> I'm planning to change the way the scheduler works anyway, so this issue
>> should go away. Haven't had time to do that yet, unfortunately.
>
> Well, the problem is that the new code inside ath9k is a lot harder to
> verify for correct behavior (regarding the queue starvation issue), and
> I would really like to avoid nasty regressions that will be a nightmare
> to debug.
Fair point, avoiding that would be good of course :)
>>> In ath9k the code used to have a loop that goes through all pending txq
>>> entries until it has filled the hw queues again. You replaced that with
>>> some calls to ath_txq_schedule which now only considers one single txq.
>>> There are several reasons why this queue could potentially not be servi=
ced:
>>> - ieee80211_tx_dequeue returned no frame
>>> - frame does not fit within BA window
>>> - txq was for another queue which is already filled
>>> Depending on the exact circumstances with enough stations this might
>>> lead to hardware queues getting starved.
>>=20
>> Well, that loop was already removed when I implemented the in-driver
>> fairness scheduler.
> Now that's not true. I'm looking at the diff for "mac80211: Add TXQ
> scheduling API", and it removes these lines:
>
> - /*
> - * If we succeed in scheduling something, immediately restart to
> make
> - * sure we keep the HW busy.
> - */
> - if(ath_tx_sched_aggr(sc, txq, tid)) {
> - if (!active) {
> - spin_lock_bh(&acq->lock);
> - list_move_tail(&tid->list, &acq->acq_old);
> - spin_unlock_bh(&acq->lock);
> - }
> - goto begin;
> - }
Yeah, that is removed (which I suppose you are right could be
problematic in itself), but it was only being called when
ath_tx_sched_aggr() succeeds. Before (pre-airtime fairness) it did a
full loop through all scheduled tids until the hwq was full, which is
what I thought you were referring to.
>> We can't really avoid those cases entirely if we
>> want to enforce fairness (the BAW case in particular; if the only
>> eligible station from an airtime PoW has a full BAW, you have to
>> throttle and can potentially starve hwqs). However, don't think this
>> happens much in practice (or we would have seen performance regressions
>> by now).
> I think so far you simply haven't had enough users hammering on the
> new code yet.
Heh. Guess that could also be the case... ;)
>> The 'txq was for another queue' case is new with this patch, but that
>> goes back to the WMM/fairness tention above.
> I agree that this is something we need to figure out. One aspect that I
> have a major problem with is the fact that this *really* intrusive patch
> that completely changes the way that ath9k schedules tx queues is hidden
> behind this "API change" patch.
>
> The way the API stands now, mt76 would require an equally big rework to
> a different scheduling model which I'm not comfortable with at this
> point.
That is a fair point. I suppose that I was thinking too far ahead to
what I wanted to do next and didn't pay enough attention to splitting
things up into incremental changes. Of course this becomes more
important when there are more drivers affected.
> I would like to suggest the following to resolve these issues:
>
> First we should revert these patches.
> We can respin them shortly after in a modified form where
> ieee80211_next_txq takes a 'queue' argument.
By 'queue' you mean 'wmm hardware queue', right?
> I'm almost done with the incremental change for that, and it also
> supports passing -1 for queue so incrementally switching to the
> scheduling that you're proposing will also work.
>
> With that in place we can replace the ath9k change with a much smaller
> patch that is easier to verify for correctness and won't introduce the
> potential regressions that I pointed out.
>
> I will take care of the mt76 porting today and I'll also help with
> sorting out the ath10k issues.
>
> Is that acceptable to you?
Yup, sounds quite reasonable. And help with ath10k will be much
appreciated, thanks :)
-Toke
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 1/3] mac80211: Add TXQ scheduling API
2017-12-14 12:44 ` Felix Fietkau
2017-12-14 14:34 ` Toke Høiland-Jørgensen
@ 2017-12-19 9:44 ` Johannes Berg
1 sibling, 0 replies; 20+ messages in thread
From: Johannes Berg @ 2017-12-19 9:44 UTC (permalink / raw)
To: Felix Fietkau, Toke Høiland-Jørgensen, make-wifi-fast,
linux-wireless
On Thu, 2017-12-14 at 13:44 +0100, Felix Fietkau wrote:
>
> First we should revert these patches.
FWIW, I've just done that, if only to make the mt76 merge possible.
> We can respin them shortly after in a modified form where
> ieee80211_next_txq takes a 'queue' argument.
I'll leave the two of you to discuss that :-)
> I'm almost done with the incremental change for that, and it also
> supports passing -1 for queue so incrementally switching to the
> scheduling that you're proposing will also work.
Sounds fine to me.
> With that in place we can replace the ath9k change with a much smaller
> patch that is easier to verify for correctness and won't introduce the
> potential regressions that I pointed out.
>
> I will take care of the mt76 porting today and I'll also help with
> sorting out the ath10k issues.
So are you going to resend Toke's patches with your adjustments folded
in? Or just one of them?
johannes
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2017-12-19 9:44 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-16 16:09 [PATCH 1/2] mac80211: Add TXQ scheduling API Toke Høiland-Jørgensen
2017-10-16 16:09 ` [PATCH 2/2] mac80211: Add airtime accounting and scheduling to TXQs Toke Høiland-Jørgensen
2017-10-17 7:07 ` Johannes Berg
2017-10-17 7:34 ` Toke Høiland-Jørgensen
2017-10-17 10:00 ` Johannes Berg
2017-10-17 10:09 ` Toke Høiland-Jørgensen
2017-10-17 6:39 ` [PATCH 1/2] mac80211: Add TXQ scheduling API Johannes Berg
2017-10-27 14:14 ` [PATCH v2 1/3] " Toke Høiland-Jørgensen
2017-10-31 0:14 ` kbuild test robot
2017-10-31 0:45 ` kbuild test robot
2017-10-31 11:27 ` [PATCH v3 " Toke Høiland-Jørgensen
2017-12-14 11:33 ` Felix Fietkau
2017-12-14 12:15 ` Toke Høiland-Jørgensen
2017-12-14 12:44 ` Felix Fietkau
2017-12-14 14:34 ` Toke Høiland-Jørgensen
2017-12-19 9:44 ` Johannes Berg
2017-10-31 11:27 ` [PATCH v3 2/3] mac80211: Add airtime account and scheduling to TXQs Toke Høiland-Jørgensen
2017-10-31 11:27 ` [PATCH v3 3/3] ath9k: Switch to mac80211 airtime API Toke Høiland-Jørgensen
2017-10-27 14:14 ` [PATCH v2 2/3] mac80211: Add airtime account and scheduling to TXQs Toke Høiland-Jørgensen
2017-10-27 14:14 ` [PATCH v2 3/3] ath9k: Switch to mac80211 airtime API Toke Høiland-Jørgensen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.