Re: [PATCH v2 1/2] mac80211: Implement Airtime-based Queue Limit (AQL)

[Yibo Zhao <yiboz@codeaurora.org>]

> diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
> index f13eb2f61ccf..dadb643a5498 100644
> --- a/net/mac80211/tx.c
> +++ b/net/mac80211/tx.c
> @@ -3669,7 +3669,8 @@ struct ieee80211_txq *ieee80211_next_txq(struct
> ieee80211_hw *hw, u8 ac)
>  {
>  	struct ieee80211_local *local = hw_to_local(hw);
>  	struct ieee80211_txq *ret = NULL;
> -	struct txq_info *txqi = NULL;
> +	struct txq_info *txqi = NULL, *head = NULL;
> +	bool found_eligible_txq = false;
> 
>  	spin_lock_bh(&local->active_txq_lock[ac]);
> 
> @@ -3680,20 +3681,32 @@ struct ieee80211_txq
> *ieee80211_next_txq(struct ieee80211_hw *hw, u8 ac)
>  	if (!txqi)
>  		goto out;
> 
> +	if (txqi == head && !found_eligible_txq)
> +		goto out;

I assume here the only txq in the list that does not meet AQL check will 
not be dequeued. Right? Will it affect peak throughput once there is 
only one station.

How about dequeuing it anyway regardless AQL because it is the only one 
active now so it is fine to occupy the rest bandwidth. Otherwise, I am 
afraid next_txq() will return NULL in the test only one station is 
present.

> @@ -3748,10 +3785,10 @@ bool ieee80211_txq_may_transmit(struct 
> ieee80211_hw *hw,
>  	struct sta_info *sta;
>  	u8 ac = txq->ac;
> 
> -	spin_lock_bh(&local->active_txq_lock[ac]);
> -
>  	if (!txqi->txq.sta)
> -		goto out;
> +		return true;

why return here? I think even a txq without sta info should get removed 
from list and added it back later in return_txq() if needed. No?

> +
> +	spin_lock_bh(&local->active_txq_lock[ac]);
> 
>  	if (list_empty(&txqi->schedule_order))
>  		goto out;


-- 
Yibo