* [PATCH RFC v5 0/4] Move TXQ scheduling and airtime fairness into mac80211
@ 2018-10-09 12:32 Toke Høiland-Jørgensen
2018-10-09 12:32 ` [PATCH RFC v5 2/4] cfg80211: Add airtime statistics and settings Toke Høiland-Jørgensen
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Toke Høiland-Jørgensen @ 2018-10-09 12:32 UTC (permalink / raw)
To: linux-wireless; +Cc: make-wifi-fast, Felix Fietkau, Rajkumar Manoharan, Kan Yan
Another updated version, addressing a few issues with the previous
version.
- Moved the airtime deficit queue wakeup code to its own tasklet to
lower overhead.
- Change the tasklet to just wake a single queue on each invocation,
relying to TX completion to continue transmissions.
- Don't try to re-schedule TXQs of stations that are being removed.
- A few cleanups and fixes.
The one thing I didn't change was to add another callback that the
driver can use to trigger the tasklet. Since it's now in its own
tasklet, hopefully the overhead is low enough that we can just call it
on every end_schedule(); and I'd rather not complicate the driver API
further.
Thanks to Rajkumar for testing the previous version. I thought I'd have
time to test this version myself and was planning to send as a non-RFC
PATCH after that, but that time didn't materialise. So I thought it was
better to send another RFC version instead of everyone having to suffer
from my tardiness :)
-Toke
---
Toke Høiland-Jørgensen (4):
mac80211: Add TXQ scheduling API
cfg80211: Add airtime statistics and settings
mac80211: Add airtime accounting and scheduling to TXQs
ath9k: Switch to mac80211 TXQ scheduling and airtime APIs
drivers/net/wireless/ath/ath9k/ath9k.h | 14 --
drivers/net/wireless/ath/ath9k/debug.c | 3
drivers/net/wireless/ath/ath9k/debug.h | 8 -
drivers/net/wireless/ath/ath9k/debug_sta.c | 54 ------
drivers/net/wireless/ath/ath9k/init.c | 3
drivers/net/wireless/ath/ath9k/recv.c | 9 -
drivers/net/wireless/ath/ath9k/xmit.c | 244 ++++++++--------------------
include/net/cfg80211.h | 10 +
include/net/mac80211.h | 113 +++++++++++++
include/uapi/linux/nl80211.h | 15 ++
net/mac80211/agg-tx.c | 2
net/mac80211/cfg.c | 3
net/mac80211/debugfs.c | 3
net/mac80211/debugfs_sta.c | 51 ++++++
net/mac80211/driver-ops.h | 9 +
net/mac80211/ieee80211_i.h | 14 ++
net/mac80211/main.c | 11 +
net/mac80211/sta_info.c | 54 ++++++
net/mac80211/sta_info.h | 13 +
net/mac80211/status.c | 6 +
net/mac80211/tx.c | 137 ++++++++++++++++
net/mac80211/util.c | 75 +++++++++
net/wireless/nl80211.c | 29 +++
23 files changed, 603 insertions(+), 277 deletions(-)
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH RFC v5 1/4] mac80211: Add TXQ scheduling API
2018-10-09 12:32 [PATCH RFC v5 0/4] Move TXQ scheduling and airtime fairness into mac80211 Toke Høiland-Jørgensen
2018-10-09 12:32 ` [PATCH RFC v5 2/4] cfg80211: Add airtime statistics and settings Toke Høiland-Jørgensen
@ 2018-10-09 12:32 ` Toke Høiland-Jørgensen
2018-10-09 12:32 ` [PATCH RFC v5 4/4] ath9k: Switch to mac80211 TXQ scheduling and airtime APIs Toke Høiland-Jørgensen
2018-10-09 12:32 ` [PATCH RFC v5 3/4] mac80211: Add airtime accounting and scheduling to TXQs Toke Høiland-Jørgensen
3 siblings, 0 replies; 14+ messages in thread
From: Toke Høiland-Jørgensen @ 2018-10-09 12:32 UTC (permalink / raw)
To: linux-wireless; +Cc: make-wifi-fast, Felix Fietkau, Rajkumar Manoharan, Kan Yan
This adds an API to mac80211 to handle scheduling of TXQs. The interface
between driver and mac80211 for TXQ handling is changed by adding two new
functions: ieee80211_next_txq(), which will return the next TXQ to schedule
in the current round-robin rotation, and ieee80211_return_txq(), which the
driver uses to indicate that it has finished scheduling a TXQ (which will
then be put back in the scheduling rotation if it isn't empty).
The driver must call ieee80211_txq_schedule_start() at the start of each
scheduling session, and ieee80211_txq_schedule_end() at the end. The API
then guarantees that the same TXQ is not returned twice in the same
session (so a driver can loop on ieee80211_next_txq() without worrying
about breaking the loop.
Usage of the new API is optional, so drivers can be ported one at a time.
In this patch, the actual scheduling performed by mac80211 is simple
round-robin, but a subsequent commit adds airtime fairness awareness to the
scheduler.
Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
---
include/net/mac80211.h | 61 +++++++++++++++++++++++++++++++++++++++++---
net/mac80211/agg-tx.c | 2 +
net/mac80211/driver-ops.h | 9 ++++++
net/mac80211/ieee80211_i.h | 9 ++++++
net/mac80211/main.c | 5 ++++
net/mac80211/sta_info.c | 2 +
net/mac80211/tx.c | 59 ++++++++++++++++++++++++++++++++++++++++++-
7 files changed, 140 insertions(+), 7 deletions(-)
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index c4fadbafbf21..469e88a32f94 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -108,9 +108,15 @@
* 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 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.
+ *
+ * Drivers can optionally delegate responsibility for scheduling queues to
+ * mac80211, to take advantage of airtime fairness accounting. In this case, to
+ * obtain the next queue to pull frames from, the driver calls
+ * ieee80211_next_txq(). The driver is then expected to return the txq using
+ * ieee80211_return_txq().
*
* For AP powersave TIM handling, the driver only needs to indicate if it has
* buffered packets in the driver specific data structures by calling
@@ -6045,13 +6051,60 @@ 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 station or virtual interface, or 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_next_txq - get next tx queue to pull packets from
+ *
+ * @hw: pointer as obtained from ieee80211_alloc_hw()
+ * @ac: AC number to return packets from.
+ *
+ * Should only be called between calls to ieee80211_txq_schedule_start()
+ * and ieee80211_txq_schedule_end().
+ * Returns the next txq if successful, %NULL if no queue is eligible. If a txq
+ * is returned, it should be returned with ieee80211_return_txq() after the
+ * driver has finished scheduling it.
+ */
+struct ieee80211_txq *ieee80211_next_txq(struct ieee80211_hw *hw, u8 ac);
+
+/**
+ * ieee80211_return_txq - return a TXQ previously acquired by ieee80211_next_txq()
+ *
+ * @hw: pointer as obtained from ieee80211_alloc_hw()
+ * @txq: pointer obtained from station or virtual interface
+ *
+ * Should only be called between calls to ieee80211_txq_schedule_start()
+ * and ieee80211_txq_schedule_end().
+ */
+void ieee80211_return_txq(struct ieee80211_hw *hw, struct ieee80211_txq *txq);
+
+/**
+ * ieee80211_txq_schedule_start - acquire locks for safe scheduling of an AC
+ *
+ * @hw: pointer as obtained from ieee80211_alloc_hw()
+ * @ac: AC number to acquire locks for
+ *
+ * Acquire locks needed to schedule TXQs from the given AC. Should be called
+ * before ieee80211_next_txq() or ieee80211_return_txq().
+ */
+void ieee80211_txq_schedule_start(struct ieee80211_hw *hw, u8 ac);
+
+/**
+ * ieee80211_txq_schedule_end - release locks for safe scheduling of an AC
+ *
+ * @hw: pointer as obtained from ieee80211_alloc_hw()
+ * @ac: AC number to acquire locks for
+ *
+ * Release locks previously acquired by ieee80211_txq_schedule_end().
+ */
+void ieee80211_txq_schedule_end(struct ieee80211_hw *hw, u8 ac);
+
/**
* 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 69e831bc317b..e94b1a0407af 100644
--- a/net/mac80211/agg-tx.c
+++ b/net/mac80211/agg-tx.c
@@ -229,7 +229,7 @@ ieee80211_agg_start_txq(struct sta_info *sta, int tid, bool enable)
clear_bit(IEEE80211_TXQ_STOP, &txqi->flags);
local_bh_disable();
rcu_read_lock();
- drv_wake_tx_queue(sta->sdata->local, txqi);
+ schedule_and_wake_txq(sta->sdata->local, txqi);
rcu_read_unlock();
local_bh_enable();
}
diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
index e42c641b6190..0d47dadee747 100644
--- a/net/mac80211/driver-ops.h
+++ b/net/mac80211/driver-ops.h
@@ -1173,6 +1173,15 @@ static inline void drv_wake_tx_queue(struct ieee80211_local *local,
local->ops->wake_tx_queue(&local->hw, &txq->txq);
}
+static inline void schedule_and_wake_txq(struct ieee80211_local *local,
+ struct txq_info *txqi)
+{
+ spin_lock_bh(&local->active_txq_lock[txqi->txq.ac]);
+ ieee80211_return_txq(&local->hw, &txqi->txq);
+ spin_unlock_bh(&local->active_txq_lock[txqi->txq.ac]);
+ drv_wake_tx_queue(local, txqi);
+}
+
static inline int drv_can_aggregate_in_amsdu(struct ieee80211_local *local,
struct sk_buff *head,
struct sk_buff *skb)
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index f40a2167935f..976531717902 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -829,6 +829,8 @@ enum txq_info_flags {
* a fq_flow which is already owned by a different tin
* @def_cvars: codel vars for @def_flow
* @frags: used to keep fragments created after dequeue
+ * @schedule_order: used with ieee80211_local->active_txqs
+ * @schedule_round: counter to prevent infinite loops on TXQ scheduling
*/
struct txq_info {
struct fq_tin tin;
@@ -836,6 +838,8 @@ struct txq_info {
struct codel_vars def_cvars;
struct codel_stats cstats;
struct sk_buff_head frags;
+ struct list_head schedule_order;
+ u16 schedule_round;
unsigned long flags;
/* keep last! */
@@ -1127,6 +1131,11 @@ struct ieee80211_local {
struct codel_vars *cvars;
struct codel_params cparams;
+ /* protects active_txqs and txqi->schedule_order */
+ spinlock_t active_txq_lock[IEEE80211_NUM_ACS];
+ struct list_head active_txqs[IEEE80211_NUM_ACS];
+ u16 schedule_round[IEEE80211_NUM_ACS];
+
const struct ieee80211_ops *ops;
/*
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 77381017bac7..d9315de90b48 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -663,6 +663,11 @@ 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);
+ for (i = 0; i < IEEE80211_NUM_ACS; i++) {
+ INIT_LIST_HEAD(&local->active_txqs[i]);
+ spin_lock_init(&local->active_txq_lock[i]);
+ }
+
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 fb8c2252ac0e..c2f5cb7df54f 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -1249,7 +1249,7 @@ void ieee80211_sta_ps_deliver_wakeup(struct sta_info *sta)
if (!sta->sta.txq[i] || !txq_has_queue(sta->sta.txq[i]))
continue;
- drv_wake_tx_queue(local, to_txq_info(sta->sta.txq[i]));
+ schedule_and_wake_txq(local, to_txq_info(sta->sta.txq[i]));
}
skb_queue_head_init(&pending);
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index c42bfa1dcd2c..1e071121cb44 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1445,6 +1445,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;
@@ -1485,6 +1486,9 @@ 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);
+ spin_lock_bh(&local->active_txq_lock[txqi->txq.ac]);
+ list_del_init(&txqi->schedule_order);
+ spin_unlock_bh(&local->active_txq_lock[txqi->txq.ac]);
}
void ieee80211_txq_set_params(struct ieee80211_local *local)
@@ -1601,7 +1605,7 @@ 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);
+ schedule_and_wake_txq(local, txqi);
return true;
}
@@ -3623,6 +3627,59 @@ struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
}
EXPORT_SYMBOL(ieee80211_tx_dequeue);
+struct ieee80211_txq *ieee80211_next_txq(struct ieee80211_hw *hw, u8 ac)
+{
+ struct ieee80211_local *local = hw_to_local(hw);
+ struct txq_info *txqi = NULL;
+
+ lockdep_assert_held(&local->active_txq_lock[ac]);
+
+ txqi = list_first_entry_or_null(&local->active_txqs[ac],
+ struct txq_info,
+ schedule_order);
+
+ if (!txqi || txqi->schedule_round == local->schedule_round[ac])
+ return NULL;
+
+ list_del_init(&txqi->schedule_order);
+ txqi->schedule_round = local->schedule_round[ac];
+ return &txqi->txq;
+}
+EXPORT_SYMBOL(ieee80211_next_txq);
+
+void ieee80211_return_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);
+
+ lockdep_assert_held(&local->active_txq_lock[txq->ac]);
+
+ if (list_empty(&txqi->schedule_order) &&
+ (!skb_queue_empty(&txqi->frags) || txqi->tin.backlog_packets))
+ list_add_tail(&txqi->schedule_order,
+ &local->active_txqs[txq->ac]);
+
+}
+EXPORT_SYMBOL(ieee80211_return_txq);
+
+void ieee80211_txq_schedule_start(struct ieee80211_hw *hw, u8 ac)
+{
+ struct ieee80211_local *local = hw_to_local(hw);
+
+ spin_lock_bh(&local->active_txq_lock[ac]);
+ local->schedule_round[ac]++;
+}
+EXPORT_SYMBOL(ieee80211_txq_schedule_start);
+
+void ieee80211_txq_schedule_end(struct ieee80211_hw *hw, u8 ac)
+{
+ struct ieee80211_local *local = hw_to_local(hw);
+
+ spin_unlock_bh(&local->active_txq_lock[ac]);
+}
+EXPORT_SYMBOL(ieee80211_txq_schedule_end);
+
void __ieee80211_subif_start_xmit(struct sk_buff *skb,
struct net_device *dev,
u32 info_flags)
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH RFC v5 2/4] cfg80211: Add airtime statistics and settings
2018-10-09 12:32 [PATCH RFC v5 0/4] Move TXQ scheduling and airtime fairness into mac80211 Toke Høiland-Jørgensen
@ 2018-10-09 12:32 ` Toke Høiland-Jørgensen
2018-10-09 12:32 ` [PATCH RFC v5 1/4] mac80211: Add TXQ scheduling API Toke Høiland-Jørgensen
` (2 subsequent siblings)
3 siblings, 0 replies; 14+ messages in thread
From: Toke Høiland-Jørgensen @ 2018-10-09 12:32 UTC (permalink / raw)
To: linux-wireless; +Cc: make-wifi-fast, Felix Fietkau, Rajkumar Manoharan, Kan Yan
This adds TX airtime statistics to the cfg80211 station dump (to go along
with the RX info already present), and adds a new parameter to set the
airtime weight of each station. The latter allows userspace to implement
policies for different stations by varying their weights.
Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
---
include/net/cfg80211.h | 10 +++++++++-
include/uapi/linux/nl80211.h | 15 +++++++++++++++
net/wireless/nl80211.c | 29 +++++++++++++++++++++++++++++
3 files changed, 53 insertions(+), 1 deletion(-)
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 9f3ed79c39d7..91d6fc222263 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -990,6 +990,7 @@ enum station_parameters_apply_mask {
* @support_p2p_ps: information if station supports P2P PS mechanism
* @he_capa: HE capabilities of station
* @he_capa_len: the length of the HE capabilities
+ * @airtime_weight: airtime scheduler weight for this station
*/
struct station_parameters {
const u8 *supported_rates;
@@ -1019,6 +1020,7 @@ struct station_parameters {
int support_p2p_ps;
const struct ieee80211_he_cap_elem *he_capa;
u8 he_capa_len;
+ u16 airtime_weight;
};
/**
@@ -1286,6 +1288,8 @@ struct cfg80211_tid_stats {
* @rx_beacon_signal_avg: signal strength average (in dBm) for beacons received
* from this peer
* @rx_duration: aggregate PPDU duration(usecs) for all the frames from a peer
+ * @tx_duration: aggregate PPDU duration(usecs) for all the frames to a peer
+ * @airtime_weight: current airtime scheduling weight
* @pertid: per-TID statistics, see &struct cfg80211_tid_stats, using the last
* (IEEE80211_NUM_TIDS) index for MSDUs not encapsulated in QoS-MPDUs.
* Note that this doesn't use the @filled bit, but is used if non-NULL.
@@ -1332,10 +1336,12 @@ struct station_info {
u32 expected_throughput;
- u64 rx_beacon;
+ u64 tx_duration;
u64 rx_duration;
+ u64 rx_beacon;
u8 rx_beacon_signal_avg;
struct cfg80211_tid_stats *pertid;
+ u16 airtime_weight;
s8 ack_signal;
s8 avg_ack_signal;
};
@@ -2363,6 +2369,8 @@ enum wiphy_params_flags {
WIPHY_PARAM_TXQ_QUANTUM = 1 << 8,
};
+#define IEEE80211_DEFAULT_AIRTIME_WEIGHT 256
+
/**
* struct cfg80211_pmksa - PMK Security Association
*
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index cfc94178d608..3664bdc7c8c1 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -2241,6 +2241,9 @@ enum nl80211_commands {
* association request when used with NL80211_CMD_NEW_STATION). Can be set
* only if %NL80211_STA_FLAG_WME is set.
*
+ * @NL80211_ATTR_AIRTIME_WEIGHT: Station's weight when scheduled by the airtime
+ * scheduler.
+ *
* @NUM_NL80211_ATTR: total number of nl80211_attrs available
* @NL80211_ATTR_MAX: highest attribute number currently defined
* @__NL80211_ATTR_AFTER_LAST: internal use
@@ -2682,6 +2685,8 @@ enum nl80211_attrs {
NL80211_ATTR_HE_CAPABILITY,
+ NL80211_ATTR_AIRTIME_WEIGHT,
+
/* add attributes here, update the policy in nl80211.c */
__NL80211_ATTR_AFTER_LAST,
@@ -3051,6 +3056,9 @@ enum nl80211_sta_bss_param {
* @NL80211_STA_INFO_PAD: attribute used for padding for 64-bit alignment
* @NL80211_STA_INFO_ACK_SIGNAL: signal strength of the last ACK frame(u8, dBm)
* @NL80211_STA_INFO_ACK_SIGNAL_AVG: avg signal strength of ACK frames (s8, dBm)
+ * @NL80211_STA_INFO_TX_DURATION: aggregate PPDU duration for all frames
+ * sent to the station (u64, usec)
+ * @NL80211_STA_INFO_AIRTIME_WEIGHT: current airtime weight for station (u16)
* @__NL80211_STA_INFO_AFTER_LAST: internal
* @NL80211_STA_INFO_MAX: highest possible station info attribute
*/
@@ -3091,6 +3099,8 @@ enum nl80211_sta_info {
NL80211_STA_INFO_PAD,
NL80211_STA_INFO_ACK_SIGNAL,
NL80211_STA_INFO_ACK_SIGNAL_AVG,
+ NL80211_STA_INFO_TX_DURATION,
+ NL80211_STA_INFO_AIRTIME_WEIGHT,
/* keep last */
__NL80211_STA_INFO_AFTER_LAST,
@@ -5231,6 +5241,10 @@ enum nl80211_feature_flags {
* if this flag is not set. Ignoring this can leak clear text packets and/or
* freeze the connection.
*
+ * @NL80211_EXT_FEATURE_AIRTIME_FAIRNESS: Driver supports getting airtime
+ * fairness for transmitted packets and has enabled airtime fairness
+ * scheduling.
+ *
* @NUM_NL80211_EXT_FEATURES: number of extended features.
* @MAX_NL80211_EXT_FEATURES: highest extended feature index.
*/
@@ -5269,6 +5283,7 @@ enum nl80211_ext_feature_index {
NL80211_EXT_FEATURE_SCAN_RANDOM_SN,
NL80211_EXT_FEATURE_SCAN_MIN_PREQ_CONTENT,
NL80211_EXT_FEATURE_CAN_REPLACE_PTK0,
+ NL80211_EXT_FEATURE_AIRTIME_FAIRNESS,
/* add new features before the definition below */
NUM_NL80211_EXT_FEATURES,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index d5f9b5235cdd..9db19ae57716 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -430,6 +430,8 @@ static const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
[NL80211_ATTR_TXQ_QUANTUM] = { .type = NLA_U32 },
[NL80211_ATTR_HE_CAPABILITY] = { .type = NLA_BINARY,
.len = NL80211_HE_MAX_CAPABILITY_LEN },
+
+ [NL80211_ATTR_AIRTIME_WEIGHT] = { .type = NLA_U16 },
};
/* policy for the key attributes */
@@ -4656,6 +4658,11 @@ static int nl80211_send_station(struct sk_buff *msg, u32 cmd, u32 portid,
PUT_SINFO(PLID, plid, u16);
PUT_SINFO(PLINK_STATE, plink_state, u8);
PUT_SINFO_U64(RX_DURATION, rx_duration);
+ PUT_SINFO_U64(TX_DURATION, tx_duration);
+
+ if (wiphy_ext_feature_isset(&rdev->wiphy,
+ NL80211_EXT_FEATURE_AIRTIME_FAIRNESS))
+ PUT_SINFO(AIRTIME_WEIGHT, airtime_weight, u16);
switch (rdev->wiphy.signal_type) {
case CFG80211_SIGNAL_TYPE_MBM:
@@ -5293,6 +5300,17 @@ static int nl80211_set_station(struct sk_buff *skb, struct genl_info *info)
nla_get_u8(info->attrs[NL80211_ATTR_OPMODE_NOTIF]);
}
+ if (info->attrs[NL80211_ATTR_AIRTIME_WEIGHT]) {
+ if (!wiphy_ext_feature_isset(&rdev->wiphy,
+ NL80211_EXT_FEATURE_AIRTIME_FAIRNESS))
+ return -EOPNOTSUPP;
+
+ params.airtime_weight =
+ nla_get_u16(info->attrs[NL80211_ATTR_AIRTIME_WEIGHT]);
+ if (!params.airtime_weight)
+ return -EINVAL;
+ }
+
/* Include parameters for TDLS peer (will check later) */
err = nl80211_set_station_tdls(info, ¶ms);
if (err)
@@ -5431,6 +5449,17 @@ static int nl80211_new_station(struct sk_buff *skb, struct genl_info *info)
return -EINVAL;
}
+ if (info->attrs[NL80211_ATTR_AIRTIME_WEIGHT]) {
+ if (!wiphy_ext_feature_isset(&rdev->wiphy,
+ NL80211_EXT_FEATURE_AIRTIME_FAIRNESS))
+ return -EOPNOTSUPP;
+
+ params.airtime_weight =
+ nla_get_u16(info->attrs[NL80211_ATTR_AIRTIME_WEIGHT]);
+ if (!params.airtime_weight)
+ return -EINVAL;
+ }
+
err = nl80211_parse_sta_channel_info(info, ¶ms);
if (err)
return err;
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH RFC v5 4/4] ath9k: Switch to mac80211 TXQ scheduling and airtime APIs
2018-10-09 12:32 [PATCH RFC v5 0/4] Move TXQ scheduling and airtime fairness into mac80211 Toke Høiland-Jørgensen
2018-10-09 12:32 ` [PATCH RFC v5 2/4] cfg80211: Add airtime statistics and settings Toke Høiland-Jørgensen
2018-10-09 12:32 ` [PATCH RFC v5 1/4] mac80211: Add TXQ scheduling API Toke Høiland-Jørgensen
@ 2018-10-09 12:32 ` Toke Høiland-Jørgensen
2018-10-09 12:32 ` [PATCH RFC v5 3/4] mac80211: Add airtime accounting and scheduling to TXQs Toke Høiland-Jørgensen
3 siblings, 0 replies; 14+ messages in thread
From: Toke Høiland-Jørgensen @ 2018-10-09 12:32 UTC (permalink / raw)
To: linux-wireless; +Cc: make-wifi-fast, Felix Fietkau, Rajkumar Manoharan, Kan Yan
This moves the ath9k driver to use the mac80211 TXQ scheduling and
airtime accounting APIs, removing the corresponding state tracking
inside the driver.
Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
---
drivers/net/wireless/ath/ath9k/ath9k.h | 14 --
drivers/net/wireless/ath/ath9k/debug.c | 3
drivers/net/wireless/ath/ath9k/debug.h | 8 -
drivers/net/wireless/ath/ath9k/debug_sta.c | 54 ------
drivers/net/wireless/ath/ath9k/init.c | 3
drivers/net/wireless/ath/ath9k/recv.c | 9 -
drivers/net/wireless/ath/ath9k/xmit.c | 244 ++++++++--------------------
7 files changed, 73 insertions(+), 262 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index 21ba20981a80..90b56766dab1 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
@@ -246,10 +244,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 {
@@ -263,12 +259,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,11 +979,6 @@ void ath_ant_comb_scan(struct ath_softc *sc, struct ath_rx_status *rs);
#define ATH9K_NUM_CHANCTX 2 /* supports 2 operating channels */
-#define AIRTIME_USE_TX BIT(0)
-#define AIRTIME_USE_RX BIT(1)
-#define AIRTIME_USE_NEW_QUEUES BIT(2)
-#define AIRTIME_ACTIVE(flags) (!!(flags & (AIRTIME_USE_TX|AIRTIME_USE_RX)))
-
struct ath_softc {
struct ieee80211_hw *hw;
struct device *dev;
@@ -1034,8 +1022,6 @@ struct ath_softc {
short nbcnvifs;
unsigned long ps_usecount;
- u16 airtime_flags; /* AIRTIME_* */
-
struct ath_rx rx;
struct ath_tx tx;
struct ath_beacon beacon;
diff --git a/drivers/net/wireless/ath/ath9k/debug.c b/drivers/net/wireless/ath/ath9k/debug.c
index 0a6eb8a8c1ed..f928d3a07caa 100644
--- a/drivers/net/wireless/ath/ath9k/debug.c
+++ b/drivers/net/wireless/ath/ath9k/debug.c
@@ -1456,9 +1456,6 @@ int ath9k_init_debug(struct ath_hw *ah)
#endif
debugfs_create_file("tpc", 0600, sc->debug.debugfs_phy, sc, &fops_tpc);
- debugfs_create_u16("airtime_flags", 0600,
- sc->debug.debugfs_phy, &sc->airtime_flags);
-
debugfs_create_file("nf_override", 0600,
sc->debug.debugfs_phy, sc, &fops_nf_override);
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 a6f45f1bb5bb..bb6f3250aa30 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", 0444, dir, an, &fops_node_aggr);
debugfs_create_file("node_recv", 0444, dir, an, &fops_node_recv);
- debugfs_create_file("airtime", 0444, dir, an, &fops_airtime);
}
diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index c070a9e51ebf..45a31f8a1514 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -676,8 +676,6 @@ 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->sc_ah = ah;
sc->dfs_detector = dfs_pattern_detector_init(common, NL80211_DFS_UNSET);
@@ -1013,6 +1011,7 @@ static void ath9k_set_hw_capab(struct ath_softc *sc, struct ieee80211_hw *hw)
SET_IEEE80211_PERM_ADDR(hw, common->macaddr);
wiphy_ext_feature_set(hw->wiphy, NL80211_EXT_FEATURE_CQM_RSSI_LIST);
+ wiphy_ext_feature_set(hw->wiphy, NL80211_EXT_FEATURE_AIRTIME_FAIRNESS);
}
int ath9k_init_device(u16 devid, struct ath_softc *sc,
diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
index a8ac42c96d71..fef8fbe12f45 100644
--- a/drivers/net/wireless/ath/ath9k/recv.c
+++ b/drivers/net/wireless/ath/ath9k/recv.c
@@ -1054,14 +1054,7 @@ 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);
+ ieee80211_sta_register_airtime(sta, tidno, 0, airtime);
exit:
rcu_read_unlock();
}
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index 43b6c8508e49..ce6763c3c3ee 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -113,44 +113,14 @@ 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;
+ struct ieee80211_txq *queue = container_of(
+ (void *)tid, struct ieee80211_txq, drv_priv);
- 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);
+ ieee80211_return_txq(sc->hw, queue);
}
-
void ath9k_wake_tx_queue(struct ieee80211_hw *hw, struct ieee80211_txq *queue)
{
struct ath_softc *sc = hw->priv;
@@ -163,11 +133,7 @@ void ath9k_wake_tx_queue(struct ieee80211_hw *hw, struct ieee80211_txq *queue)
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);
}
@@ -217,8 +183,8 @@ ath_get_skb_tid(struct ath_softc *sc, struct ath_node *an, struct sk_buff *skb)
return ATH_AN_2_TID(an, tidno);
}
-static struct sk_buff *
-ath_tid_pull(struct ath_atx_tid *tid)
+static int
+ath_tid_pull(struct ath_atx_tid *tid, struct sk_buff **skbuf)
{
struct ieee80211_txq *txq = container_of((void*)tid, struct ieee80211_txq, drv_priv);
struct ath_softc *sc = tid->an->sc;
@@ -229,20 +195,15 @@ ath_tid_pull(struct ath_atx_tid *tid)
};
struct sk_buff *skb;
struct ath_frame_info *fi;
- int q;
-
- if (!tid->has_queued)
- return NULL;
+ int q, ret;
skb = ieee80211_tx_dequeue(hw, txq);
- if (!skb) {
- tid->has_queued = false;
- return NULL;
- }
+ if (!skb)
+ return -ENOENT;
- if (ath_tx_prepare(hw, skb, &txctl)) {
+ if ((ret = ath_tx_prepare(hw, skb, &txctl))) {
ieee80211_free_txskb(hw, skb);
- return NULL;
+ return ret;
}
q = skb_get_queue_mapping(skb);
@@ -252,24 +213,19 @@ ath_tid_pull(struct ath_atx_tid *tid)
++tid->txq->pending_frames;
}
- return skb;
-}
-
-
-static bool ath_tid_has_buffered(struct ath_atx_tid *tid)
-{
- return !skb_queue_empty(&tid->retry_q) || tid->has_queued;
-}
+ *skbuf = skb;
+ return 0;
+ }
-static struct sk_buff *ath_tid_dequeue(struct ath_atx_tid *tid)
+static int ath_tid_dequeue(struct ath_atx_tid *tid,
+ struct sk_buff **skb)
{
- struct sk_buff *skb;
+ int ret = 0;
+ *skb = __skb_dequeue(&tid->retry_q);
+ if (!*skb)
+ ret = ath_tid_pull(tid, skb);
- skb = __skb_dequeue(&tid->retry_q);
- if (!skb)
- skb = ath_tid_pull(tid);
-
- return skb;
+ return ret;
}
static void ath_tx_flush_tid(struct ath_softc *sc, struct ath_atx_tid *tid)
@@ -365,11 +321,12 @@ static void ath_tid_drain(struct ath_softc *sc, struct ath_txq *txq,
struct list_head bf_head;
struct ath_tx_status ts;
struct ath_frame_info *fi;
+ int ret;
memset(&ts, 0, sizeof(ts));
INIT_LIST_HEAD(&bf_head);
- while ((skb = ath_tid_dequeue(tid))) {
+ while ((ret = ath_tid_dequeue(tid, &skb)) == 0) {
fi = get_frame_info(skb);
bf = fi->bf;
@@ -681,7 +638,6 @@ 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);
-
if (ts->ts_status & (ATH9K_TXERR_FILT | ATH9K_TXERR_XRETRY))
tid->clear_ps_filter = true;
}
@@ -708,11 +664,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,
- struct ath_tx_status *ts)
+static void ath_tx_count_airtime(struct ath_softc *sc, struct ieee80211_sta *sta,
+ struct ath_buf *bf, struct ath_tx_status *ts)
{
- struct ath_txq *txq = tid->txq;
u32 airtime = 0;
int i;
@@ -722,17 +676,7 @@ 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];
-
- 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);
+ ieee80211_sta_register_airtime(sta, ts->tid, airtime, 0);
}
static void ath_tx_process_buffer(struct ath_softc *sc, struct ath_txq *txq,
@@ -762,7 +706,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, sta, bf, ts);
if (ts->ts_status & (ATH9K_TXERR_FILT | ATH9K_TXERR_XRETRY))
tid->clear_ps_filter = true;
}
@@ -946,20 +890,21 @@ static int ath_compute_num_delims(struct ath_softc *sc, struct ath_atx_tid *tid,
return ndelim;
}
-static struct ath_buf *
+static int
ath_tx_get_tid_subframe(struct ath_softc *sc, struct ath_txq *txq,
- struct ath_atx_tid *tid)
+ struct ath_atx_tid *tid, struct ath_buf **buf)
{
struct ieee80211_tx_info *tx_info;
struct ath_frame_info *fi;
- struct sk_buff *skb, *first_skb = NULL;
struct ath_buf *bf;
+ struct sk_buff *skb, *first_skb = NULL;
u16 seqno;
+ int ret;
while (1) {
- skb = ath_tid_dequeue(tid);
- if (!skb)
- break;
+ ret = ath_tid_dequeue(tid, &skb);
+ if (ret < 0)
+ return ret;
fi = get_frame_info(skb);
bf = fi->bf;
@@ -991,7 +936,7 @@ ath_tx_get_tid_subframe(struct ath_softc *sc, struct ath_txq *txq,
if (!(tx_info->flags & IEEE80211_TX_CTL_AMPDU)) {
bf->bf_state.bf_type = 0;
- return bf;
+ break;
}
bf->bf_state.bf_type = BUF_AMPDU | BUF_AGGR;
@@ -1010,7 +955,7 @@ ath_tx_get_tid_subframe(struct ath_softc *sc, struct ath_txq *txq,
first_skb = skb;
continue;
}
- break;
+ return -EINPROGRESS;
}
if (tid->bar_index > ATH_BA_INDEX(tid->seq_start, seqno)) {
@@ -1027,10 +972,11 @@ ath_tx_get_tid_subframe(struct ath_softc *sc, struct ath_txq *txq,
if (bf_isampdu(bf))
ath_tx_addto_baw(sc, tid, bf);
- return bf;
+ break;
}
- return NULL;
+ *buf = bf;
+ return 0;
}
static int
@@ -1040,7 +986,7 @@ ath_tx_form_aggr(struct ath_softc *sc, struct ath_txq *txq,
{
#define PADBYTES(_len) ((4 - ((_len) % 4)) % 4)
struct ath_buf *bf = bf_first, *bf_prev = NULL;
- int nframes = 0, ndelim;
+ int nframes = 0, ndelim, ret;
u16 aggr_limit = 0, al = 0, bpad = 0,
al_delta, h_baw = tid->baw_size / 2;
struct ieee80211_tx_info *tx_info;
@@ -1092,7 +1038,9 @@ ath_tx_form_aggr(struct ath_softc *sc, struct ath_txq *txq,
bf_prev = bf;
- bf = ath_tx_get_tid_subframe(sc, txq, tid);
+ ret = ath_tx_get_tid_subframe(sc, txq, tid, &bf);
+ if (ret < 0)
+ break;
}
goto finish;
stop:
@@ -1489,7 +1437,7 @@ ath_tx_form_burst(struct ath_softc *sc, struct ath_txq *txq,
struct ath_buf *bf_first)
{
struct ath_buf *bf = bf_first, *bf_prev = NULL;
- int nframes = 0;
+ int nframes = 0, ret;
do {
struct ieee80211_tx_info *tx_info;
@@ -1503,8 +1451,8 @@ ath_tx_form_burst(struct ath_softc *sc, struct ath_txq *txq,
if (nframes >= 2)
break;
- bf = ath_tx_get_tid_subframe(sc, txq, tid);
- if (!bf)
+ ret = ath_tx_get_tid_subframe(sc, txq, tid, &bf);
+ if (ret < 0)
break;
tx_info = IEEE80211_SKB_CB(bf->bf_mpdu);
@@ -1517,30 +1465,27 @@ 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 ath_buf *bf = NULL;
struct ieee80211_tx_info *tx_info;
struct list_head bf_q;
- int aggr_len = 0;
+ int aggr_len = 0, ret;
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;
+ ret = ath_tx_get_tid_subframe(sc, txq, tid, &bf);
+ if (ret < 0)
+ return ret;
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 -EBUSY;
}
ath_set_rates(tid->an->vif, tid->an->sta, bf);
@@ -1550,7 +1495,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 -EAGAIN;
if (tid->clear_ps_filter || tid->an->no_ps_filter) {
tid->clear_ps_filter = false;
@@ -1559,7 +1504,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,
@@ -1622,28 +1567,16 @@ 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;
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;
- }
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);
}
}
@@ -1662,11 +1595,12 @@ void ath_tx_aggr_wakeup(struct ath_softc *sc, struct ath_node *an)
ath_txq_lock(sc, txq);
tid->clear_ps_filter = true;
- if (ath_tid_has_buffered(tid)) {
+ if (!skb_queue_empty(&tid->retry_q)) {
ath_tx_queue_tid(sc, tid);
ath_txq_schedule(sc, txq);
}
ath_txq_unlock_complete(sc, txq);
+
}
}
@@ -1697,9 +1631,9 @@ void ath9k_release_buffered_frames(struct ieee80211_hw *hw,
struct ath_txq *txq = sc->tx.uapsdq;
struct ieee80211_tx_info *info;
struct list_head bf_q;
- struct ath_buf *bf_tail = NULL, *bf;
+ struct ath_buf *bf_tail = NULL, *bf = NULL;
int sent = 0;
- int i;
+ int i, ret;
INIT_LIST_HEAD(&bf_q);
for (i = 0; tids && nframes; i++, tids >>= 1) {
@@ -1712,8 +1646,8 @@ void ath9k_release_buffered_frames(struct ieee80211_hw *hw,
ath_txq_lock(sc, tid->txq);
while (nframes > 0) {
- bf = ath_tx_get_tid_subframe(sc, sc->tx.uapsdq, tid);
- if (!bf)
+ ret = ath_tx_get_tid_subframe(sc, sc->tx.uapsdq, tid, &bf);
+ if (ret < 0)
break;
ath9k_set_moredata(sc, bf, true);
@@ -1979,11 +1913,11 @@ void ath_tx_cleanupq(struct ath_softc *sc, struct ath_txq *txq)
*/
void ath_txq_schedule(struct ath_softc *sc, struct ath_txq *txq)
{
+ struct 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);
+ int ret;
if (txq->mac80211_qnum < 0)
return;
@@ -1991,58 +1925,26 @@ void ath_txq_schedule(struct ath_softc *sc, struct ath_txq *txq)
if (test_bit(ATH_OP_HW_RESET, &common->op_flags))
return;
+ ieee80211_txq_schedule_start(hw, txq->mac80211_qnum);
spin_lock_bh(&sc->chan_lock);
rcu_read_lock();
- acq = &sc->cur_chan->acq[txq->mac80211_qnum];
if (sc->cur_chan->stopped)
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;
- }
+ while ((queue = ieee80211_next_txq(hw, txq->mac80211_qnum))) {
+ tid = (struct ath_atx_tid *) queue->drv_priv;
+ ret = ath_tx_sched_aggr(sc, txq, tid);
+ ath_dbg(common, QUEUE, "ath_tx_sched_aggr returned %d\n", ret);
- /*
- * 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;
+ ieee80211_return_txq(hw, queue);
}
out:
rcu_read_unlock();
spin_unlock_bh(&sc->chan_lock);
+ ieee80211_txq_schedule_end(hw, txq->mac80211_qnum);
}
void ath_txq_schedule_all(struct ath_softc *sc)
@@ -2886,9 +2788,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;
@@ -2898,7 +2797,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);
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH RFC v5 3/4] mac80211: Add airtime accounting and scheduling to TXQs
2018-10-09 12:32 [PATCH RFC v5 0/4] Move TXQ scheduling and airtime fairness into mac80211 Toke Høiland-Jørgensen
` (2 preceding siblings ...)
2018-10-09 12:32 ` [PATCH RFC v5 4/4] ath9k: Switch to mac80211 TXQ scheduling and airtime APIs Toke Høiland-Jørgensen
@ 2018-10-09 12:32 ` Toke Høiland-Jørgensen
2018-10-10 4:52 ` Rajkumar Manoharan
3 siblings, 1 reply; 14+ messages in thread
From: Toke Høiland-Jørgensen @ 2018-10-09 12:32 UTC (permalink / raw)
To: linux-wireless; +Cc: make-wifi-fast, Felix Fietkau, Rajkumar Manoharan, Kan Yan
This adds airtime accounting and scheduling to the mac80211 TXQ
scheduler. A new callback, ieee80211_sta_register_airtime(), is added
that drivers can call to report airtime usage for stations.
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. If no airtime usage is reported
by the driver, the scheduler will default to round-robin scheduling.
For drivers that don't control TXQ scheduling in software, a new API
function, ieee80211_txq_may_transmit(), is added which the driver can use
to check if the TXQ is eligible for transmission, or should be throttled to
enforce fairness. Calls to this function must also be enclosed in
ieee80211_txq_schedule_{start,end}() calls to ensure proper locking. TXQs
that are throttled by ieee802111_txq_may_transmit() will be woken up again
by a check added to the ieee80211_wake_txqs() tasklet.
Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
---
include/net/mac80211.h | 52 +++++++++++++++++++++++++++
net/mac80211/cfg.c | 3 ++
net/mac80211/debugfs.c | 3 ++
net/mac80211/debugfs_sta.c | 51 +++++++++++++++++++++++++-
net/mac80211/ieee80211_i.h | 5 +++
net/mac80211/main.c | 6 +++
net/mac80211/sta_info.c | 52 +++++++++++++++++++++++++--
net/mac80211/sta_info.h | 13 +++++++
net/mac80211/status.c | 6 +++
net/mac80211/tx.c | 86 ++++++++++++++++++++++++++++++++++++++++++--
net/mac80211/util.c | 75 ++++++++++++++++++++++++++++++++++++++
11 files changed, 341 insertions(+), 11 deletions(-)
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 469e88a32f94..fa10420a53ff 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -5349,6 +5349,34 @@ void ieee80211_sta_eosp(struct ieee80211_sta *pubsta);
*/
void ieee80211_send_eosp_nullfunc(struct ieee80211_sta *pubsta, int tid);
+/**
+ * ieee80211_sta_register_airtime - register airtime usage for a sta/tid
+ *
+ * Register airtime usage for a given sta on a given tid. The driver can call
+ * this function to notify mac80211 that a station used a certain amount of
+ * airtime. This information will be used by the TXQ scheduler to schedule
+ * stations in a way that ensures airtime fairness.
+ *
+ * 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.
+ *
+ * The driver can either call this function synchronously for every packet or
+ * aggregate, or asynchronously as airtime usage information becomes available.
+ * TX and RX airtime can be reported together, or separately by setting one of
+ * them to 0.
+ *
+ * @pubsta: the station
+ * @tid: the TID to register airtime for
+ * @tx_airtime: airtime used during TX (in usec)
+ * @rx_airtime: airtime used during RX (in usec)
+ */
+void ieee80211_sta_register_airtime(struct ieee80211_sta *pubsta, u8 tid,
+ u32 tx_airtime, u32 rx_airtime);
+
/**
* ieee80211_iter_keys - iterate keys programmed into the device
* @hw: pointer obtained from ieee80211_alloc_hw()
@@ -6105,6 +6133,30 @@ void ieee80211_txq_schedule_start(struct ieee80211_hw *hw, u8 ac);
*/
void ieee80211_txq_schedule_end(struct ieee80211_hw *hw, u8 ac);
+/**
+ * ieee80211_txq_may_transmit - check whether TXQ is allowed to transmit
+ *
+ * This function is used to check whether given txq is allowed to transmit by
+ * the airtime scheduler, and can be used by drivers to access the airtime
+ * fairness accounting without going using the scheduling order enfored by
+ * next_txq().
+ *
+ * Returns %true if the airtime scheduler thinks the TXQ should be allowed to
+ * transmit, and %false if it should be throttled. This function can also have
+ * the side effect of rotating the TXQ in the scheduler rotation, which will
+ * eventually bring the deficit to positive and allow the station to transmit
+ * again. If a TXQ is throttled, it will be marked and eventually woken up again
+ * through drv_wake_tx_queue().
+ *
+ * If this function returns %true, the driver is expected to schedule packets
+ * for transmission, and then return the TXQ through ieee80211_return_txq().
+ *
+ * @hw: pointer as obtained from ieee80211_alloc_hw()
+ * @txq: pointer obtained from station or virtual interface
+ */
+bool ieee80211_txq_may_transmit(struct ieee80211_hw *hw,
+ struct ieee80211_txq *txq);
+
/**
* ieee80211_txq_get_depth - get pending frame/byte count of given txq
*
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 504627e2117f..c640d3ee5f04 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1388,6 +1388,9 @@ static int sta_apply_parameters(struct ieee80211_local *local,
if (ieee80211_vif_is_mesh(&sdata->vif))
sta_apply_mesh_params(local, sta, params);
+ if (params->airtime_weight)
+ sta->airtime_weight = params->airtime_weight;
+
/* set the STA state after all sta info from usermode has been set */
if (test_sta_flag(sta, WLAN_STA_TDLS_PEER) ||
set & BIT(NL80211_STA_FLAG_ASSOCIATED)) {
diff --git a/net/mac80211/debugfs.c b/net/mac80211/debugfs.c
index 3fe541e358f3..81c5fec2eae7 100644
--- a/net/mac80211/debugfs.c
+++ b/net/mac80211/debugfs.c
@@ -383,6 +383,9 @@ void debugfs_hw_add(struct ieee80211_local *local)
if (local->ops->wake_tx_queue)
DEBUGFS_ADD_MODE(aqm, 0600);
+ debugfs_create_u16("airtime_flags", 0600,
+ phyd, &local->airtime_flags);
+
statsd = debugfs_create_dir("statistics", phyd);
/* if the dir failed, don't put all the other things into the root! */
diff --git a/net/mac80211/debugfs_sta.c b/net/mac80211/debugfs_sta.c
index af5185a836e5..701c532d95a6 100644
--- a/net/mac80211/debugfs_sta.c
+++ b/net/mac80211/debugfs_sta.c
@@ -181,9 +181,10 @@ static ssize_t sta_aqm_read(struct file *file, char __user *userbuf,
txqi->tin.tx_bytes,
txqi->tin.tx_packets,
txqi->flags,
- txqi->flags & (1<<IEEE80211_TXQ_STOP) ? "STOP" : "RUN",
- txqi->flags & (1<<IEEE80211_TXQ_AMPDU) ? " AMPDU" : "",
- txqi->flags & (1<<IEEE80211_TXQ_NO_AMSDU) ? " NO-AMSDU" : "");
+ test_bit(IEEE80211_TXQ_STOP, &txqi->flags) ? "STOP" :
+ test_bit(IEEE80211_TXQ_AIRTIME_THROTTLE, &txqi->flags) ? "THROTTLE" : "RUN",
+ test_bit(IEEE80211_TXQ_AMPDU, &txqi->flags) ? " AMPDU" : "",
+ test_bit(IEEE80211_TXQ_NO_AMSDU, &txqi->flags) ? " NO-AMSDU" : "");
}
rcu_read_unlock();
@@ -195,6 +196,46 @@ 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;
+ struct ieee80211_local *local = sta->sdata->local;
+ size_t bufsz = 200;
+ char *buf = kzalloc(bufsz, GFP_KERNEL), *p = buf;
+ u64 rx_airtime = 0, tx_airtime = 0;
+ s64 deficit[IEEE80211_NUM_ACS];
+ ssize_t rv;
+ int ac;
+
+ if (!buf)
+ return -ENOMEM;
+
+ for (ac = 0; ac < IEEE80211_NUM_ACS; ac++) {
+ spin_lock_bh(&local->active_txq_lock[ac]);
+ rx_airtime += sta->airtime[ac].rx_airtime;
+ tx_airtime += sta->airtime[ac].tx_airtime;
+ deficit[ac] = sta->airtime[ac].deficit;
+ spin_unlock_bh(&local->active_txq_lock[ac]);
+ }
+
+ p += scnprintf(p, bufsz + buf - p,
+ "RX: %llu us\nTX: %llu us\nWeight: %u\n"
+ "Deficit: VO: %lld us VI: %lld us BE: %lld us BK: %lld us\n",
+ rx_airtime,
+ tx_airtime,
+ sta->airtime_weight,
+ deficit[0],
+ deficit[1],
+ deficit[2],
+ deficit[3]);
+
+ 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)
{
@@ -906,6 +947,10 @@ void ieee80211_sta_debugfs_add(struct sta_info *sta)
if (local->ops->wake_tx_queue)
DEBUGFS_ADD(aqm);
+ if (wiphy_ext_feature_isset(local->hw.wiphy,
+ NL80211_EXT_FEATURE_AIRTIME_FAIRNESS))
+ 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 976531717902..21040d9ad693 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -819,6 +819,7 @@ enum txq_info_flags {
IEEE80211_TXQ_AMPDU,
IEEE80211_TXQ_NO_AMSDU,
IEEE80211_TXQ_STOP_NETIF_TX,
+ IEEE80211_TXQ_AIRTIME_THROTTLE,
};
/**
@@ -1136,6 +1137,8 @@ struct ieee80211_local {
struct list_head active_txqs[IEEE80211_NUM_ACS];
u16 schedule_round[IEEE80211_NUM_ACS];
+ u16 airtime_flags;
+
const struct ieee80211_ops *ops;
/*
@@ -1240,6 +1243,7 @@ struct ieee80211_local {
struct sk_buff_head pending[IEEE80211_MAX_QUEUES];
struct tasklet_struct tx_pending_tasklet;
struct tasklet_struct wake_txqs_tasklet;
+ struct tasklet_struct airtime_tasklet;
atomic_t agg_queue_stop[IEEE80211_MAX_QUEUES];
@@ -2053,6 +2057,7 @@ void ieee80211_txq_remove_vlan(struct ieee80211_local *local,
void ieee80211_fill_txq_stats(struct cfg80211_txq_stats *txqstats,
struct txq_info *txqi);
void ieee80211_wake_txqs(unsigned long data);
+void ieee80211_kick_airtime(unsigned long data);
void ieee80211_send_auth(struct ieee80211_sub_if_data *sdata,
u16 transaction, u16 auth_alg, u16 status,
const u8 *extra, size_t extra_len, const u8 *bssid,
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index d9315de90b48..cfa0b7936e14 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -667,6 +667,7 @@ struct ieee80211_hw *ieee80211_alloc_hw_nm(size_t priv_data_len,
INIT_LIST_HEAD(&local->active_txqs[i]);
spin_lock_init(&local->active_txq_lock[i]);
}
+ local->airtime_flags = AIRTIME_USE_TX | AIRTIME_USE_RX;
INIT_LIST_HEAD(&local->chanctx_list);
mutex_init(&local->chanctx_mtx);
@@ -702,9 +703,12 @@ struct ieee80211_hw *ieee80211_alloc_hw_nm(size_t priv_data_len,
tasklet_init(&local->tx_pending_tasklet, ieee80211_tx_pending,
(unsigned long)local);
- if (ops->wake_tx_queue)
+ if (ops->wake_tx_queue) {
tasklet_init(&local->wake_txqs_tasklet, ieee80211_wake_txqs,
(unsigned long)local);
+ tasklet_init(&local->airtime_tasklet, ieee80211_kick_airtime,
+ (unsigned long)local);
+ }
tasklet_init(&local->tasklet,
ieee80211_tasklet_handler,
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index c2f5cb7df54f..98b7e9239df1 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -90,7 +90,6 @@ static void __cleanup_single_sta(struct sta_info *sta)
struct tid_ampdu_tx *tid_tx;
struct ieee80211_sub_if_data *sdata = sta->sdata;
struct ieee80211_local *local = sdata->local;
- struct fq *fq = &local->fq;
struct ps_data *ps;
if (test_sta_flag(sta, WLAN_STA_PS_STA) ||
@@ -120,9 +119,7 @@ static void __cleanup_single_sta(struct sta_info *sta)
txqi = to_txq_info(sta->sta.txq[i]);
- spin_lock_bh(&fq->lock);
ieee80211_txq_purge(local, txqi);
- spin_unlock_bh(&fq->lock);
}
}
@@ -387,9 +384,12 @@ struct sta_info *sta_info_alloc(struct ieee80211_sub_if_data *sdata,
if (sta_prepare_rate_control(local, sta, gfp))
goto free_txq;
+ sta->airtime_weight = IEEE80211_DEFAULT_AIRTIME_WEIGHT;
+
for (i = 0; i < IEEE80211_NUM_ACS; i++) {
skb_queue_head_init(&sta->ps_tx_buf[i]);
skb_queue_head_init(&sta->tx_filtered[i]);
+ sta->airtime[i].deficit = sta->airtime_weight;
}
for (i = 0; i < IEEE80211_NUM_TIDS; i++)
@@ -1826,6 +1826,35 @@ void ieee80211_sta_set_buffered(struct ieee80211_sta *pubsta,
}
EXPORT_SYMBOL(ieee80211_sta_set_buffered);
+void ieee80211_sta_register_airtime(struct ieee80211_sta *pubsta, u8 tid,
+ u32 tx_airtime, u32 rx_airtime)
+{
+ struct sta_info *sta = container_of(pubsta, struct sta_info, sta);
+ struct ieee80211_local *local = sta->sdata->local;
+ struct txq_info *txqi;
+ u8 ac = ieee80211_ac_from_tid(tid);
+ u32 airtime = 0;
+
+ if (sta->local->airtime_flags & AIRTIME_USE_TX)
+ airtime += tx_airtime;
+ if (sta->local->airtime_flags & AIRTIME_USE_RX)
+ airtime += rx_airtime;
+
+ spin_lock_bh(&local->active_txq_lock[ac]);
+ sta->airtime[ac].tx_airtime += tx_airtime;
+ sta->airtime[ac].rx_airtime += rx_airtime;
+ sta->airtime[ac].deficit -= airtime;
+
+ if (sta->airtime[ac].deficit < 0) {
+ txqi = to_txq_info(pubsta->txq[tid]);
+ if (list_empty(&txqi->schedule_order))
+ list_add_tail(&txqi->schedule_order,
+ &local->active_txqs[ac]);
+ }
+ spin_unlock_bh(&local->active_txq_lock[ac]);
+}
+EXPORT_SYMBOL(ieee80211_sta_register_airtime);
+
int sta_info_move_state(struct sta_info *sta,
enum ieee80211_sta_state new_state)
{
@@ -2188,6 +2217,23 @@ void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo,
sinfo->filled |= BIT_ULL(NL80211_STA_INFO_TX_FAILED);
}
+ if (!(sinfo->filled & BIT(NL80211_STA_INFO_RX_DURATION))) {
+ for (ac = 0; ac < IEEE80211_NUM_ACS; ac++)
+ sinfo->rx_duration += sta->airtime[ac].rx_airtime;
+ sinfo->filled |= BIT(NL80211_STA_INFO_RX_DURATION);
+ }
+
+ if (!(sinfo->filled & BIT(NL80211_STA_INFO_TX_DURATION))) {
+ for (ac = 0; ac < IEEE80211_NUM_ACS; ac++)
+ sinfo->tx_duration += sta->airtime[ac].tx_airtime;
+ sinfo->filled |= BIT(NL80211_STA_INFO_TX_DURATION);
+ }
+
+ if (!(sinfo->filled & BIT(NL80211_STA_INFO_AIRTIME_WEIGHT))) {
+ sinfo->airtime_weight = sta->airtime_weight;
+ sinfo->filled |= BIT(NL80211_STA_INFO_AIRTIME_WEIGHT);
+ }
+
sinfo->rx_dropped_misc = sta->rx_stats.dropped;
if (sta->pcpu_rx_stats) {
for_each_possible_cpu(cpu) {
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 9a04327d71d1..b1b0fd6a2e21 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -127,6 +127,16 @@ enum ieee80211_agg_stop_reason {
AGG_STOP_DESTROY_STA,
};
+/* Debugfs flags to enable/disable use of RX/TX airtime in scheduler */
+#define AIRTIME_USE_TX BIT(0)
+#define AIRTIME_USE_RX BIT(1)
+
+struct airtime_info {
+ u64 rx_airtime;
+ u64 tx_airtime;
+ s64 deficit;
+};
+
struct sta_info;
/**
@@ -563,6 +573,9 @@ struct sta_info {
} tx_stats;
u16 tid_seq[IEEE80211_QOS_CTL_TID_MASK + 1];
+ struct airtime_info airtime[IEEE80211_NUM_ACS];
+ u16 airtime_weight;
+
/*
* Aggregation information, locked with lock.
*/
diff --git a/net/mac80211/status.c b/net/mac80211/status.c
index 9a6d7208bf4f..664379797c46 100644
--- a/net/mac80211/status.c
+++ b/net/mac80211/status.c
@@ -821,6 +821,12 @@ static void __ieee80211_tx_status(struct ieee80211_hw *hw,
ieee80211_sta_tx_notify(sta->sdata, (void *) skb->data,
acked, info->status.tx_time);
+ if (info->status.tx_time &&
+ wiphy_ext_feature_isset(local->hw.wiphy,
+ NL80211_EXT_FEATURE_AIRTIME_FAIRNESS))
+ ieee80211_sta_register_airtime(&sta->sta, tid,
+ info->status.tx_time, 0);
+
if (ieee80211_hw_check(&local->hw, REPORTS_TX_ACK_STATUS)) {
if (info->flags & IEEE80211_TX_STAT_ACK) {
if (sta->status_stats.lost_packets)
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 1e071121cb44..e86d3e464523 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1484,8 +1484,11 @@ void ieee80211_txq_purge(struct ieee80211_local *local,
struct fq *fq = &local->fq;
struct fq_tin *tin = &txqi->tin;
+ spin_lock_bh(&fq->lock);
fq_tin_reset(fq, tin, fq_skb_free_func);
ieee80211_purge_tx_queue(&local->hw, &txqi->frags);
+ spin_unlock_bh(&fq->lock);
+
spin_lock_bh(&local->active_txq_lock[txqi->txq.ac]);
list_del_init(&txqi->schedule_order);
spin_unlock_bh(&local->active_txq_lock[txqi->txq.ac]);
@@ -3634,11 +3637,27 @@ struct ieee80211_txq *ieee80211_next_txq(struct ieee80211_hw *hw, u8 ac)
lockdep_assert_held(&local->active_txq_lock[ac]);
+ begin:
txqi = list_first_entry_or_null(&local->active_txqs[ac],
struct txq_info,
schedule_order);
+ if (!txqi)
+ return NULL;
+
+ if (txqi->txq.sta) {
+ struct sta_info *sta = container_of(txqi->txq.sta,
+ struct sta_info, sta);
+
+ if (sta->airtime[txqi->txq.ac].deficit < 0) {
+ sta->airtime[txqi->txq.ac].deficit += sta->airtime_weight;
+ list_move_tail(&txqi->schedule_order,
+ &local->active_txqs[txqi->txq.ac]);
+ goto begin;
+ }
+ }
+
- if (!txqi || txqi->schedule_round == local->schedule_round[ac])
+ if (txqi->schedule_round == local->schedule_round[ac])
return NULL;
list_del_init(&txqi->schedule_order);
@@ -3656,13 +3675,71 @@ void ieee80211_return_txq(struct ieee80211_hw *hw,
lockdep_assert_held(&local->active_txq_lock[txq->ac]);
if (list_empty(&txqi->schedule_order) &&
- (!skb_queue_empty(&txqi->frags) || txqi->tin.backlog_packets))
- list_add_tail(&txqi->schedule_order,
- &local->active_txqs[txq->ac]);
+ (!skb_queue_empty(&txqi->frags) || txqi->tin.backlog_packets)) {
+ /* If airtime accounting is active, always enqueue STAs at the
+ * head of the list to ensure that they only get moved to the
+ * back by the airtime DRR scheduler once they have a negative
+ * deficit. A station that already has a negative deficit will
+ * get immediately moved to the back of the list on the next
+ * call to ieee80211_next_txq().
+ */
+ if (wiphy_ext_feature_isset(local->hw.wiphy,
+ NL80211_EXT_FEATURE_AIRTIME_FAIRNESS)
+ && txqi->txq.sta)
+ list_add(&txqi->schedule_order,
+ &local->active_txqs[txq->ac]);
+ else
+ list_add_tail(&txqi->schedule_order,
+ &local->active_txqs[txq->ac]);
+ }
}
EXPORT_SYMBOL(ieee80211_return_txq);
+bool ieee80211_txq_may_transmit(struct ieee80211_hw *hw,
+ struct ieee80211_txq *txq)
+{
+ struct ieee80211_local *local = hw_to_local(hw);
+ struct txq_info *txqi = to_txq_info(txq);
+ struct sta_info *sta;
+ bool ret = false;
+
+ lockdep_assert_held(&local->active_txq_lock[txqi->txq.ac]);
+
+ if (!txqi->txq.sta) {
+ ret = true;
+ goto out;
+ }
+
+ sta = container_of(txqi->txq.sta, struct sta_info, sta);
+ if (sta->airtime[txqi->txq.ac].deficit >= 0) {
+ ret = true;
+ goto out;
+ }
+
+ /* Not currently eligible, but if the txq is first in the scheduler
+ * queue, increase deficit and rotate queues so it may be eligible
+ * next time.
+ */
+ if (txqi == list_first_entry(&local->active_txqs[txqi->txq.ac],
+ struct txq_info,
+ schedule_order)) {
+ sta->airtime[txqi->txq.ac].deficit += sta->airtime_weight;
+ list_move_tail(&txqi->schedule_order,
+ &local->active_txqs[txqi->txq.ac]);
+ }
+
+ out:
+ if (ret) {
+ clear_bit(IEEE80211_TXQ_AIRTIME_THROTTLE, &txqi->flags);
+ list_del_init(&txqi->schedule_order);
+ } else
+ set_bit(IEEE80211_TXQ_AIRTIME_THROTTLE, &txqi->flags);
+
+ return ret;
+}
+EXPORT_SYMBOL(ieee80211_txq_may_transmit);
+
void ieee80211_txq_schedule_start(struct ieee80211_hw *hw, u8 ac)
{
struct ieee80211_local *local = hw_to_local(hw);
@@ -3677,6 +3754,7 @@ void ieee80211_txq_schedule_end(struct ieee80211_hw *hw, u8 ac)
struct ieee80211_local *local = hw_to_local(hw);
spin_unlock_bh(&local->active_txq_lock[ac]);
+ tasklet_schedule(&local->airtime_tasklet);
}
EXPORT_SYMBOL(ieee80211_txq_schedule_end);
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 36a3c2ada515..bcad2298a975 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -240,6 +240,77 @@ __le16 ieee80211_ctstoself_duration(struct ieee80211_hw *hw,
}
EXPORT_SYMBOL(ieee80211_ctstoself_duration);
+static bool __ieee80211_kick_airtime(struct ieee80211_local *local, int ac)
+{
+ struct txq_info *txqi = NULL;
+ bool reschedule = false;
+ struct sta_info *sta;
+
+ spin_lock_bh(&local->active_txq_lock[ac]);
+
+ begin:
+ if (list_empty(&local->active_txqs[ac]))
+ goto out;
+
+ list_for_each_entry(txqi, &local->active_txqs[ac], schedule_order) {
+ /* If we find a non-sta TXQ before an eligible STA-TXQ, schedule
+ * this */
+ if (!txqi->txq.sta)
+ goto out;
+
+ sta = container_of(txqi->txq.sta, struct sta_info, sta);
+
+ if (sta->removed) {
+ reschedule = true;
+ txqi = NULL;
+ goto out;
+ }
+
+ if (sta->airtime[ac].deficit >= 0) {
+ if (!test_and_clear_bit(IEEE80211_TXQ_AIRTIME_THROTTLE,
+ &txqi->flags))
+ continue;
+
+ goto out;
+ }
+ }
+
+ /* We made it through the list without finding an eligible TXQ, which
+ * means TX is stuck; increase all deficits to get things unstuck, then
+ * start over.
+ */
+ list_for_each_entry(txqi, &local->active_txqs[ac], schedule_order) {
+ sta = container_of(txqi->txq.sta, struct sta_info, sta);
+ sta->airtime[ac].deficit += sta->airtime_weight;
+ }
+ goto begin;
+
+ out:
+ spin_unlock_bh(&local->active_txq_lock[ac]);
+ if (txqi)
+ drv_wake_tx_queue(local, txqi);
+
+ return reschedule;
+}
+
+void ieee80211_kick_airtime(unsigned long data)
+{
+ struct ieee80211_local *local = (struct ieee80211_local *)data;
+ bool reschedule = false;
+ int i;
+
+ rcu_read_lock();
+
+ for (i = 0; i < IEEE80211_NUM_ACS; i++)
+ if (__ieee80211_kick_airtime(local, i))
+ reschedule = true;
+
+ rcu_read_unlock();
+
+ if (reschedule)
+ tasklet_schedule(&local->airtime_tasklet);
+}
+
static void __ieee80211_wake_txqs(struct ieee80211_sub_if_data *sdata, int ac)
{
struct ieee80211_local *local = sdata->local;
@@ -250,6 +321,10 @@ static void __ieee80211_wake_txqs(struct ieee80211_sub_if_data *sdata, int ac)
struct sta_info *sta;
int i;
+ if (wiphy_ext_feature_isset(local->hw.wiphy,
+ NL80211_EXT_FEATURE_AIRTIME_FAIRNESS))
+ __ieee80211_kick_airtime(local, ac);
+
spin_lock_bh(&fq->lock);
if (sdata->vif.type == NL80211_IFTYPE_AP)
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH RFC v5 3/4] mac80211: Add airtime accounting and scheduling to TXQs
2018-10-09 12:32 ` [PATCH RFC v5 3/4] mac80211: Add airtime accounting and scheduling to TXQs Toke Høiland-Jørgensen
@ 2018-10-10 4:52 ` Rajkumar Manoharan
2018-10-10 11:15 ` Toke Høiland-Jørgensen
0 siblings, 1 reply; 14+ messages in thread
From: Rajkumar Manoharan @ 2018-10-10 4:52 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: linux-wireless, make-wifi-fast, Felix Fietkau, Kan Yan
On 2018-10-09 05:32, Toke Høiland-Jørgensen wrote:
> This adds airtime accounting and scheduling to the mac80211 TXQ
> scheduler. A new callback, ieee80211_sta_register_airtime(), is added
> that drivers can call to report airtime usage for stations.
>
> 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. If no airtime usage is reported
> by the driver, the scheduler will default to round-robin scheduling.
>
> For drivers that don't control TXQ scheduling in software, a new API
> function, ieee80211_txq_may_transmit(), is added which the driver can
> use
> to check if the TXQ is eligible for transmission, or should be
> throttled to
> enforce fairness. Calls to this function must also be enclosed in
> ieee80211_txq_schedule_{start,end}() calls to ensure proper locking.
> TXQs
> that are throttled by ieee802111_txq_may_transmit() will be woken up
> again
> by a check added to the ieee80211_wake_txqs() tasklet.
>
Toke,
I am observing soft lockup issues again with this new series while
running
traffic with 50 clients. I am continuing testing with earlier series
along with
snippet I shared. When driver operates in pull-mode, throttled txqs are
marked
and refilled in airtime_tasklet. This is causing major throughput drops
and
packet loss and I am suspecting the latency in replenishing deficit.
Whereas in push-mode or in ath9k model, refill happens quicker at every
packet
indication as well as tx completion.
I am planning to get rid of tasklet completely as it is only meant for
pull-mode.
It would be better to refill in may_transmit() itself.
-Rajkumar
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH RFC v5 3/4] mac80211: Add airtime accounting and scheduling to TXQs
2018-10-10 4:52 ` Rajkumar Manoharan
@ 2018-10-10 11:15 ` Toke Høiland-Jørgensen
2018-10-10 23:20 ` Rajkumar Manoharan
0 siblings, 1 reply; 14+ messages in thread
From: Toke Høiland-Jørgensen @ 2018-10-10 11:15 UTC (permalink / raw)
To: Rajkumar Manoharan; +Cc: linux-wireless, make-wifi-fast, Felix Fietkau, Kan Yan
Rajkumar Manoharan <rmanohar@codeaurora.org> writes:
> On 2018-10-09 05:32, Toke Høiland-Jørgensen wrote:
>> This adds airtime accounting and scheduling to the mac80211 TXQ
>> scheduler. A new callback, ieee80211_sta_register_airtime(), is added
>> that drivers can call to report airtime usage for stations.
>>
>> 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. If no airtime usage is reported
>> by the driver, the scheduler will default to round-robin scheduling.
>>
>> For drivers that don't control TXQ scheduling in software, a new API
>> function, ieee80211_txq_may_transmit(), is added which the driver can
>> use
>> to check if the TXQ is eligible for transmission, or should be
>> throttled to
>> enforce fairness. Calls to this function must also be enclosed in
>> ieee80211_txq_schedule_{start,end}() calls to ensure proper locking.
>> TXQs
>> that are throttled by ieee802111_txq_may_transmit() will be woken up
>> again
>> by a check added to the ieee80211_wake_txqs() tasklet.
>>
>
> Toke,
>
> I am observing soft lockup issues again with this new series while
> running traffic with 50 clients. I am continuing testing with earlier
> series along with snippet I shared.
Are these new lockups (that was not in your patched previous version),
or did I just not get all your lock-related fixes incorporated?
> When driver operates in pull-mode, throttled txqs are marked and
> refilled in airtime_tasklet. This is causing major throughput drops
> and packet loss and I am suspecting the latency in replenishing
> deficit. Whereas in push-mode or in ath9k model, refill happens
> quicker at every packet indication as well as tx completion.
Yeah, the tasklet shouldn't be the main source of deficit replenishing.
Can see why that would give bad performance :)
> I am planning to get rid of tasklet completely as it is only meant for
> pull-mode. It would be better to refill in may_transmit() itself.
Hmm, right. So the way to do this correctly (from a fairness point of
view) would be something like this (in max_tx()):
if (this_txq.stn.deficit > 0)
return true;
else if (any queued TXQ currently have positive deficit)
return false; /* other TXQ should try may_tx() later and get permission */
else /* all deficits < 0 */
return replenish_deficits(this_txq);
And replenish_deficits() would be something like:
replenish_deficits(this_txq) {
repeat:
for (txq in queued txqs) {
txq.stn.deficit += stn.weight;
if (txq.stn.deficit > 0 && !wake_txq)
wake_txq = txq;
}
if not wake_txq:
goto repeat;
if (this_txq.stn.deficit > 0)
return true;
else
drv_wake_tx_queue(wake_txq);
}
The wake_tx_queue call may have to be delegated to a tasklet still, to
avoid the infinite recursion problem I mentioned earlier. But the
tasklet could be made simpler and wouldn't have to be called so often...
Does the above make sense?
-Toke
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH RFC v5 3/4] mac80211: Add airtime accounting and scheduling to TXQs
2018-10-10 11:15 ` Toke Høiland-Jørgensen
@ 2018-10-10 23:20 ` Rajkumar Manoharan
2018-10-11 10:38 ` Toke Høiland-Jørgensen
0 siblings, 1 reply; 14+ messages in thread
From: Rajkumar Manoharan @ 2018-10-10 23:20 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: linux-wireless, make-wifi-fast, Felix Fietkau, Kan Yan,
linux-wireless-owner
On 2018-10-10 04:15, Toke Høiland-Jørgensen wrote:
> Rajkumar Manoharan <rmanohar@codeaurora.org> writes:
>
>> On 2018-10-09 05:32, Toke Høiland-Jørgensen wrote:
>>> This adds airtime accounting and scheduling to the mac80211 TXQ
>>> scheduler. A new callback, ieee80211_sta_register_airtime(), is added
>>> that drivers can call to report airtime usage for stations.
>>>
>>> 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. If no airtime usage is
>>> reported
>>> by the driver, the scheduler will default to round-robin scheduling.
>>>
>>> For drivers that don't control TXQ scheduling in software, a new API
>>> function, ieee80211_txq_may_transmit(), is added which the driver can
>>> use
>>> to check if the TXQ is eligible for transmission, or should be
>>> throttled to
>>> enforce fairness. Calls to this function must also be enclosed in
>>> ieee80211_txq_schedule_{start,end}() calls to ensure proper locking.
>>> TXQs
>>> that are throttled by ieee802111_txq_may_transmit() will be woken up
>>> again
>>> by a check added to the ieee80211_wake_txqs() tasklet.
>>>
>>
>> Toke,
>>
>> I am observing soft lockup issues again with this new series while
>> running traffic with 50 clients. I am continuing testing with earlier
>> series along with snippet I shared.
>
> Are these new lockups (that was not in your patched previous version),
> or did I just not get all your lock-related fixes incorporated?
>
>> When driver operates in pull-mode, throttled txqs are marked and
>> refilled in airtime_tasklet. This is causing major throughput drops
>> and packet loss and I am suspecting the latency in replenishing
>> deficit. Whereas in push-mode or in ath9k model, refill happens
>> quicker at every packet indication as well as tx completion.
>
> Yeah, the tasklet shouldn't be the main source of deficit replenishing.
> Can see why that would give bad performance :)
>
>> I am planning to get rid of tasklet completely as it is only meant for
>> pull-mode. It would be better to refill in may_transmit() itself.
>
> Hmm, right. So the way to do this correctly (from a fairness point of
> view) would be something like this (in max_tx()):
>
> if (this_txq.stn.deficit > 0)
> return true;
>
> else if (any queued TXQ currently have positive deficit)
> return false; /* other TXQ should try may_tx() later and get
> permission */
>
> else /* all deficits < 0 */
> return replenish_deficits(this_txq);
>
> And replenish_deficits() would be something like:
>
> replenish_deficits(this_txq) {
> repeat:
> for (txq in queued txqs) {
> txq.stn.deficit += stn.weight;
> if (txq.stn.deficit > 0 && !wake_txq)
> wake_txq = txq;
> }
> if not wake_txq:
> goto repeat;
>
> if (this_txq.stn.deficit > 0)
> return true;
> else
> drv_wake_tx_queue(wake_txq);
> }
>
> The wake_tx_queue call may have to be delegated to a tasklet still, to
> avoid the infinite recursion problem I mentioned earlier. But the
> tasklet could be made simpler and wouldn't have to be called so
> often...
>
> Does the above make sense?
>
Hmm... mine is bit different. txqs are refilled only once for all txqs.
It will give more opportunity for non-served txqs. drv_wake_tx_queue
won't be
called from may_tx as the driver anyway will not push packets in
pull-mode.
bool ieee80211_txq_may_transmit(struct ieee80211_hw *hw,
struct ieee80211_txq *txq)
{
struct ieee80211_local *local = hw_to_local(hw);
struct txq_info *txqi = to_txq_info(txq);
struct sta_info *sta;
u8 ac = txq->ac;
lockdep_assert_held(&local->active_txq_lock[ac]);
if (!txqi->txq.sta)
goto out;
sta = container_of(txqi->txq.sta, struct sta_info, sta);
if (sta->airtime[ac].deficit >= 0)
goto out;
list_for_each_entry(txqi, &local->active_txqs[ac],
schedule_order) {
if (!txqi->txq.sta)
continue;
sta = container_of(txqi->txq.sta, struct sta_info, sta);
sta->airtime[ac].deficit += sta->airtime_weight;
}
return false;
out:
list_del_init(&txqi->schedule_order);
return true;
}
-Rajkumar
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH RFC v5 3/4] mac80211: Add airtime accounting and scheduling to TXQs
2018-10-10 23:20 ` Rajkumar Manoharan
@ 2018-10-11 10:38 ` Toke Høiland-Jørgensen
2018-10-12 7:38 ` Rajkumar Manoharan
0 siblings, 1 reply; 14+ messages in thread
From: Toke Høiland-Jørgensen @ 2018-10-11 10:38 UTC (permalink / raw)
To: Rajkumar Manoharan
Cc: linux-wireless, make-wifi-fast, Felix Fietkau, Kan Yan,
linux-wireless-owner
Rajkumar Manoharan <rmanohar@codeaurora.org> writes:
> On 2018-10-10 04:15, Toke Høiland-Jørgensen wrote:
>> Rajkumar Manoharan <rmanohar@codeaurora.org> writes:
>>
>>> On 2018-10-09 05:32, Toke Høiland-Jørgensen wrote:
>>>> This adds airtime accounting and scheduling to the mac80211 TXQ
>>>> scheduler. A new callback, ieee80211_sta_register_airtime(), is added
>>>> that drivers can call to report airtime usage for stations.
>>>>
>>>> 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. If no airtime usage is
>>>> reported
>>>> by the driver, the scheduler will default to round-robin scheduling.
>>>>
>>>> For drivers that don't control TXQ scheduling in software, a new API
>>>> function, ieee80211_txq_may_transmit(), is added which the driver can
>>>> use
>>>> to check if the TXQ is eligible for transmission, or should be
>>>> throttled to
>>>> enforce fairness. Calls to this function must also be enclosed in
>>>> ieee80211_txq_schedule_{start,end}() calls to ensure proper locking.
>>>> TXQs
>>>> that are throttled by ieee802111_txq_may_transmit() will be woken up
>>>> again
>>>> by a check added to the ieee80211_wake_txqs() tasklet.
>>>>
>>>
>>> Toke,
>>>
>>> I am observing soft lockup issues again with this new series while
>>> running traffic with 50 clients. I am continuing testing with earlier
>>> series along with snippet I shared.
>>
>> Are these new lockups (that was not in your patched previous version),
>> or did I just not get all your lock-related fixes incorporated?
>>
>>> When driver operates in pull-mode, throttled txqs are marked and
>>> refilled in airtime_tasklet. This is causing major throughput drops
>>> and packet loss and I am suspecting the latency in replenishing
>>> deficit. Whereas in push-mode or in ath9k model, refill happens
>>> quicker at every packet indication as well as tx completion.
>>
>> Yeah, the tasklet shouldn't be the main source of deficit replenishing.
>> Can see why that would give bad performance :)
>>
>>> I am planning to get rid of tasklet completely as it is only meant for
>>> pull-mode. It would be better to refill in may_transmit() itself.
>>
>> Hmm, right. So the way to do this correctly (from a fairness point of
>> view) would be something like this (in max_tx()):
>>
>> if (this_txq.stn.deficit > 0)
>> return true;
>>
>> else if (any queued TXQ currently have positive deficit)
>> return false; /* other TXQ should try may_tx() later and get
>> permission */
>>
>> else /* all deficits < 0 */
>> return replenish_deficits(this_txq);
>>
>> And replenish_deficits() would be something like:
>>
>> replenish_deficits(this_txq) {
>> repeat:
>> for (txq in queued txqs) {
>> txq.stn.deficit += stn.weight;
>> if (txq.stn.deficit > 0 && !wake_txq)
>> wake_txq = txq;
>> }
>> if not wake_txq:
>> goto repeat;
>>
>> if (this_txq.stn.deficit > 0)
>> return true;
>> else
>> drv_wake_tx_queue(wake_txq);
>> }
>>
>> The wake_tx_queue call may have to be delegated to a tasklet still, to
>> avoid the infinite recursion problem I mentioned earlier. But the
>> tasklet could be made simpler and wouldn't have to be called so
>> often...
>>
>> Does the above make sense?
>>
> Hmm... mine is bit different. txqs are refilled only once for all txqs.
> It will give more opportunity for non-served txqs. drv_wake_tx_queue
> won't be
> called from may_tx as the driver anyway will not push packets in
> pull-mode.
So, as far as I can tell, this requires the hardware to "keep trying"?
I.e., if it just stops scheduling a TXQ after may_transmit() returns
false, there is no guarantee that that TXQ will ever get re-awoken
unless a new packet arrives for it?
-Toke
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH RFC v5 3/4] mac80211: Add airtime accounting and scheduling to TXQs
2018-10-11 10:38 ` Toke Høiland-Jørgensen
@ 2018-10-12 7:38 ` Rajkumar Manoharan
2018-10-12 10:16 ` Toke Høiland-Jørgensen
2018-10-13 7:09 ` [Make-wifi-fast] " Dave Taht
0 siblings, 2 replies; 14+ messages in thread
From: Rajkumar Manoharan @ 2018-10-12 7:38 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: linux-wireless, make-wifi-fast, Felix Fietkau, Kan Yan,
linux-wireless-owner
On 2018-10-11 03:38, Toke Høiland-Jørgensen wrote:
> Rajkumar Manoharan <rmanohar@codeaurora.org> writes:
>
>> Hmm... mine is bit different. txqs are refilled only once for all
>> txqs.
>> It will give more opportunity for non-served txqs. drv_wake_tx_queue
>> won't be
>> called from may_tx as the driver anyway will not push packets in
>> pull-mode.
>
> So, as far as I can tell, this requires the hardware to "keep trying"?
> I.e., if it just stops scheduling a TXQ after may_transmit() returns
> false, there is no guarantee that that TXQ will ever get re-awoken
> unless a new packet arrives for it?
>
That is true and even now ath10k operates the same way in pull mode. Not
just packet arrival, even napi poll routine tries to pushes the packets.
One more thing, fetch indication may pull ~4ms/8ms of packets from each
tid.
This makes deficit too low and so refilling txqs by just airtime_weight
becomes
cumbersome. In may_transmit, the deficit are incremented by 20 *
airtime_weight.
In future this will be also replaced by station specific quantum. we can
revisit
this once BQL in place. Performance issue is resolved by this approach.
Do you foresee any issues?
#define IEEE80211_TXQ_MAY_TX_QUANTUM 20
bool ieee80211_txq_may_transmit(struct ieee80211_hw *hw,
struct ieee80211_txq *txq)
{
struct ieee80211_local *local = hw_to_local(hw);
struct txq_info *txqi = to_txq_info(txq);
struct sta_info *sta;
u8 ac = txq->ac;
lockdep_assert_held(&local->active_txq_lock[ac]);
if (!txqi->txq.sta)
goto out;
sta = container_of(txqi->txq.sta, struct sta_info, sta);
if (sta->airtime[ac].deficit >= 0)
goto out;
list_for_each_entry(txqi, &local->active_txqs[ac],
schedule_order) {
if (!txqi->txq.sta)
continue;
sta = container_of(txqi->txq.sta, struct sta_info, sta);
sta->airtime[ac].deficit +=
(IEEE80211_TXQ_MAY_TX_QUANTUM *
sta->airtime_weight);
}
return false;
out:
list_del_init(&txqi->schedule_order);
return true;
}
-Rajkumar
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH RFC v5 3/4] mac80211: Add airtime accounting and scheduling to TXQs
2018-10-12 7:38 ` Rajkumar Manoharan
@ 2018-10-12 10:16 ` Toke Høiland-Jørgensen
2018-10-16 7:07 ` Rajkumar Manoharan
2018-10-13 7:09 ` [Make-wifi-fast] " Dave Taht
1 sibling, 1 reply; 14+ messages in thread
From: Toke Høiland-Jørgensen @ 2018-10-12 10:16 UTC (permalink / raw)
To: Rajkumar Manoharan; +Cc: linux-wireless, make-wifi-fast, Felix Fietkau, Kan Yan
Rajkumar Manoharan <rmanohar@codeaurora.org> writes:
> On 2018-10-11 03:38, Toke Høiland-Jørgensen wrote:
>> Rajkumar Manoharan <rmanohar@codeaurora.org> writes:
>>
>>> Hmm... mine is bit different. txqs are refilled only once for all
>>> txqs.
>>> It will give more opportunity for non-served txqs. drv_wake_tx_queue
>>> won't be
>>> called from may_tx as the driver anyway will not push packets in
>>> pull-mode.
>>
>> So, as far as I can tell, this requires the hardware to "keep trying"?
>> I.e., if it just stops scheduling a TXQ after may_transmit() returns
>> false, there is no guarantee that that TXQ will ever get re-awoken
>> unless a new packet arrives for it?
>>
> That is true and even now ath10k operates the same way in pull mode.
> Not just packet arrival, even napi poll routine tries to pushes the
> packets.
I'm not sure I'm following? At every NAPI poll, the driver tries to push
to *all* TXQs?
> One more thing, fetch indication may pull ~4ms/8ms of packets from
> each tid. This makes deficit too low and so refilling txqs by just
> airtime_weight becomes cumbersome.
Yeah, in general we can't assume that each dequeue uses the same amount
of airtime as the quantum. This is why there's a loop; to fill up
quantum until the first stations gets into the positive.
> In may_transmit, the deficit are incremented by 20 * airtime_weight.
> In future this will be also replaced by station specific quantum. we
> can revisit this once BQL in place. Performance issue is resolved by
> this approach. Do you foresee any issues?
Just using a larger quantum will work as long as all stations send
roughly the same amount of data (airtime) at each transmission. Which is
often the case when you're benchmarking, but not in general. Think of
the size of the quantum as the granularity that the scheduler can
provide fairness at.
What I'd suggest is that instead of increasing the quantum, you do one
of the following:
- Just loop with the smaller quantum until one of the stations go into
the positive (what we do now).
- Go through all active stations, find the one that is closest being in
the positive, and add that amount to the quantum. I.e., something
like (assuming no station has positive deficit; if one does, you don't
want to add anything anyway):
to_add = -(max(stn.deficit) for stn in active stations)
for stn in active stations:
stn.deficit += to_add + stn.weight
-Toke
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Make-wifi-fast] [PATCH RFC v5 3/4] mac80211: Add airtime accounting and scheduling to TXQs
2018-10-12 7:38 ` Rajkumar Manoharan
2018-10-12 10:16 ` Toke Høiland-Jørgensen
@ 2018-10-13 7:09 ` Dave Taht
1 sibling, 0 replies; 14+ messages in thread
From: Dave Taht @ 2018-10-13 7:09 UTC (permalink / raw)
To: Rajkumar Manoharan
Cc: Toke Høiland-Jørgensen, Kan Yan, Make-Wifi-fast,
linux-wireless-owner, linux-wireless, Felix Fietkau
On Fri, Oct 12, 2018 at 12:38 AM Rajkumar Manoharan
<rmanohar@codeaurora.org> wrote:
>
> On 2018-10-11 03:38, Toke Høiland-Jørgensen wrote:
> > Rajkumar Manoharan <rmanohar@codeaurora.org> writes:
> >
> >> Hmm... mine is bit different. txqs are refilled only once for all
> >> txqs.
> >> It will give more opportunity for non-served txqs. drv_wake_tx_queue
> >> won't be
> >> called from may_tx as the driver anyway will not push packets in
> >> pull-mode.
> >
> > So, as far as I can tell, this requires the hardware to "keep trying"?
> > I.e., if it just stops scheduling a TXQ after may_transmit() returns
> > false, there is no guarantee that that TXQ will ever get re-awoken
> > unless a new packet arrives for it?
> >
> That is true and even now ath10k operates the same way in pull mode. Not
> just packet arrival, even napi poll routine tries to pushes the packets.
> One more thing, fetch indication may pull ~4ms/8ms of packets from each
> tid.
> This makes deficit too low and so refilling txqs by just airtime_weight
> becomes
> cumbersome. In may_transmit, the deficit are incremented by 20 *
> airtime_weight.
> In future this will be also replaced by station specific quantum. we can
> revisit
> this once BQL in place. Performance issue is resolved by this approach.
> Do you foresee any issues?
I'll have some time in the coming weeks to be able to test this stuff.
I'm mostly interested
in algorithmic correctness more than the API changes...
Is there a version of these patches that is stable enough on ath9 or ath10k?
Do I foresee any issues? Jeeze, no, we *never* have any issues with wifi.
"fetch indication may pull ~4ms/8ms of packets from each tid"
made me really twitchy.
>
> #define IEEE80211_TXQ_MAY_TX_QUANTUM 20
> bool ieee80211_txq_may_transmit(struct ieee80211_hw *hw,
> struct ieee80211_txq *txq)
> {
> struct ieee80211_local *local = hw_to_local(hw);
> struct txq_info *txqi = to_txq_info(txq);
> struct sta_info *sta;
> u8 ac = txq->ac;
>
> lockdep_assert_held(&local->active_txq_lock[ac]);
>
> if (!txqi->txq.sta)
> goto out;
>
> sta = container_of(txqi->txq.sta, struct sta_info, sta);
> if (sta->airtime[ac].deficit >= 0)
> goto out;
>
> list_for_each_entry(txqi, &local->active_txqs[ac],
> schedule_order) {
> if (!txqi->txq.sta)
> continue;
> sta = container_of(txqi->txq.sta, struct sta_info, sta);
> sta->airtime[ac].deficit +=
> (IEEE80211_TXQ_MAY_TX_QUANTUM *
> sta->airtime_weight);
> }
>
> return false;
>
> out:
> list_del_init(&txqi->schedule_order);
> return true;
> }
>
> -Rajkumar
> _______________________________________________
> Make-wifi-fast mailing list
> Make-wifi-fast@lists.bufferbloat.net
> https://lists.bufferbloat.net/listinfo/make-wifi-fast
--
Dave Täht
CTO, TekLibre, LLC
http://www.teklibre.com
Tel: 1-831-205-9740
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH RFC v5 3/4] mac80211: Add airtime accounting and scheduling to TXQs
2018-10-12 10:16 ` Toke Høiland-Jørgensen
@ 2018-10-16 7:07 ` Rajkumar Manoharan
2018-10-16 10:20 ` Toke Høiland-Jørgensen
0 siblings, 1 reply; 14+ messages in thread
From: Rajkumar Manoharan @ 2018-10-16 7:07 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: linux-wireless, make-wifi-fast, Felix Fietkau, Kan Yan,
linux-wireless-owner
[-- Attachment #1: Type: text/plain, Size: 2439 bytes --]
On 2018-10-12 03:16, Toke Høiland-Jørgensen wrote:
>
> - Just loop with the smaller quantum until one of the stations go into
> the positive (what we do now).
>
> - Go through all active stations, find the one that is closest being in
> the positive, and add that amount to the quantum. I.e., something
> like (assuming no station has positive deficit; if one does, you
> don't
> want to add anything anyway):
>
> to_add = -(max(stn.deficit) for stn in active stations)
> for stn in active stations:
> stn.deficit += to_add + stn.weight
>
Toke,
Sorry for the delayed response. I did lot of experiments. Below are my
observations.
Sorry for lengthy reply.
In current model, next_txq() is main routine that serves DRR and
fairness is
enforced by serving only only first txq. Here the first node could be
either
newly initiated traffic or returned node by return_txq(). This works
perfectly
as long as the driver is running any RR algo.
Whereas in ath10k, firmware runs its own RR in pull mode and builds txq
list
based on driver's hostq table. In this case it can not be simply assumed
that
firmware always gives fetch request for first node of mac80211's txq
list.
i.e both RR algo could be out of sync.
Two major differences b/w ath9k and ath10k
1) Serving txqs
The ath9k always serves txq by next_txq and so that the txqs_list is
rotated to serve
other txq. But in ath10k (pull-mode), first node becomes sticky one
until it is
picked by firmware via fetch indication and it becomes negative deficit.
The sequence is followed in wake_tx_queue
- dequeue first node
- push is not allowed
- enqueue same txq back to head
2) Refill rate of deficit.
The ath9k refills deficit mostly in hot path by next_txq() in tx & isr
routine.
In case of ath10k, due to above problem, deficits wont be filled in hot
path.
Either it should be filled in fetch_ind itself or by scheduling another
task.
Both the approaches are slower compared to hot path when the driver is
bursting
aggregation. On an idle condition a single fetch indication can dequeue
~190 msdus
from each tid of give stn list. This drains the deficit quickly and
becomes too low.
To speed up this, either refill the station by multiples of stn airtime
weight or
allows the txqs_list rotation. So that next_txq will be used for
refilling deficit.
Attaching return_txq() change that helps to get rid of quantum multiple.
-Rajkumar
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: return_tail.patch --]
[-- Type: text/x-diff; name=return_tail.patch, Size: 5268 bytes --]
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 625a4ab37ea0..269ae8311056 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -2352,7 +2352,7 @@ static void ath10k_htt_rx_tx_fetch_ind(struct ath10k *ar, struct sk_buff *skb)
num_msdus++;
num_bytes += ret;
}
- ieee80211_return_txq(hw, txq);
+ ieee80211_return_txq(hw, txq, true);
ieee80211_txq_schedule_end(hw, txq->ac);
record->num_msdus = cpu_to_le16(num_msdus);
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index cf64d9e02a24..d39bc841ea04 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -4206,7 +4206,7 @@ static int ath10k_mac_schedule_txq(struct ieee80211_hw *hw, u32 ac)
if (ret < 0)
break;
}
- ieee80211_return_txq(hw, txq);
+ ieee80211_return_txq(hw, txq, true);
ath10k_htt_tx_txq_update(hw, txq);
if (ret == -EBUSY)
break;
@@ -4475,18 +4475,21 @@ static void ath10k_mac_op_wake_tx_queue(struct ieee80211_hw *hw,
{
u8 ac = txq->ac;
int ret = 0;
+ bool pushed = false;
+ ath10k_htt_tx_txq_update(hw, txq);
ieee80211_txq_schedule_start(hw, ac);
txq = ieee80211_next_txq(hw, ac);
if (!txq)
goto out;
while (ath10k_mac_tx_can_push(hw, txq)) {
+ pushed = true;
ret = ath10k_mac_tx_push_txq(hw, txq);
if (ret < 0)
break;
}
- ieee80211_return_txq(hw, txq);
+ ieee80211_return_txq(hw, txq, pushed);
ath10k_htt_tx_txq_update(hw, txq);
out:
ieee80211_txq_schedule_end(hw, ac);
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index 6aab06909e76..40ff0bdbf7c9 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -117,7 +117,7 @@ void ath_tx_queue_tid(struct ath_softc *sc, struct ath_atx_tid *tid)
struct ieee80211_txq *queue = container_of(
(void *)tid, struct ieee80211_txq, drv_priv);
- ieee80211_return_txq(sc->hw, queue);
+ ieee80211_return_txq(sc->hw, queue, false);
}
void ath9k_wake_tx_queue(struct ieee80211_hw *hw, struct ieee80211_txq *queue)
@@ -1913,7 +1913,7 @@ void ath_txq_schedule(struct ath_softc *sc, struct ath_txq *txq)
ret = ath_tx_sched_aggr(sc, txq, tid);
ath_dbg(common, QUEUE, "ath_tx_sched_aggr returned %d\n", ret);
- ieee80211_return_txq(hw, queue);
+ ieee80211_return_txq(hw, queue, false);
}
out:
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 9cadfa408f50..995e19e29d9e 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -6102,7 +6102,7 @@ struct ieee80211_txq *ieee80211_next_txq(struct ieee80211_hw *hw, u8 ac);
* Should only be called between calls to ieee80211_txq_schedule_start()
* and ieee80211_txq_schedule_end().
*/
-void ieee80211_return_txq(struct ieee80211_hw *hw, struct ieee80211_txq *txq);
+void ieee80211_return_txq(struct ieee80211_hw *hw, struct ieee80211_txq *txqi, bool to_tail);
/**
* ieee80211_txq_schedule_start - acquire locks for safe scheduling of an AC
diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
index 39dc40ac5abe..530270f8cc52 100644
--- a/net/mac80211/driver-ops.h
+++ b/net/mac80211/driver-ops.h
@@ -1175,7 +1175,7 @@ static inline void schedule_and_wake_txq(struct ieee80211_local *local,
struct txq_info *txqi)
{
spin_lock_bh(&local->active_txq_lock[txqi->txq.ac]);
- ieee80211_return_txq(&local->hw, &txqi->txq);
+ ieee80211_return_txq(&local->hw, &txqi->txq, false);
spin_unlock_bh(&local->active_txq_lock[txqi->txq.ac]);
drv_wake_tx_queue(local, txqi);
}
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index d8682930a469..acf0505bdd7f 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -3623,7 +3623,8 @@ struct ieee80211_txq *ieee80211_next_txq(struct ieee80211_hw *hw, u8 ac)
EXPORT_SYMBOL(ieee80211_next_txq);
void ieee80211_return_txq(struct ieee80211_hw *hw,
- struct ieee80211_txq *txq)
+ struct ieee80211_txq *txq,
+ bool to_tail)
{
struct ieee80211_local *local = hw_to_local(hw);
struct txq_info *txqi = to_txq_info(txq);
@@ -3641,7 +3642,7 @@ void ieee80211_return_txq(struct ieee80211_hw *hw,
*/
if (wiphy_ext_feature_isset(local->hw.wiphy,
NL80211_EXT_FEATURE_AIRTIME_FAIRNESS)
- && txqi->txq.sta)
+ && txqi->txq.sta && !to_tail)
list_add(&txqi->schedule_order,
&local->active_txqs[txq->ac]);
else
@@ -3652,7 +3653,6 @@ void ieee80211_return_txq(struct ieee80211_hw *hw,
}
EXPORT_SYMBOL(ieee80211_return_txq);
-#define IEEE80211_TXQ_MAY_TX_QUANTUM 20
bool ieee80211_txq_may_transmit(struct ieee80211_hw *hw,
struct ieee80211_txq *txq)
{
@@ -3670,13 +3670,8 @@ bool ieee80211_txq_may_transmit(struct ieee80211_hw *hw,
if (sta->airtime[ac].deficit >= 0)
goto out;
- list_for_each_entry(txqi, &local->active_txqs[ac], schedule_order) {
- if (!txqi->txq.sta)
- continue;
- sta = container_of(txqi->txq.sta, struct sta_info, sta);
- sta->airtime[ac].deficit +=
- (IEEE80211_TXQ_MAY_TX_QUANTUM * sta->airtime_weight);
- }
+ sta->airtime[ac].deficit += sta->airtime_weight;
+ list_move_tail(&txqi->schedule_order, &local->active_txqs[ac]);
return false;
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH RFC v5 3/4] mac80211: Add airtime accounting and scheduling to TXQs
2018-10-16 7:07 ` Rajkumar Manoharan
@ 2018-10-16 10:20 ` Toke Høiland-Jørgensen
0 siblings, 0 replies; 14+ messages in thread
From: Toke Høiland-Jørgensen @ 2018-10-16 10:20 UTC (permalink / raw)
To: Rajkumar Manoharan
Cc: linux-wireless, make-wifi-fast, Felix Fietkau, Kan Yan,
linux-wireless-owner
Rajkumar Manoharan <rmanohar@codeaurora.org> writes:
> On 2018-10-12 03:16, Toke Høiland-Jørgensen wrote:
>>
>> - Just loop with the smaller quantum until one of the stations go into
>> the positive (what we do now).
>>
>> - Go through all active stations, find the one that is closest being in
>> the positive, and add that amount to the quantum. I.e., something
>> like (assuming no station has positive deficit; if one does, you
>> don't
>> want to add anything anyway):
>>
>> to_add = -(max(stn.deficit) for stn in active stations)
>> for stn in active stations:
>> stn.deficit += to_add + stn.weight
>>
> Toke,
>
> Sorry for the delayed response. I did lot of experiments. Below are my
> observations.
> Sorry for lengthy reply.
>
> In current model, next_txq() is main routine that serves DRR and
> fairness is enforced by serving only only first txq. Here the first
> node could be either newly initiated traffic or returned node by
> return_txq(). This works perfectly as long as the driver is running
> any RR algo.
>
> Whereas in ath10k, firmware runs its own RR in pull mode and builds
> txq list based on driver's hostq table. In this case it can not be
> simply assumed that firmware always gives fetch request for first node
> of mac80211's txq list. i.e both RR algo could be out of sync.
So I'm wondering why they don't sync; if the hardware is just doing RR
scheduling, eventually it should hit the TXQ that's first in the queue
and keep in sync after that?
How are you testing, and what metrics are you using?
> On an idle condition a single fetch indication can dequeue ~190 msdus
> from each tid of give stn list.
Wow, that sounds pretty bad. Guess we need the airtime queue limits! :)
> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
> index 625a4ab37ea0..269ae8311056 100644
> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
> @@ -2352,7 +2352,7 @@ static void ath10k_htt_rx_tx_fetch_ind(struct ath10k *ar, struct sk_buff *skb)
> num_msdus++;
> num_bytes += ret;
> }
> - ieee80211_return_txq(hw, txq);
> + ieee80211_return_txq(hw, txq, true);
I don't like the extra parameter; a similar one was in an earlier
version of my patch set, but I'd prefer that mac80211 just does the
right thing...
Do I understand it correctly that push/pull mode is selected solely by
hardware/firmware versions? Because in that case we could split it into
two feature flags instead...
> @@ -3670,13 +3670,8 @@ bool ieee80211_txq_may_transmit(struct ieee80211_hw *hw,
> if (sta->airtime[ac].deficit >= 0)
> goto out;
>
> - list_for_each_entry(txqi, &local->active_txqs[ac], schedule_order) {
> - if (!txqi->txq.sta)
> - continue;
> - sta = container_of(txqi->txq.sta, struct sta_info, sta);
> - sta->airtime[ac].deficit +=
> - (IEEE80211_TXQ_MAY_TX_QUANTUM * sta->airtime_weight);
> - }
> + sta->airtime[ac].deficit += sta->airtime_weight;
> + list_move_tail(&txqi->schedule_order, &local->active_txqs[ac]);
I'm wondering whether this actually succeeds in achieving fairness? This
basically allows a TXQ to be plucked from any point in the list, get a
quantum increase and be put back on, no matter the state of other TXQs.
Did you test how well the stations divide their airtime? And if so,
under which conditions?
-Toke
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2018-10-16 10:20 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-09 12:32 [PATCH RFC v5 0/4] Move TXQ scheduling and airtime fairness into mac80211 Toke Høiland-Jørgensen
2018-10-09 12:32 ` [PATCH RFC v5 2/4] cfg80211: Add airtime statistics and settings Toke Høiland-Jørgensen
2018-10-09 12:32 ` [PATCH RFC v5 1/4] mac80211: Add TXQ scheduling API Toke Høiland-Jørgensen
2018-10-09 12:32 ` [PATCH RFC v5 4/4] ath9k: Switch to mac80211 TXQ scheduling and airtime APIs Toke Høiland-Jørgensen
2018-10-09 12:32 ` [PATCH RFC v5 3/4] mac80211: Add airtime accounting and scheduling to TXQs Toke Høiland-Jørgensen
2018-10-10 4:52 ` Rajkumar Manoharan
2018-10-10 11:15 ` Toke Høiland-Jørgensen
2018-10-10 23:20 ` Rajkumar Manoharan
2018-10-11 10:38 ` Toke Høiland-Jørgensen
2018-10-12 7:38 ` Rajkumar Manoharan
2018-10-12 10:16 ` Toke Høiland-Jørgensen
2018-10-16 7:07 ` Rajkumar Manoharan
2018-10-16 10:20 ` Toke Høiland-Jørgensen
2018-10-13 7:09 ` [Make-wifi-fast] " Dave Taht
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).