Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Felix Fietkau <nbd@nbd.name>
To:     linux-wireless@vger.kernel.org
Cc:     johannes@sipsolutions.net, toke@kernel.org
Subject: [PATCH v2 3/4] mac80211: fix ieee80211_txq_may_transmit regression
Date:   Sat, 11 Jun 2022 22:01:05 +0200
Message-Id: <20220611200106.34319-3-nbd@nbd.name>
In-Reply-To: <20220611200106.34319-1-nbd@nbd.name>
References: <20220611200106.34319-1-nbd@nbd.name>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

After switching to the virtual time based airtime scheduler, there were reports
that ath10k with tx queueing in push-pull mode was experiencing significant
latency for some stations.
The reason for it is the fact that queues from which the ath10k firmware wants
to pull are getting starved by airtime fairness constraints.
Theoretically the same issue should have been there before the switch to virtual
time, however it seems that in the old round-robin implementation it was simply
looping until the requested txq was considered eligible, which led to it pretty
much ignoring fairness constraints anyway.

In order to fix the immediate regression, let's make bypassing airtime fairness
explicit for now.
Also update the documentation for ieee80211_txq_may_transmit, which was still
referring to implementation details of the old round-robin scheduler

Fixes: 2433647bc8d9 ("mac80211: Switch to a virtual time-based airtime scheduler")
Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
 include/net/mac80211.h | 21 +++++----------------
 net/mac80211/tx.c      | 17 +----------------
 2 files changed, 6 insertions(+), 32 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index d8c4d49bcc49..cb65abab8f36 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -6821,22 +6821,11 @@ void ieee80211_return_txq(struct ieee80211_hw *hw, struct ieee80211_txq *txq,
 /**
  * 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.
- *
- * The API ieee80211_txq_may_transmit() also ensures that TXQ list will be
- * aligned against driver's own round-robin scheduler list. i.e it rotates
- * the TXQ list till it makes the requested node becomes the first entry
- * in TXQ list. Thus both the TXQ list and driver's list are in sync. If this
- * function returns %true, the driver is expected to schedule packets
+ * Returns %true if there is remaining AQL budget for the tx queue and %false
+ * if it should be throttled. It will also mark the queue as active for the
+ * airtime scheduler.
+ *
+ * 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()
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index ecb3975680bc..f844e9ed4c14 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -4099,15 +4099,13 @@ EXPORT_SYMBOL(ieee80211_txq_airtime_check);
 bool ieee80211_txq_may_transmit(struct ieee80211_hw *hw,
 				struct ieee80211_txq *txq)
 {
-	struct txq_info *first_txqi = NULL, *txqi = to_txq_info(txq);
 	struct ieee80211_local *local = hw_to_local(hw);
+	struct txq_info *txqi = to_txq_info(txq);
 	struct airtime_sched_info *air_sched;
 	struct airtime_info *air_info;
-	struct rb_node *node = NULL;
 	bool ret = false;
 	u64 now;
 
-
 	if (!ieee80211_txq_airtime_check(hw, txq))
 		return false;
 
@@ -4119,19 +4117,6 @@ bool ieee80211_txq_may_transmit(struct ieee80211_hw *hw,
 
 	now = ktime_get_coarse_boottime_ns();
 
-	/* Like in ieee80211_next_txq(), make sure the first station in the
-	 * scheduling order is eligible for transmission to avoid starvation.
-	 */
-	node = rb_first_cached(&air_sched->active_txqs);
-	if (node) {
-		first_txqi = container_of(node, struct txq_info,
-					  schedule_order);
-		air_info = to_airtime_info(&first_txqi->txq);
-
-		if (air_sched->v_t < air_info->v_t)
-			airtime_catchup_v_t(air_sched, air_info->v_t, now);
-	}
-
 	air_info = to_airtime_info(&txqi->txq);
 	if (air_info->v_t <= air_sched->v_t) {
 		air_sched->last_schedule_activity = now;
-- 
2.36.1