Re: [PATCH v1] wifi: mac80211: Initialize EWMA fail avg to 1

[Felix Fietkau <nbd@nbd.name>]

On 20.04.23 12:27, Johannes Berg wrote:
> On Thu, 2023-04-20 at 11:30 +0200, Benjamin Beichler wrote:
>> > To me, basically, I see two ways to solve this:
>> > 
>> > 1) we have DECLARE_EWMA_ZERO_VALID() or something like that which
>> >     *doesn't* treat 0 as an uninitialized value, and either has a
>> >     separate "not initialized yet" bit (but that's iffy storage wise),
>> >     or simply has another argument to _init() for the initial value or
>> >     so.
>> > 
>> > 2) you don't just don't use 0 and 100 but say 1 and 100, that results in
>> >     basically the same behaviour, but avoids the special 0.
>> > 
>> > johannes
>> 
>> I also ran into that problem in the past, and reviewing it again with a 
>> college, I think, this is a real bug in the EWMA implementation. I try 
>> to provide a proper patch in the next days, but actually the EWMA 
>> handles the internal value zero, always like in the initialization, 
>> which is wrong, e.g., for positive/negative averaged values.
> 
> Yes, it's always wrong as long as you feed it something zero, or values
> with different sign.
> 
> For a lot of use cases, however, that doesn't matter. Originally, it was
> used e.g. for signal strength averaging, average packet lengths, etc.
> where it really doesn't matter since you can never use 0 or values that
> have different sign.
> 
>> A quick research shows, this bug is since the first implementation of 
>> the ewma in the code ...
>> 
> 
> Yeah, I'm aware of that, I was around for it ;-)
> 
> But see above, I'm not sure I'd even call it a bug, at least not
> originally with the users that we had intended.
> 
> Hence I don't know if it's really good to fix this in general - for many
> of these cases zero can still be treated specially (and like I mentioned
> in my previous email, we can even here avoid 0), and then we don't spend
> an extra byte (or likely 4) to hold a "first time" flag.
> 
> Dunno. Maybe it's not worth thinking about the extra memory space vs.
> the extra maintenance cost. But maybe at least on 64-bit we could steal
> a bit from the unsigned long? Not sure what all the users are..
We don't actually need a full bit. We can just add 1 to the internal 
value for initialized values. How about this (completely untested):
https://nbd.name/p/69b00c5b

- Felix