stats: incorrect clock square algorithm

Since the original IBM code import, OpenAFS has an algorithm for
squaring clock values, implemented identically in three different
places.  This algorithm does not account correctly for microsecs
overflow into seconds, resulting in incorrect "sum-of-squares" values
for queue and execution time in several OpenAFS performance utilities.

Specifically, this code:

        t1.tv_usec += (2 * t2.tv_sec * t2.tv_usec) % 1000000                   \
                      + (t2.tv_usec / 1000)*(t2.tv_usec / 1000)                \
                      + 2 * (t2.tv_usec / 1000) * (t2.tv_usec % 1000) / 1000   \
                      + (((t2.tv_usec % 1000) > 707) ? 1 : 0);                 \

Can allow for the tv_usec field to be increased by a theoretical max
of around:

        t1.tv_usec += 999998                                                   \
                      + 999*999                                                \
                      + 2 * 999 * 999 / 1000                                   \
                      + 1;                                                     \

Or:

        t1.tv_usec += 1999996;                                                 \

If t1.tv_usec is already 999999, after this calculation its value
could be as high as 2999995. So just checking once if t1.tv_usec is
over 1000000 is not sufficient, since the resulting value (1999995) is
still over 1000000.

Correct all implementations by repeatedly checking if tv_usec is over
1000000 after the above calculation:

macro                   affected utility
=====================   ============================
afs_stats_SquareAddTo   xstat_cm_test
fs_stats_SquareAddTo    xstat_fs_test
clock_AddSq             rxstat_get_process and _peer

Change-Id: I3145d592ba6bc1556729eac657f43d476c99eede
Reviewed-on: https://gerrit.openafs.org/14376
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

diff --git a/src/afs/afs_stats.h b/src/afs/afs_stats.h
index 0fd5298..1d6c86a 100644 (file)
--- a/src/afs/afs_stats.h
+++ b/src/afs/afs_stats.h
@@ -1067,7 +1067,7 @@ struct afs_stats_xferData {
                      + 2 * (t2.tv_usec / 1000) * (t2.tv_usec % 1000) / 1000   \
                      + (((t2.tv_usec % 1000) > 707) ? 1 : 0);                 \
      }                                                                        \
-   if (t1.tv_usec > 1000000) {                                                \
+   while (t1.tv_usec > 1000000) {                                             \
         t1.tv_usec -= 1000000;                                                \
         t1.tv_sec++;                                                          \
    }                                                                          \

diff --git a/src/rx/rx_clock.h b/src/rx/rx_clock.h
index c1184e9..a836fab 100644 (file)
--- a/src/rx/rx_clock.h
+++ b/src/rx/rx_clock.h
@@ -183,7 +183,7 @@ clock_GetTime(struct clock *cv)
                      + 2 * ((c2)->usec / 1000) * ((c2)->usec % 1000) / 1000   \
                      + ((((c2)->usec % 1000) > 707) ? 1 : 0);                 \
      }                                                                        \
-   if ((c1)->usec > 1000000) {                                                \
+   while ((c1)->usec > 1000000) {                                             \
         (c1)->usec -= 1000000;                                                \
         (c1)->sec++;                                                          \
    }                                                                          \

diff --git a/src/viced/fs_stats.h b/src/viced/fs_stats.h
index e3c0c3f..9cb6cf3 100644 (file)
--- a/src/viced/fs_stats.h
+++ b/src/viced/fs_stats.h
@@ -286,7 +286,7 @@ struct fs_stats_xferData {
       {                                                      \
        t1.tv_usec += (int) ((0.000001 * t2.tv_usec) * t2.tv_usec);   \
       }                                                      \
-    if (t1.tv_usec > 1000000) {                              \
+    while (t1.tv_usec > 1000000) {                           \
         t1.tv_usec -= 1000000;                               \
         t1.tv_sec++;                                         \
     }                                                        \