[Make-wifi-fast] [PATCH v6 3/4] mac80211: Implement Airtime-based Queue Limit (AQL)

[Toke Høiland-Jørgensen]

Johannes Berg <johannes at sipsolutions.net> writes:

> On Fri, 2019-11-08 at 11:56 +0100, Toke Høiland-Jørgensen wrote:
>> Johannes Berg <johannes at sipsolutions.net> writes:
>> 
>> > On Wed, 2019-10-23 at 11:59 +0200, Toke Høiland-Jørgensen wrote:
>> > >  
>> > > +void ieee80211_sta_update_pending_airtime(struct ieee80211_local *local,
>> > > +					  struct sta_info *sta, u8 ac,
>> > > +					  u16 tx_airtime, bool tx_completed)
>> > > +{
>> > > +	spin_lock_bh(&local->active_txq_lock[ac]);
>> > > +	if (tx_completed) {
>> > > +		if (sta) {
>> > > +			if (WARN_ONCE(sta->airtime[ac].aql_tx_pending < tx_airtime,
>> > > +				      "TXQ pending airtime underflow: %u, %u",
>> > > +				      sta->airtime[ac].aql_tx_pending, tx_airtime))
>> > 
>> > Maybe add the STA/AC to the message?
>> 
>> Can do. Any idea why we might be seeing underflows (as Kan reported)?
>
> No, I really have no idea. The shifting looked OK to me, though I didn't
> review it carefully enough to say I've really looked at all places ...

Right, bugger. I was thinking maybe there's a case where skbs can be
cloned (and retain the tx_time_est field) and then released twice? Or
maybe somewhere that steps on the skb->cb field in some other way?
Couldn't find anything obvious on a first perusal of the TX path code,
but maybe you could think of something?

Otherwise I guess we'll be forced to go and do some actual,
old-fashioned debugging ;)

-Toke