On Fri, 2021-02-05 at 14:29 +0100, Toke Høiland-Jørgensen wrote:
> Ryder Lee <ryder.lee@xxxxxxxxxxxx> writes:
>
> > For some drivers and hardware may report faulty airtime, which ends up
> > with excessive hold time (~0.9s on mt7915 multiclent tests) impacting
> > system performance.
> >
> > Although issue has been fixed in driver, but it make sense to select txqi
> > depends on a reasonable airtime reporting to prevent such a case from
> > happening again.
>
> I think I see what you're trying to do with the patch, but this commit
> message makes no sense. What, exactly, was the error you were seeing
> that this is supposed to fix?
I will make commit message more straightforward - if a station takes
large amount of airtime and fails the check that will keep getting
recycled through the list along with excessive lock hold time. Add this
patch to avoid breaking fairness.
> > Tested-by: Jiao Bo <jiao.bao@xxxxxxxxxxxx>
> > Tested-by: Sujuan Chen <sujuan.chen@xxxxxxxxxxxx>
> > Signed-off-by: Ryder Lee <ryder.lee@xxxxxxxxxxxx>
> > ---
> > net/mac80211/tx.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
> > index 6422da6690f7..0b8a8c3600f4 100644
> > --- a/net/mac80211/tx.c
> > +++ b/net/mac80211/tx.c
> > @@ -3770,6 +3770,10 @@ struct ieee80211_txq *ieee80211_next_txq(struct ieee80211_hw *hw, u8 ac)
> > sta->airtime_weight;
> >
> > if (deficit < 0 || !aql_check) {
> > + if (txqi->schedule_round == local->schedule_round[ac])
> > + goto out;
> > +
> > + txqi->schedule_round = local->schedule_round[ac];
>
> I think this change may be worth making anyway, but for a different
> reason: Without it, a station that fails aql_check will keep getting
> recycled through the list, advancing its deficit. Which could actually
> be the reason AQL breaks airtime fairness; did you observe any
> difference in fairness with this change?
Our case is: mt7915 provides per-peer airtime counters. However, some of
them were not properly configured, so certain stations reported large
amount of airtime which led to deficit < 0, and as you said, ending up
with recycle + very longer lock hold time (0.9s in our tests) and
breaking fairness.
Ryder
