On 5/3/24 1:46 PM, Reinette Chatre wrote:
Hi John, On 5/3/2024 12:12 PM, John Hubbard wrote:
On 5/3/24 11:37 AM, Reinette Chatre wrote:
On 5/3/2024 9:52 AM, John Hubbard wrote:
On 5/3/24 1:00 AM, Ilpo Järvinen wrote:
On Thu, 2 May 2024, John Hubbard wrote:
...
...
I assumed that this code did not expect to handle negative numbers, because it is using unsigned math throughout.
If you do expect it to handle cases where, for example, this happens:
avg_bw_imc > avg_bw_resc
The existing code seems to handle this ok. A sample program with this scenario comparing existing computation with your first proposal is below:
#include <stdio.h> #include <stdlib.h>
void main(void) { unsigned long avg_bw_resc = 20000; unsigned long avg_bw_imc = 40000; float avg_diff;
/* Existing code */ avg_diff = (float)labs(avg_bw_resc - avg_bw_imc) / avg_bw_imc; printf("Existing code: avg_diff = %f\n", avg_diff);
/* Original proposed fix */ avg_diff = (float)(avg_bw_resc - avg_bw_imc) / avg_bw_imc; printf("Original proposed fix: avg_diff = %f\n", avg_diff); }
output: Existing code: avg_diff = 0.500000 Original proposed fix: avg_diff = 461168590192640.000000
That seems "a little bit" wrong. haha :)
...then a proper solution is easy, and looks like this:
diff --git a/tools/testing/selftests/resctrl/mbm_test.c b/tools/testing/selftests/resctrl/mbm_test.c index c873793d016d..b87f91a41494 100644 --- a/tools/testing/selftests/resctrl/mbm_test.c +++ b/tools/testing/selftests/resctrl/mbm_test.c @@ -17,8 +17,8 @@ static int show_bw_info(unsigned long *bw_imc, unsigned long *bw_resc, size_t span) { - unsigned long avg_bw_imc = 0, avg_bw_resc = 0; - unsigned long sum_bw_imc = 0, sum_bw_resc = 0; + long avg_bw_imc = 0, avg_bw_resc = 0; + long sum_bw_imc = 0, sum_bw_resc = 0; int runs, ret, avg_diff_per; float avg_diff = 0;
Should I resend the patch with that approach?
ok. That indeed makes the computations easier to understand. I assume you intend to fix the snippet in mba_test.c also?
Yes, will do that. Thanks for spotting the bug in the original "fix"!
thanks,