Re: [Make-wifi-fast] [PATCH RFC v4 1/4] mac80211: Add TXQ scheduling API

[Rajkumar Manoharan <rmanohar@codeaurora.org>]

On 2018-09-18 13:41, Toke Høiland-Jørgensen wrote:
> Rajkumar Manoharan <rmanohar@codeaurora.org> writes:
> 
>>>> Also an option to add the node at head or tail would be preferred. 
>>>> If
>>>> return_txq adds node at head of list, then it is forcing the driver 
>>>> to
>>>> serve same txq until it becomes empty. Also this will not allow the
>>>> driver to send N frames from each txqs.
>>> 
>>> The whole point of this patch set is to move those kinds of decisions
>>> out of the driver and into mac80211. The airtime scheduler won't
>>> achieve
>>> fairness if it allows queues to be queued to the end of the rotation
>>> before its deficit turns negative. And obviously there's some lag in
>>> this since we're using after-the-fact airtime information.
>>> 
>> Hmm.. As you know ath10k kind of doing fairness by serving fixed 
>> frames
>> from each txq. This approach will be removed from ath10k.
>> 
>>> For ath9k this has not really been a problem in my tests; if the lag
>>> turns out to be too great for ath10k (which I suppose is a 
>>> possibility
>>> since we don't get airtime information on every TX-compl), I figure 
>>> we
>>> can use the same estimated airtime value that is used for throttling
>>> the
>>> queues to adjust the deficit immediately...
>>> 
>> Thats true. I am porting Kan's changes of airtime estimation for each
>> msdu for firmware that does not report airtime.
> 
> Right. My thinking with this was that we could put the per-frame 
> airtime
> estimation into ieee80211_tx_dequeue(), which could track the
> outstanding airtime and just return NULL if it goes over the threshold.
> I think this is fairly straight-forward to do on its own; the biggest
> problem is probably finding the space in the mac80211 cb?
> 
> Is this what you are working on porting? Because then I'll wait for 
> your
> patch rather than starting to write this code myself :)
> 
Kind of.. something like below.

tx_dequeue(){
     compute airtime_est from last_tx_rate
     if (sta->airtime[ac].deficit < airtime_est)
         return NULL;
     dequeue skb and store airtime_est in cb
}

Unfortunately ath10k is not reporting last_tx_rate in tx_status(). So I
also applied this "ath10k: report tx rate using ieee80211_tx_status" 
change.

> This mechanism on its own will get us the queue limiting and latency
> reduction goodness for firmwares with deep queues. And for that it can
> be completely independent of the airtime fairness scheduler, which can
> use the after-tx-compl airtime information to presumably get more
> accurate fairness which includes retransmissions etc.
> 
> Now, we could *also* use the ahead-of-time airtime estimation for
> fairness; either just as a fallback for drivers that can't get actual
> airtime usage information for the hardware, or as an alternative in
> cases where it works better for other reasons. But I think that
> separating the two in the initial implementation makes more sense; that
> will make it easier to experiment with different combinations of the
> two.
> 
> Does that make sense? :)
> 
Completely agree. I was thinking of using this as fallback for devices
that does not report airtime but tx rate.

-Rajkumar