2019-03-17 17:02:59

by Felix Fietkau

[permalink] [raw]
Subject: [PATCH v2] mac80211: make ieee80211_schedule_txq schedule empty TXQs

Currently there is no way for the driver to signal to mac80211 that it should
schedule a TXQ even if there are no packets on the mac80211 part of that queue.
This is problematic if the driver has an internal retry queue to deal with
software A-MPDU retry.

This patch changes the behavior of ieee80211_schedule_txq to always schedule
the queue, as its only user (ath9k) seems to expect such behavior already:
it calls this function on tx status and on powersave wakeup whenever its
internal retry queue is not empty

Signed-off-by: Felix Fietkau <[email protected]>
---
include/net/mac80211.h | 17 ++++++++++++++---
net/mac80211/tx.c | 10 ++++++----
2 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 0de0aba580eb..630941e5a20e 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -6291,15 +6291,26 @@ static inline void ieee80211_txq_schedule_end(struct ieee80211_hw *hw, u8 ac)
{
}

+void __ieee80211_schedule_txq(struct ieee80211_hw *hw,
+ struct ieee80211_txq *txq, bool force);
+
/**
* ieee80211_schedule_txq - schedule a TXQ for transmission
*
* @hw: pointer as obtained from ieee80211_alloc_hw()
* @txq: pointer obtained from station or virtual interface
*
- * Schedules a TXQ for transmission if it is not already scheduled.
+ * Schedules a TXQ for transmission if it is not already scheduled,
+ * even if mac80211 does not have any packets buffered.
+ *
+ * The driver may call this function if it has buffered packets for
+ * this TXQ internally.
*/
-void ieee80211_schedule_txq(struct ieee80211_hw *hw, struct ieee80211_txq *txq);
+static inline void
+ieee80211_schedule_txq(struct ieee80211_hw *hw, struct ieee80211_txq *txq)
+{
+ __ieee80211_schedule_txq(hw, txq, true);
+}

/**
* ieee80211_return_txq - return a TXQ previously acquired by ieee80211_next_txq()
@@ -6310,7 +6321,7 @@ void ieee80211_schedule_txq(struct ieee80211_hw *hw, struct ieee80211_txq *txq);
static inline void
ieee80211_return_txq(struct ieee80211_hw *hw, struct ieee80211_txq *txq)
{
- ieee80211_schedule_txq(hw, txq);
+ __ieee80211_schedule_txq(hw, txq, false);
}

/**
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index f85344c9af62..72279fb318e0 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -3706,8 +3706,9 @@ struct ieee80211_txq *ieee80211_next_txq(struct ieee80211_hw *hw, u8 ac)
}
EXPORT_SYMBOL(ieee80211_next_txq);

-void ieee80211_schedule_txq(struct ieee80211_hw *hw,
- struct ieee80211_txq *txq)
+void __ieee80211_schedule_txq(struct ieee80211_hw *hw,
+ struct ieee80211_txq *txq,
+ bool force)
{
struct ieee80211_local *local = hw_to_local(hw);
struct txq_info *txqi = to_txq_info(txq);
@@ -3715,7 +3716,8 @@ void ieee80211_schedule_txq(struct ieee80211_hw *hw,
spin_lock_bh(&local->active_txq_lock[txq->ac]);

if (list_empty(&txqi->schedule_order) &&
- (!skb_queue_empty(&txqi->frags) || txqi->tin.backlog_packets)) {
+ (force || !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
@@ -3735,7 +3737,7 @@ void ieee80211_schedule_txq(struct ieee80211_hw *hw,

spin_unlock_bh(&local->active_txq_lock[txq->ac]);
}
-EXPORT_SYMBOL(ieee80211_schedule_txq);
+EXPORT_SYMBOL(__ieee80211_schedule_txq);

bool ieee80211_txq_may_transmit(struct ieee80211_hw *hw,
struct ieee80211_txq *txq)
--
2.17.0



2019-03-17 22:00:36

by Toke Høiland-Jørgensen

[permalink] [raw]
Subject: Re: [PATCH v2] mac80211: make ieee80211_schedule_txq schedule empty TXQs

Felix Fietkau <[email protected]> writes:

> Currently there is no way for the driver to signal to mac80211 that it should
> schedule a TXQ even if there are no packets on the mac80211 part of that queue.
> This is problematic if the driver has an internal retry queue to deal with
> software A-MPDU retry.
>
> This patch changes the behavior of ieee80211_schedule_txq to always schedule
> the queue, as its only user (ath9k) seems to expect such behavior already:
> it calls this function on tx status and on powersave wakeup whenever its
> internal retry queue is not empty
>
> Signed-off-by: Felix Fietkau <[email protected]>
> ---
> include/net/mac80211.h | 17 ++++++++++++++---
> net/mac80211/tx.c | 10 ++++++----
> 2 files changed, 20 insertions(+), 7 deletions(-)
>
> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> index 0de0aba580eb..630941e5a20e 100644
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -6291,15 +6291,26 @@ static inline void ieee80211_txq_schedule_end(struct ieee80211_hw *hw, u8 ac)
> {
> }
>
> +void __ieee80211_schedule_txq(struct ieee80211_hw *hw,
> + struct ieee80211_txq *txq, bool force);
> +

So are you planning to also change return_txq() to take the 'force'
argument and wrap __ieee80211_schedule_txq()? Since that was the context
we originally started discussing this :)

-Toke