Hi John,
On 5/7/2024 6:16 PM, John Hubbard wrote:
> On 5/7/24 3:30 PM, Reinette Chatre wrote:
> ...
>>> diff --git a/tools/testing/selftests/resctrl/cmt_test.c b/tools/testing/selftests/resctrl/cmt_test.c
>>> index a81f91222a89..af33abd1cca7 100644
>>> --- a/tools/testing/selftests/resctrl/cmt_test.c
>>> +++ b/tools/testing/selftests/resctrl/cmt_test.c
>>> @@ -29,22 +29,22 @@ static int cmt_setup(const struct resctrl_test *test,
>>> return 0;
>>> }
>>> -static int show_results_info(unsigned long sum_llc_val, int no_of_bits,
>>> - unsigned long cache_span, unsigned long max_diff,
>>> - unsigned long max_diff_percent, unsigned long num_of_runs,
>>> +static int show_results_info(long sum_llc_val, int no_of_bits,
>>> + long cache_span, long max_diff,
>>> + long max_diff_percent, long num_of_runs,
>>> bool platform)
>>> {
>>> - unsigned long avg_llc_val = 0;
>>> + long avg_llc_val = 0;
>>> float diff_percent;
>>> long avg_diff = 0;
>>> int ret;
>>> avg_llc_val = sum_llc_val / num_of_runs;
>>> - avg_diff = (long)abs(cache_span - avg_llc_val);
>>> + avg_diff = labs(cache_span - avg_llc_val);
>>> diff_percent = ((float)cache_span - avg_llc_val) / cache_span * 100;
>>> ret = platform && abs((int)diff_percent) > max_diff_percent &&
>>> - abs(avg_diff) > max_diff;
>>> + labs(avg_diff) > max_diff;
>>> ksft_print_msg("%s Check cache miss rate within %lu%%\n",
>>> ret ? "Fail:" : "Pass:", max_diff_percent);
>>
>> The changes in this hunk are unexpected. The changes to this area made by previous
>> version was ok, no? It really seems like this just does a brute force of everything
>
> Well, not entirely. That first version was when I still believed clang's
> claim that abs()/labs() was a no-op. I've since been corrected! :)
>
>> to long (while taking labs() twice) unnecessarily.
>
> Which part exactly is unnecessary? Are you looking at the function args?
> Or something else? I've stared at it too much and am not spotting the
> issue yet.
>
The following (what was in v1) looks good to me. What am I missing?
diff --git a/tools/testing/selftests/resctrl/cmt_test.c b/tools/testing/selftests/resctrl/cmt_test.c
index a81f91222a89..05a241519ae8 100644
--- a/tools/testing/selftests/resctrl/cmt_test.c
+++ b/tools/testing/selftests/resctrl/cmt_test.c
@@ -40,11 +40,11 @@ static int show_results_info(unsigned long sum_llc_val, int no_of_bits,
int ret;
avg_llc_val = sum_llc_val / num_of_runs;
- avg_diff = (long)abs(cache_span - avg_llc_val);
+ avg_diff = (long)(cache_span - avg_llc_val);
diff_percent = ((float)cache_span - avg_llc_val) / cache_span * 100;
ret = platform && abs((int)diff_percent) > max_diff_percent &&
- abs(avg_diff) > max_diff;
+ labs(avg_diff) > max_diff;
ksft_print_msg("%s Check cache miss rate within %lu%%\n",
ret ? "Fail:" : "Pass:", max_diff_percent);
Reinette
