Re: [PATCH 9/9] mac80211: rc80211_minstrel: remove variance / stddev calculation

[Dave Taht <dave.taht@gmail.com>]

On Sat, Oct 6, 2018 at 11:18 AM Felix Fietkau <nbd@nbd.name> wrote:
>
> On 2018-10-06 19:59, Dave Taht wrote:
> > On Sat, Oct 6, 2018 at 10:37 AM Felix Fietkau <nbd@nbd.name> wrote:
> >>
> >> When there are few packets (e.g. for sampling attempts), the exponentially
> >> weighted variance is usually vastly overestimated, making the resulting data
> >> essentially useless. As far as I know, there has not been any practical use
> >> for this, so let's not waste any cycles on it.
> >>
> >> Signed-off-by: Felix Fietkau <nbd@nbd.name>
> >> ---
> >>  net/mac80211/rc80211_minstrel.c            |  6 -----
> >>  net/mac80211/rc80211_minstrel.h            | 26 +---------------------
> >>  net/mac80211/rc80211_minstrel_debugfs.c    | 14 ++++--------
> >>  net/mac80211/rc80211_minstrel_ht_debugfs.c | 14 ++++--------
> >>  4 files changed, 9 insertions(+), 51 deletions(-)
> >>
> >> diff --git a/net/mac80211/rc80211_minstrel.c b/net/mac80211/rc80211_minstrel.c
> >> index dead57ba9eac..a34e9c2ca626 100644
> >> --- a/net/mac80211/rc80211_minstrel.c
> >> +++ b/net/mac80211/rc80211_minstrel.c
> >> @@ -167,12 +167,6 @@ minstrel_calc_rate_stats(struct minstrel_rate_stats *mrs)
> >>                 if (unlikely(!mrs->att_hist)) {
> >>                         mrs->prob_ewma = cur_prob;
> >>                 } else {
> >> -                       /* update exponential weighted moving variance */
> >> -                       mrs->prob_ewmv = minstrel_ewmv(mrs->prob_ewmv,
> >> -                                                       cur_prob,
> >> -                                                       mrs->prob_ewma,
> >> -                                                       EWMA_LEVEL);
> >> -
> >>                         /*update exponential weighted moving avarage */
> >>                         mrs->prob_ewma = minstrel_ewma(mrs->prob_ewma,
> >>                                                        cur_prob,
> >> diff --git a/net/mac80211/rc80211_minstrel.h b/net/mac80211/rc80211_minstrel.h
> >> index 54b2b2c3e10a..23ec953e3a24 100644
> >> --- a/net/mac80211/rc80211_minstrel.h
> >> +++ b/net/mac80211/rc80211_minstrel.h
> >> @@ -35,19 +35,6 @@ minstrel_ewma(int old, int new, int weight)
> >>         return old + incr;
> >>  }
> >>
> >> -/*
> >> - * Perform EWMV (Exponentially Weighted Moving Variance) calculation
> >> - */
> >
> > I worry about this one. where are you getting your proof from?
> I've done quite a few measurements myself to see if this can be usable
> for further rate control improvements or for the upcoming TPC work.
> The data this generates simply fluctuates wildly and incoherently based
> on the sampling behavior, making it completely useless.
> Together with Thomas (who introduced this code), I tried a few times to
> fix this, but couldn't find any way to make it coherent and usable.
>
> Thomas and I both agreed that it's better to just remove it until
> somebody has a better idea what to do.
>
> Also, this was only used for debugfs statistics, not for any actual rate
> control behavior.

OK, thanks. I'm totally delighted to see this patchset otherwise.

> - Felix



-- 

Dave Täht
CTO, TekLibre, LLC
http://www.teklibre.com
Tel: 1-831-205-9740