[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <112f84b0-c7c1-7b92-75f9-d71d557ecbe2@linuxfoundation.org>
Date: Mon, 25 Jan 2021 19:38:07 -0700
From: Shuah Khan <skhan@...uxfoundation.org>
To: Fenghua Yu <fenghua.yu@...el.com>, Shuah Khan <shuah@...nel.org>,
Tony Luck <tony.luck@...el.com>,
Reinette Chatre <reinette.chatre@...el.com>,
David Binderman <dcb314@...mail.com>,
Babu Moger <babu.moger@....com>,
James Morse <james.morse@....com>,
Ravi V Shankar <ravi.v.shankar@...el.com>,
Shuah Khan <skhan@...uxfoundation.org>
Cc: linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 10/17] selftests/resctrl: Fix MBA/MBM results reporting
format
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@...el.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
Powered by blists - more mailing lists