From: Reinette Chatre <reinette.chatre@intel.com>
To: Shaopeng Tan <tan.shaopeng@jp.fujitsu.com>,
Fenghua Yu <fenghua.yu@intel.com>, Shuah Khan <shuah@kernel.org>
Cc: <linux-kernel@vger.kernel.org>, <linux-kselftest@vger.kernel.org>
Subject: Re: [PATCH v3 5/5] selftests/resctrl: Remove duplicate codes that clear each test result file
Date: Mon, 7 Nov 2022 15:53:58 -0800 [thread overview]
Message-ID: <afa849bf-e89b-0c48-6bf2-f7ca58940567@intel.com> (raw)
In-Reply-To: <20221101094341.3383073-6-tan.shaopeng@jp.fujitsu.com>
Hi Shaopeng,
On 11/1/2022 2:43 AM, Shaopeng Tan wrote:
> Before exiting each test function(run_cmt/cat/mbm/mba_test()),
> test results("ok","not ok") are printed by ksft_test_result() and then
> temporary result files are cleaned by function
> cmt/cat/mbm/mba_test_cleanup().
> However, before running ksft_test_result(),
> function cmt/cat/mbm/mba_test_cleanup()
> has been run in each test function as follows:
> cmt_resctrl_val()
> cat_perf_miss_val()
> mba_schemata_change()
> mbm_bw_change()
>
> Remove duplicate codes that clear each test result file.
>
> Signed-off-by: Shaopeng Tan <tan.shaopeng@jp.fujitsu.com>
> ---
> tools/testing/selftests/resctrl/resctrl_tests.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c b/tools/testing/selftests/resctrl/resctrl_tests.c
> index df0d8d8526fc..8732cf736528 100644
> --- a/tools/testing/selftests/resctrl/resctrl_tests.c
> +++ b/tools/testing/selftests/resctrl/resctrl_tests.c
> @@ -88,7 +88,6 @@ static void run_mbm_test(bool has_ben, char **benchmark_cmd, int span,
> ksft_test_result(!res, "MBM: bw change\n");
> if ((get_vendor() == ARCH_INTEL) && res)
> ksft_print_msg("Intel MBM may be inaccurate when Sub-NUMA Clustering is enabled. Check BIOS configuration.\n");
> - mbm_test_cleanup();
> }
>
From what I can tell this still seem to suffer from the problem where
the test files may not be cleaned. With the removal of mbm_test_cleanup()
the cleanup is now expected to be done in mbm_bw_change().
Note that:
mbm_bw_change()
{
...
ret = resctrl_val(benchmark_cmd, ¶m);
if (ret)
return ret;
/* Test results stored in file */
ret = check_results(span);
if (ret)
return ret; <== Return without cleaning test result file
mbm_test_cleanup(); <== Test result file cleaned only when test passed.
return 0;
}
> static void run_mba_test(bool has_ben, char **benchmark_cmd, int span,
> @@ -107,7 +106,6 @@ static void run_mba_test(bool has_ben, char **benchmark_cmd, int span,
> sprintf(benchmark_cmd[1], "%d", span);
> res = mba_schemata_change(cpu_no, bw_report, benchmark_cmd);
> ksft_test_result(!res, "MBA: schemata change\n");
> - mba_test_cleanup();
> }
mba_schemata_change() has the same pattern as mbm_bw_change() so test result
files may linger when test fails.
>
> static void run_cmt_test(bool has_ben, char **benchmark_cmd, int cpu_no)
> @@ -126,7 +124,6 @@ static void run_cmt_test(bool has_ben, char **benchmark_cmd, int cpu_no)
> ksft_test_result(!res, "CMT: test\n");
> if ((get_vendor() == ARCH_INTEL) && res)
> ksft_print_msg("Intel CMT may be inaccurate when Sub-NUMA Clustering is enabled. Check BIOS configuration.\n");
> - cmt_test_cleanup();
> }
Same pattern again.
>
> static void run_cat_test(int cpu_no, int no_of_bits)
> @@ -142,7 +139,6 @@ static void run_cat_test(int cpu_no, int no_of_bits)
>
> res = cat_perf_miss_val(cpu_no, no_of_bits, "L3");
> ksft_test_result(!res, "CAT: test\n");
> - cat_test_cleanup();
> }
Patch 4 makes this work. Thanks for doing that.
Reinette
next prev parent reply other threads:[~2022-11-07 23:54 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-01 9:43 [PATCH v3 0/5] Some improvements of resctrl selftest Shaopeng Tan
2022-11-01 9:43 ` [PATCH v3 1/5] selftests/resctrl: Fix set up schemata with 100% allocation on first run in MBM test Shaopeng Tan
2022-11-08 0:04 ` Reinette Chatre
2022-11-01 9:43 ` [PATCH v3 2/5] selftests/resctrl: Return MBA check result and make it to output message Shaopeng Tan
2022-11-02 9:27 ` Shuah Khan
2022-11-01 9:43 ` [PATCH v3 3/5] selftests/resctrl: Flush stdout file buffer before executing fork() Shaopeng Tan
2022-11-02 9:29 ` Shuah Khan
2022-11-01 9:43 ` [PATCH v3 4/5] selftests/resctrl: Cleanup properly when an error occurs in CAT test Shaopeng Tan
2022-11-02 9:41 ` Shuah Khan
2022-11-07 22:31 ` Reinette Chatre
2022-11-08 8:32 ` Shaopeng Tan (Fujitsu)
2022-11-07 22:40 ` Reinette Chatre
2022-11-01 9:43 ` [PATCH v3 5/5] selftests/resctrl: Remove duplicate codes that clear each test result file Shaopeng Tan
2022-11-02 9:51 ` Shuah Khan
2022-11-07 23:53 ` Reinette Chatre [this message]
2022-11-08 8:32 ` Shaopeng Tan (Fujitsu)
2022-11-08 17:07 ` Reinette Chatre
2022-11-10 7:43 ` Shaopeng Tan (Fujitsu)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=afa849bf-e89b-0c48-6bf2-f7ca58940567@intel.com \
--to=reinette.chatre@intel.com \
--cc=fenghua.yu@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=shuah@kernel.org \
--cc=tan.shaopeng@jp.fujitsu.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).