BUG/MINOR: freq_ctr: fix possible negative rate with the scaled API

In 1.9 with commit 627505d36 ("MINOR: freq_ctr: add swrate_add_scaled()
to work with large samples") we got the ability to indicate when adding
some values that they represent a number of samples. However there is an
issue in the calculation which is that the number of samples that is
added to the sum before the division in order to avoid fading away too
fast, is multiplied by the scale. The problem it causes is that this is
done in the negative part of the expression, and that as soon if the sum
of old_sum and v*s is too small (e.g. zero), we end up with a negative
value of -s.

This is visible in "show pools" which occasionally report a very large
value on "needed_avg" since 2.9, though the bug has been there for longer.
Indeed in 2.9 since they're hashed in buckets, it suffices that any
thread got one such error once for the sum to be wrong. One possible
impact is memory usage not shrinking after a short burst due to pools
refraining from releasing objects, believing they don't have enough.

This must be backported to all versions. Note that the opportunistic
version can be dropped before 2.8.

diff --git a/include/haproxy/freq_ctr.h b/include/haproxy/freq_ctr.h
index 956061177674374c9fe1818c7620f3aa234629e4..cecf8221830fdcc9584a6a50bd4e361b923f2847 100644 (file)
--- a/include/haproxy/freq_ctr.h
+++ b/include/haproxy/freq_ctr.h
@@ -343,7 +343,7 @@ static inline unsigned int swrate_add_scaled(unsigned int *sum, unsigned int n,
 
        old_sum = *sum;
        do {
-               new_sum = old_sum + v * s - div64_32((unsigned long long)(old_sum + n) * s, n);
+               new_sum = old_sum + v * s - div64_32((unsigned long long)old_sum * s + n - 1, n);
        } while (!HA_ATOMIC_CAS(sum, &old_sum, new_sum) && __ha_cpu_relax());
        return new_sum;
 }
@@ -378,7 +378,7 @@ static inline uint swrate_add_scaled_opportunistic(uint *sum, uint n, uint v, ui
        uint new_sum, old_sum;
 
        old_sum = *sum;
-       new_sum = old_sum + v * s - div64_32((unsigned long long)(old_sum + n) * s, n);
+       new_sum = old_sum + v * s - div64_32((unsigned long long)old_sum * s + n - 1, n);
        HA_ATOMIC_CAS(sum, &old_sum, new_sum);
        return new_sum;
 }