Johannes Berg <johannes@xxxxxxxxxxxxxxxx> writes: > On Fri, 2019-11-08 at 11:56 +0100, Toke Høiland-Jørgensen wrote: >> Johannes Berg <johannes@xxxxxxxxxxxxxxxx> 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