time: fix overflow in timespec_add_msec

Commit 8b6a404cdd2c40715885e562416c3db039912773 ("nanosecond: initial
commit changing timeval to timespec") introduced a change to
timespec_add_msec that decreased the time to (unsigned) overflow of
adj_nsec leading to strange values in the write_*_log files when using
log_avg_msec. The following fio job reproduces the problem:

./fio --ioengine=null --size=2G --time_based --runtime=11s \
 --ramp_time=5s --write_iops_log=overflow --log_avg_msec=5000 \
 --name=overflow

cat overflow_iops.1.log shows the following:
5000, 997853, 0, 0
10640, 141323, 0, 0

Compiling fio with clang's undefined behaviour
sanitizer with unsigned wraparound detection turned on by using

CC=clang ./configure --extra-cflags="-fsanitize=undefined,integer"

and running the previous job produces a lot of false positives but also
flags up this:

time.c:11:35: runtime error: unsigned integer overflow: 1000000 * 5000 cannot be represented in type 'unsigned int'

Fix this by forcing the type of the integer literal to ensure that a 64
bit multiplication takes place. Further, force the result variables to
be uint64_t allowing us to remove UL from other literals. Finally adjust
some spacing while we're here.

Fixes https://github.com/axboe/fio/issues/426 (which was also reported
on the mailing list - http://www.spinics.net/lists/fio/msg06190.html ).

Reported-by: Joseph R Gruher <joseph.r.gruher@intel.com>
Suggested-by: Vincent Fu <vincent.fu@wdc.com>
Suggested-by: Jeff Furlong <jeff.furlong@wdc.com>
Reviewed-by: Vincent Fu <vincent.fu@wdc.com>
Tested-by: Joseph R Gruher <joseph.r.gruher@intel.com>
Signed-off-by: Sitsofe Wheeler <sitsofe@yahoo.com>

diff --git a/time.c b/time.c
index edfe779bc2423876e949be48b11fb3ed354af986..07984190d67a914173e1980f0ed0461b45ea5ee8 100644 (file)
--- a/time.c
+++ b/time.c
@@ -8,17 +8,17 @@ static unsigned long ns_granularity;
 
 void timespec_add_msec(struct timespec *ts, unsigned int msec)
 {
 
 void timespec_add_msec(struct timespec *ts, unsigned int msec)
 {

-       unsigned long adj_nsec = 1000000 * msec;
+       uint64_t adj_nsec = 1000000ULL * msec;

 
        ts->tv_nsec += adj_nsec;
        if (adj_nsec >= 1000000000) {
 
        ts->tv_nsec += adj_nsec;
        if (adj_nsec >= 1000000000) {

-               unsigned long adj_sec = adj_nsec / 1000000000UL;
+               uint64_t adj_sec = adj_nsec / 1000000000;

 
 

-               ts->tv_nsec -=  adj_sec * 1000000000UL;
+               ts->tv_nsec -= adj_sec * 1000000000;

                ts->tv_sec += adj_sec;
        }
                ts->tv_sec += adj_sec;
        }

-       if (ts->tv_nsec >= 1000000000UL){
-               ts->tv_nsec -= 1000000000UL;
+       if (ts->tv_nsec >= 1000000000){
+               ts->tv_nsec -= 1000000000;

                ts->tv_sec++;
        }
 }
                ts->tv_sec++;
        }
 }