[LTP] [PATCH v2 2/2] ebizzy: prevent integer overflow in 64-bit systems

[Vishal Chourasia]

On 8/21/23 18:54, Cyril Hrubis wrote:
> Hi!
>> Signed-off-by: Vishal Chourasia <vishalc@linux.ibm.com>
>> Reviewed-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
>> Reviewed-by: Shrikanth Hegde <sshegde@linux.vnet.ibm.com>
>> ---
>>  utils/benchmark/ebizzy-0.3/ebizzy.c | 7 +++----
>>  1 file changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/utils/benchmark/ebizzy-0.3/ebizzy.c b/utils/benchmark/ebizzy-0.3/ebizzy.c
>> index ae0981fbd..c3eac1133 100644
>> --- a/utils/benchmark/ebizzy-0.3/ebizzy.c
>> +++ b/utils/benchmark/ebizzy-0.3/ebizzy.c
>> @@ -456,7 +456,7 @@ static void start_threads(void)
>>  	unsigned int i;
>>  	struct rusage start_ru, end_ru;
>>  	struct timeval usr_time, sys_time;
>> -	uintptr_t records_read = 0;
>> +	uintptr_t records_per_sec = 0;
>>  	int err;
>>  
>>  	if (verbose)
>> @@ -493,14 +493,13 @@ static void start_threads(void)
>>  			fprintf(stderr, "Error joining thread %d\n", i);
>>  			exit(1);
>>  		}
>> -		records_read += record_thread;
>> +		records_per_sec += (record_thread / elapsed);
> 
> This actually introduces quite a bit rounding errors. Note that even
> with 10 second runtime we effectively throw away the last digit for each
> thread records and it's even worse for larger runtime. This kind of
> naive summing can easily add up to quite large differencies.
> 
> E.g. for a machine with 256 CPUs and 10 seconds the value would be on
> average smaller by:
> 
> 	512 (number of threads) * 5 (average last digit) / 10 (runtime)
> 
> So on average the result printed would be smaller by 256 units.
> 
> Either we have to switch the records_per_sec to double and compute the
> number in floating point, which should add up fine as long as the work
> done by a threads is more or less the same.
> 
> Or use uint64_t as a fixed point and use one or two bits for a fraction,
> which may be a bit safer than double and should work fine as long as
> we round properly.
> 
> This should be easy to debug as well, just print the number computed by
> the method that sum first and then divide and compare it with the number
> that sums the already divided values. These two shouldn't differ much,
> ideally the should be the same.
> 
To address this, We can modify the code to keep the records per second
as a `double` type until the point of printing. This ensures that we
maintain precision throughout the calculation.

diff --git a/utils/benchmark/ebizzy-0.3/ebizzy.c
b/utils/benchmark/ebizzy-0.3/ebizzy.c
index ffc439a24..f9bc41ba9 100644
--- a/utils/benchmark/ebizzy-0.3/ebizzy.c
+++ b/utils/benchmark/ebizzy-0.3/ebizzy.c
@@ -456,6 +456,7 @@ static void start_threads(void)
        unsigned int i;
        struct rusage start_ru, end_ru;
        struct timeval usr_time, sys_time;
+       uintptr_t records = 0;
        double records_per_sec = 0.0;
        int err;

@@ -493,13 +494,15 @@ static void start_threads(void)
                        fprintf(stderr, "Error joining thread %d\n", i);
                        exit(1);
                }
+               records += record_thread;
                records_per_sec += ((double)record_thread / elapsed);
        }

        if (verbose)
                printf("Threads finished\n");

-       printf("%tu records/s\n", (uintptr_t) records_per_sec);
+       printf("Method 1: %tu records/s\n", (uintptr_t)((double)records
/ elapsed));
+       printf("Method 2: %tu records/s\n", (uintptr_t) records_per_sec);

        usr_time = difftimeval(&end_ru.ru_utime, &start_ru.ru_utime);
        sys_time = difftimeval(&end_ru.ru_stime, &start_ru.ru_stime);

Both methods yield the same result, confirming the accuracy of the
calculation. Here are some sample outputs:


$ ./ebizzy -S 10 -t 512
Method 1: 3049076 records/s
Method 2: 3049076 records/s

$ ./ebizzy -S 10 -t 1000
Method 1: 2273819 records/s
Method 2: 2273819 records/s

I will be sending a proper patch as reply to this thread.

vishal.c