Hi Reinette, > On 9/13/2022 6:51 PM, Shaopeng Tan wrote: > > Before exiting each test function(run_cmt/cat/mbm/mba_test()), > > test results are printed by ksft_print_msg() and then temporary result > > files are cleaned by function cmt/cat/mbm/mba_test_cleanup(). > > However, before running ksft_print_msg(), function > > before -> after? I think it is "before". > > 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. > > > > Another good catch, thank you. > > > Signed-off-by: Shaopeng Tan <tan.shaopeng@xxxxxxxxxxxxxx> > > --- > > tools/testing/selftests/resctrl/cat_test.c | 1 - > > tools/testing/selftests/resctrl/cmt_test.c | 2 -- > > tools/testing/selftests/resctrl/mba_test.c | 2 -- > > tools/testing/selftests/resctrl/mbm_test.c | 2 -- > > 4 files changed, 7 deletions(-) > > > > diff --git a/tools/testing/selftests/resctrl/cat_test.c > > b/tools/testing/selftests/resctrl/cat_test.c > > index 1c5e90c63254..d1134f66469f 100644 > > --- a/tools/testing/selftests/resctrl/cat_test.c > > +++ b/tools/testing/selftests/resctrl/cat_test.c > > @@ -221,7 +221,6 @@ int cat_perf_miss_val(int cpu_no, int n, char > *cache_type) > > kill(bm_pid, SIGKILL); > > } > > > > - cat_test_cleanup(); > > if (bm_pid) > > umount_resctrlfs(); > > > > It makes it much easier to understand code if it is symmetrical. Since the files > are created within cat_perf_miss_val() I think it would be better to perform the > cleanup in the same function. So, keep this cleanup but remove the call from > run_cat_test() instead. > > Similar for the cleanups below ... could you please keep them and instead > remove the duplicate cleanup from run_cmt/mbm/mba_test() instead? > > When you do so, please be careful since it seems that there is (another!) bug > where the cleanup is not done if the test fails. Thanks for your advices, I will remove the duplicate cleanups from run_cmt/mbm/mba_test(). Best Regards, Shaopeng > > diff --git a/tools/testing/selftests/resctrl/cmt_test.c > > b/tools/testing/selftests/resctrl/cmt_test.c > > index 8968e36db99d..b3b17fb876f4 100644 > > --- a/tools/testing/selftests/resctrl/cmt_test.c > > +++ b/tools/testing/selftests/resctrl/cmt_test.c > > @@ -139,7 +139,5 @@ int cmt_resctrl_val(int cpu_no, int n, char > **benchmark_cmd) > > if (ret) > > return ret; > > > > - cmt_test_cleanup(); > > - > > return 0; > > } > > diff --git a/tools/testing/selftests/resctrl/mba_test.c > > b/tools/testing/selftests/resctrl/mba_test.c > > index 1a1bdb6180cf..93ffacb416df 100644 > > --- a/tools/testing/selftests/resctrl/mba_test.c > > +++ b/tools/testing/selftests/resctrl/mba_test.c > > @@ -166,7 +166,5 @@ int mba_schemata_change(int cpu_no, char > *bw_report, char **benchmark_cmd) > > if (ret) > > return ret; > > > > - mba_test_cleanup(); > > - > > return 0; > > } > > diff --git a/tools/testing/selftests/resctrl/mbm_test.c > > b/tools/testing/selftests/resctrl/mbm_test.c > > index 38a3b3ad1c76..a453db4c221f 100644 > > --- a/tools/testing/selftests/resctrl/mbm_test.c > > +++ b/tools/testing/selftests/resctrl/mbm_test.c > > @@ -134,7 +134,5 @@ int mbm_bw_change(int span, int cpu_no, char > *bw_report, char **benchmark_cmd) > > if (ret) > > return ret; > > > > - mbm_test_cleanup(); > > - > > return 0; > > } > > Thank you > > Reinette
