Messages in this thread | | | Subject | Re: [PATCH v4 10/17] selftests/resctrl: Fix MBA/MBM results reporting format | From | Shuah Khan <> | Date | Mon, 25 Jan 2021 19:38:07 -0700 |
| |
On 11/30/20 1:20 PM, Fenghua Yu wrote: > MBM unit test starts fill_buf (default built-in benchmark) in a new con_mon > group (c1, m1) and records resctrl reported mbm values and iMC (Integrated > Memory Controller) values every second. It does this for five seconds > (randomly chosen value) in total. It then calculates average of resctrl_mbm > values and imc_mbm values and if the difference is greater than 300 MB/sec > (randomly chosen value), the test treats it as a failure. MBA unit test is > similar to MBM but after every run it changes schemata. > > Checking for a difference of 300 MB/sec doesn't look very meaningful when > the mbm values are changing over a wide range. For example, below are the > values running MBA test on SKL with different allocations > > 1. With 10% as schemata both iMC and resctrl mbm_values are around 2000 > MB/sec > 2. With 100% as schemata both iMC and resctrl mbm_values are around 10000 > MB/sec > > A 300 MB/sec difference between resctrl_mbm and imc_mbm values is > acceptable at 100% schemata but it isn't acceptable at 10% schemata because > that's a huge difference. > > So, fix this by checking for percentage difference instead of absolute > difference i.e. check if the difference between resctrl_mbm value and > imc_mbm value is within 5% (randomly chosen value) of imc_mbm value. If the > difference is greater than 5% of imc_mbm value, treat it is a failure. > > Signed-off-by: Fenghua Yu <fenghua.yu@intel.com> > --- > tools/testing/selftests/resctrl/mba_test.c | 20 +++++++++++--------- > tools/testing/selftests/resctrl/mbm_test.c | 13 +++++++------ > 2 files changed, 18 insertions(+), 15 deletions(-) > > diff --git a/tools/testing/selftests/resctrl/mba_test.c b/tools/testing/selftests/resctrl/mba_test.c > index 6449fbd96096..b4c81d2ee53b 100644 > --- a/tools/testing/selftests/resctrl/mba_test.c > +++ b/tools/testing/selftests/resctrl/mba_test.c > @@ -12,7 +12,7 @@ > > #define RESULT_FILE_NAME "result_mba" > #define NUM_OF_RUNS 5 > -#define MAX_DIFF 300 > +#define MAX_DIFF_PERCENT 5 > #define ALLOCATION_MAX 100 > #define ALLOCATION_MIN 10 > #define ALLOCATION_STEP 10 > @@ -62,7 +62,8 @@ static void show_mba_info(unsigned long *bw_imc, unsigned long *bw_resc) > allocation++) { > unsigned long avg_bw_imc, avg_bw_resc; > unsigned long sum_bw_imc = 0, sum_bw_resc = 0; > - unsigned long avg_diff; > + int avg_diff_per; > + float avg_diff; > > /* > * The first run is discarded due to inaccurate value from > @@ -76,17 +77,18 @@ static void show_mba_info(unsigned long *bw_imc, unsigned long *bw_resc) > > avg_bw_imc = sum_bw_imc / (NUM_OF_RUNS - 1); > avg_bw_resc = sum_bw_resc / (NUM_OF_RUNS - 1); > - avg_diff = labs((long)(avg_bw_resc - avg_bw_imc)); > + avg_diff = (float)labs(avg_bw_resc - avg_bw_imc) / avg_bw_imc; > + avg_diff_per = (int)(avg_diff * 100); > > - printf("%sok MBA schemata percentage %u smaller than %d %%\n", > - avg_diff > MAX_DIFF ? "not " : "", > - ALLOCATION_MAX - ALLOCATION_STEP * allocation, > - MAX_DIFF); > + printf("%sok MBA: diff within %d%% for schemata %u\n", > + avg_diff_per > MAX_DIFF_PERCENT ? "not " : "", > + MAX_DIFF_PERCENT, > + ALLOCATION_MAX - ALLOCATION_STEP * allocation); > tests_run++; > - printf("# avg_diff: %lu\n", avg_diff); > + printf("# avg_diff_per: %d%%\n", avg_diff_per); > printf("# avg_bw_imc: %lu\n", avg_bw_imc); > printf("# avg_bw_resc: %lu\n", avg_bw_resc); > - if (avg_diff > MAX_DIFF) > + if (avg_diff_per > MAX_DIFF_PERCENT) > failed = true; > } > > diff --git a/tools/testing/selftests/resctrl/mbm_test.c b/tools/testing/selftests/resctrl/mbm_test.c > index ec6cfe01c9c2..672d3ddd6e85 100644 > --- a/tools/testing/selftests/resctrl/mbm_test.c > +++ b/tools/testing/selftests/resctrl/mbm_test.c > @@ -11,7 +11,7 @@ > #include "resctrl.h" > > #define RESULT_FILE_NAME "result_mbm" > -#define MAX_DIFF 300 > +#define MAX_DIFF_PERCENT 5 > #define NUM_OF_RUNS 5 > > static void > @@ -19,8 +19,8 @@ show_bw_info(unsigned long *bw_imc, unsigned long *bw_resc, int span) > { > unsigned long avg_bw_imc = 0, avg_bw_resc = 0; > unsigned long sum_bw_imc = 0, sum_bw_resc = 0; > - long avg_diff = 0; > - int runs; > + int runs, avg_diff_per; > + float avg_diff = 0; > > /* > * Discard the first value which is inaccurate due to monitoring setup > @@ -33,12 +33,13 @@ show_bw_info(unsigned long *bw_imc, unsigned long *bw_resc, int span) > > avg_bw_imc = sum_bw_imc / 4; > avg_bw_resc = sum_bw_resc / 4; > - avg_diff = avg_bw_resc - avg_bw_imc; > + avg_diff = (float)labs(avg_bw_resc - avg_bw_imc) / avg_bw_imc; > + avg_diff_per = (int)(avg_diff * 100); > > printf("%sok MBM: diff within %d%%\n", > - labs(avg_diff) > MAX_DIFF ? "not " : "", MAX_DIFF); > + avg_diff_per > MAX_DIFF_PERCENT ? "not " : "", MAX_DIFF_PERCENT); > tests_run++; > - printf("# avg_diff: %lu\n", labs(avg_diff)); > + printf("# avg_diff_per: %d%%\n", avg_diff_per); > printf("# Span (MB): %d\n", span); > printf("# avg_bw_imc: %lu\n", avg_bw_imc); > printf("# avg_bw_resc: %lu\n", avg_bw_resc); >
Can we move all the name changes after the real fixes?
thanks, -- Shuah
| |